Skip to content

Consolidate CallContext with PolarisCallContext #1806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 4, 2025
Merged

Consolidate CallContext with PolarisCallContext #1806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
226789c
Consolidate CallContext with PolarisCallContext
Jun 3, 2025
c4a9c44
Fix copyOf used by the taskHanlder
Jun 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions ...main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,11 @@ private PrincipalSecretsResult bootstrapServiceAndCreatePolarisPrincipalForRealm
// CallContext may not have been resolved yet.
PolarisCallContext polarisContext =
new PolarisCallContext(
sessionSupplierMap.get(realmContext.getRealmIdentifier()).get(), diagServices);
realmContext,
sessionSupplierMap.get(realmContext.getRealmIdentifier()).get(),
diagServices);
if (CallContext.getCurrentContext() == null) {
CallContext.setCurrentContext(CallContext.of(realmContext, polarisContext));
CallContext.setCurrentContext(polarisContext);
}

EntityResult preliminaryRootPrincipalLookup =
Expand Down Expand Up @@ -286,9 +288,11 @@ private void checkPolarisServiceBootstrappedForRealm(
RealmContext realmContext, PolarisMetaStoreManager metaStoreManager) {
PolarisCallContext polarisContext =
new PolarisCallContext(
sessionSupplierMap.get(realmContext.getRealmIdentifier()).get(), diagServices);
realmContext,
sessionSupplierMap.get(realmContext.getRealmIdentifier()).get(),
diagServices);
if (CallContext.getCurrentContext() == null) {
CallContext.setCurrentContext(CallContext.of(realmContext, polarisContext));
CallContext.setCurrentContext(polarisContext);
}

EntityResult rootPrincipalLookup =
Expand Down
49 changes: 41 additions & 8 deletions polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@
import java.time.Clock;
import java.time.ZoneId;
import org.apache.polaris.core.config.PolarisConfigurationStore;
import org.apache.polaris.core.context.CallContext;
import org.apache.polaris.core.context.RealmContext;
import org.apache.polaris.core.persistence.BasePersistence;

