adutra commented on code in PR #1817: URL: https://github.com/apache/polaris/pull/1817#discussion_r2129171982
########## quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/PolarisRequestContext.java: ########## @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.service.quarkus.config; + +import jakarta.enterprise.context.RequestScoped; +import jakarta.ws.rs.container.ContainerRequestContext; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.polaris.core.context.RealmContext; + +/** + * A container for request-scoped information discovered during request execution. + * + * <p>This is an equivalent for {@link ContainerRequestContext}, but for use in non-HTTP requests. + */ +@RequestScoped +public class PolarisRequestContext { + private final AtomicReference<RealmContext> realmCtx = new AtomicReference<>(); + + /** + * Records the {@link RealmContext} that applies to current request. The realm context may be + * determined from REST API header or by passing explicit realm ID values from one CDI context to + * another. + * + * <p>During the execution of a particular request, this method should be called before {@link + * #realmContext()}. + */ + public void setRealmContext(RealmContext rc) { + System.out.println("SET: " + this + ": RC: " + rc); + realmCtx.set(rc); + } + + /** + * Returns the realm context for this request previously set via {@link + * #setRealmContext(RealmContext)}. + */ + public RealmContext realmContext() { + return realmCtx.get(); Review Comment: Should we add a runtime check here to prevent returning null? ########## quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisAuthzTestBase.java: ########## @@ -242,11 +238,16 @@ public void before(TestInfo testInfo) { diagServices, configurationStore, clock); - this.entityManager = realmEntityManagerFactory.getOrCreateEntityManager(realmContext); - callContext = polarisContext; CallContext.setCurrentContext(callContext); + metaStoreManager = managerFactory.getOrCreateMetaStoreManager(realmContext); Review Comment: What is the motivation for this change? ########## quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogViewTest.java: ########## @@ -175,15 +175,14 @@ public void before(TestInfo testInfo) { diagServices, configurationStore, Clock.systemDefaultZone()); + CallContext.setCurrentContext(polarisContext); Review Comment: Same: is this change required? ########## service/common/src/main/java/org/apache/polaris/service/config/DefaultConfigurationStore.java: ########## @@ -51,7 +54,10 @@ public DefaultConfigurationStore( @Override public <T> @Nullable T getConfiguration(@Nonnull RealmContext realmContext, String configName) { - String realm = realmContext.getRealmIdentifier(); + String realm = realmContextInstance.get().getRealmIdentifier(); Review Comment: Can you explain the rationale here? I thought we would be removing `realmContextInstance` completely. It's imho more elegant to receive the realm context as a method parameter than injecting an `Instance<RealmContext>` – especially since this bean is application-scoped, so receiving a realm context as a method parameter aligns more with the common idiom that we have everywhere in Polaris where an application-scoped bean exposes a method that takes a realm context and returns something for that realm (e.g. all the `getOrCreateXYZ` methods). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