/**
* The Call context is allocated each time a new REST request is processed. It contains instances of
* low-level services required to process that request
*/
public class PolarisCallContext {
public class PolarisCallContext implements CallContext {
Copy link
Contributor

@eric-maynard eric-maynard Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, why not go the other way? It seems like it would be easier to have CallContext extend PolarisCallContext, since it can immediately implement all the methods just by delegating to the current PolarisCallContext

Copy link
Contributor Author

@flyrain flyrain Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's also a nice idea, however, it will need to change CallContext from an interface to a class, which isn't trivial -- all anonymous classes have to go. Besides, it's reasonable to keep CallContext as an interface, while PolarisCallContext as an impl. I also list the step 3 which promotes methods in PolarisCallContext to the interface CallContext, like getMetaStore(). This will even clean up more, so that, PolarisCallContext doesn't have to return itself from the method getPolarisCallContext().


// meta store which is used to persist Polaris entity metadata
private final BasePersistence metaStore;
Expand All @@ -40,31 +42,52 @@ public class PolarisCallContext {

private final Clock clock;

// will make it final once we remove deprecated constructor
private RealmContext realmContext = null;

public PolarisCallContext(
@Nonnull RealmContext realmContext,
@Nonnull BasePersistence metaStore,
@Nonnull PolarisDiagnostics diagServices,
@Nonnull PolarisConfigurationStore configurationStore,
@Nonnull Clock clock) {
this.realmContext = realmContext;
this.metaStore = metaStore;
this.diagServices = diagServices;
this.configurationStore = configurationStore;
this.clock = clock;
}

@Deprecated
public PolarisCallContext(
@Nonnull BasePersistence metaStore, @Nonnull PolarisDiagnostics diagServices) {
@Nonnull BasePersistence metaStore,
@Nonnull PolarisDiagnostics diagServices,
@Nonnull PolarisConfigurationStore configurationStore,
@Nonnull Clock clock) {
this.metaStore = metaStore;
this.diagServices = diagServices;
this.configurationStore = configurationStore;
this.clock = clock;
}

public PolarisCallContext(
@Nonnull RealmContext realmContext,
@Nonnull BasePersistence metaStore,
@Nonnull PolarisDiagnostics diagServices) {
this.realmContext = realmContext;
this.metaStore = metaStore;
this.diagServices = diagServices;
this.configurationStore = new PolarisConfigurationStore() {};
this.clock = Clock.system(ZoneId.systemDefault());
Copy link
Contributor

@dimas-b dimas-b Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not Clock.systemUTC()?

Copy link
Contributor Author

@flyrain flyrain Jun 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a copy from the deprecated method, see line 90. I'm OK to change it to UTC. I'd suggest to make that change in a separate PR, as it isn't related to the refactor in this PR.

Copy link
Contributor

@dimas-b dimas-b Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok to defer, but we must not forget... IIRC we already have a mix of clocks in the codebase. It'd be nice to have a uniform pattern.

}

public static PolarisCallContext copyOf(PolarisCallContext original) {
return new PolarisCallContext(
original.getMetaStore().detach(),
original.getDiagServices(),
original.getConfigurationStore(),
original.getClock());
@Deprecated
public PolarisCallContext(
@Nonnull BasePersistence metaStore, @Nonnull PolarisDiagnostics diagServices) {
this.metaStore = metaStore;
this.diagServices = diagServices;
this.configurationStore = new PolarisConfigurationStore() {};
this.clock = Clock.system(ZoneId.systemDefault());
}

public BasePersistence getMetaStore() {
Expand All @@ -82,4 +105,14 @@ public PolarisConfigurationStore getConfigurationStore() {
public Clock getClock() {
return clock;
}

@Override
public RealmContext getRealmContext() {
return realmContext;
}

@Override
public PolarisCallContext getPolarisCallContext() {
return this;
}
}
13 changes: 11 additions & 2 deletions polaris-core/src/main/java/org/apache/polaris/core/context/CallContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ static void unsetCurrentContext() {
CURRENT_CONTEXT.remove();
}

// only tests are using this method now, we can get rid of them easily in a followup
static CallContext of(
final RealmContext realmContext, final PolarisCallContext polarisCallContext) {
return new CallContext() {
Expand All @@ -68,7 +69,15 @@ public PolarisCallContext getPolarisCallContext() {
static CallContext copyOf(CallContext base) {
String realmId = base.getRealmContext().getRealmIdentifier();
RealmContext realmContext = () -> realmId;
PolarisCallContext polarisCallContext = PolarisCallContext.copyOf(base.getPolarisCallContext());
PolarisCallContext originalPolarisCallContext = base.getPolarisCallContext();
PolarisCallContext newPolarisCallContext =
new PolarisCallContext(
realmContext,
originalPolarisCallContext.getMetaStore(),
originalPolarisCallContext.getDiagServices(),
originalPolarisCallContext.getConfigurationStore(),
originalPolarisCallContext.getClock());

return new CallContext() {
@Override
public RealmContext getRealmContext() {
Expand All @@ -77,7 +86,7 @@ public RealmContext getRealmContext() {

@Override
public PolarisCallContext getPolarisCallContext() {
return polarisCallContext;
return newPolarisCallContext;
}
};
}
Expand Down
14 changes: 9 additions & 5 deletions ...rc/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,13 @@ private PrincipalSecretsResult bootstrapServiceAndCreatePolarisPrincipalForRealm
RealmContext realmContext, PolarisMetaStoreManager metaStoreManager) {
// While bootstrapping we need to act as a fake privileged context since the real
// CallContext may not have been resolved yet.
PolarisCallContext polarisContext =
var polarisContext =
new PolarisCallContext(
sessionSupplierMap.get(realmContext.getRealmIdentifier()).get(), diagServices);
realmContext,
sessionSupplierMap.get(realmContext.getRealmIdentifier()).get(),
diagServices);
if (CallContext.getCurrentContext() == null) {
CallContext.setCurrentContext(CallContext.of(realmContext, polarisContext));
CallContext.setCurrentContext(polarisContext);
}

EntityResult preliminaryRootPrincipalLookup =
Expand Down Expand Up @@ -251,9 +253,11 @@ private void checkPolarisServiceBootstrappedForRealm(
RealmContext realmContext, PolarisMetaStoreManager metaStoreManager) {
PolarisCallContext polarisContext =
new PolarisCallContext(
sessionSupplierMap.get(realmContext.getRealmIdentifier()).get(), diagServices);
realmContext,
sessionSupplierMap.get(realmContext.getRealmIdentifier()).get(),
diagServices);
if (CallContext.getCurrentContext() == null) {
CallContext.setCurrentContext(CallContext.of(realmContext, polarisContext));
CallContext.setCurrentContext(polarisContext);
}

EntityResult rootPrincipalLookup =
Expand Down
11 changes: 3 additions & 8 deletions ...kus/service/src/main/java/org/apache/polaris/service/quarkus/config/QuarkusProducers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,21 +121,16 @@ public RealmContext realmContext(@Context ContainerRequestContext request) {

@Produces
@RequestScoped
public PolarisCallContext polarisCallContext(
public CallContext polarisCallContext(
RealmContext realmContext,
PolarisDiagnostics diagServices,
PolarisConfigurationStore configurationStore,
MetaStoreManagerFactory metaStoreManagerFactory,
Clock clock) {
BasePersistence metaStoreSession =
metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get();
return new PolarisCallContext(metaStoreSession, diagServices, configurationStore, clock);
}

@Produces
@RequestScoped
public CallContext callContext(RealmContext realmContext, PolarisCallContext polarisCallContext) {
return CallContext.of(realmContext, polarisCallContext);
return new PolarisCallContext(
realmContext, metaStoreSession, diagServices, configurationStore, clock);
}

// Polaris service beans - selected from @Identifier-annotated beans
Expand Down
5 changes: 2 additions & 3 deletions ...e/common/src/main/java/org/apache/polaris/service/context/DefaultCallContextResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ public CallContext resolveCallContext(
// factories would then inject something else instead if needed.
BasePersistence metaStoreSession =
metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get();
PolarisCallContext polarisContext =
new PolarisCallContext(metaStoreSession, diagnostics, configurationStore, clock);
return CallContext.of(realmContext, polarisContext);
return new PolarisCallContext(
realmContext, metaStoreSession, diagnostics, configurationStore, clock);
}
}
Loading