From 7f5a713721daaa9075265aee0c7bcad972d673b8 Mon Sep 17 00:00:00 2001 From: Yufei Date: Sun, 16 Nov 2025 16:36:12 -0800 Subject: [PATCH 1/3] Core: resolveAll() must be called before reading resolution results --- .../resolver/PolarisResolutionManifest.java | 14 ++++- .../PolarisResolutionManifestTest.java | 55 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 polaris-core/src/test/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifestTest.java diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java index 1b9c26e02a..7f5d39bbaa 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java @@ -68,6 +68,14 @@ public class PolarisResolutionManifest implements PolarisResolutionManifestCatal // Set when resolveAll is called private ResolverStatus primaryResolverStatus = null; + private ResolverStatus requirePrimaryResolverStatus() { + diagnostics.checkNotNull( + primaryResolverStatus, + "resolver_not_run_before_access", + "resolveAll() must be called before reading resolution results"); + return primaryResolverStatus; + } + public PolarisResolutionManifest( PolarisDiagnostics diagnostics, RealmContext realmContext, @@ -256,7 +264,7 @@ public Set getAllActivatedPrincipalRoleEntities() { } private @Nullable ResolvedPolarisEntity getResolvedRootContainerEntity() { - if (primaryResolverStatus.getStatus() != ResolverStatus.StatusEnum.SUCCESS) { + if (requirePrimaryResolverStatus().getStatus() != ResolverStatus.StatusEnum.SUCCESS) { return null; } ResolvedPolarisEntity resolvedEntity = @@ -277,6 +285,7 @@ public PolarisResolvedPathWrapper getResolvedRootContainerEntityAsPath() { public PolarisResolvedPathWrapper getResolvedReferenceCatalogEntity( boolean prependRootContainer) { + requirePrimaryResolverStatus(); // This is a server error instead of being able to legitimately return null, since this means // a callsite failed to incorporate a reference catalog into its authorization flow but is // still trying to perform operations on the (nonexistence) reference catalog. @@ -298,6 +307,7 @@ public PolarisResolvedPathWrapper getResolvedReferenceCatalogEntity( } public PolarisEntitySubType getLeafSubType(Object key) { + requirePrimaryResolverStatus(); diagnostics.check( pathLookup.containsKey(key), "never_registered_key_for_resolved_path", @@ -320,6 +330,7 @@ public PolarisEntitySubType getLeafSubType(Object key) { * "optional" */ public PolarisResolvedPathWrapper getResolvedPath(Object key, boolean prependRootContainer) { + requirePrimaryResolverStatus(); diagnostics.check( pathLookup.containsKey(key), "never_registered_key_for_resolved_path", @@ -384,6 +395,7 @@ public PolarisResolvedPathWrapper getResolvedPath( public PolarisResolvedPathWrapper getResolvedTopLevelEntity( String entityName, PolarisEntityType entityType) { + requirePrimaryResolverStatus(); // For now, all top-level entities will have the root container prepended so we don't have // a variation of this method that allows specifying whether to prepend the root container. diagnostics.check( diff --git a/polaris-core/src/test/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifestTest.java b/polaris-core/src/test/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifestTest.java new file mode 100644 index 0000000000..23f6d8e2b6 --- /dev/null +++ b/polaris-core/src/test/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifestTest.java @@ -0,0 +1,55 @@ +/* + * 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.core.persistence.resolver; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; +import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.auth.PolarisPrincipal; +import org.apache.polaris.core.context.RealmContext; +import org.apache.polaris.core.entity.PolarisEntityType; +import org.junit.jupiter.api.Test; + +class PolarisResolutionManifestTest { + + @Test + void accessingResultsBeforeResolveAllFailsFast() { + PolarisDiagnostics diagnostics = new PolarisDefaultDiagServiceImpl(); + RealmContext realmContext = () -> "test-realm"; + PolarisPrincipal principal = mock(PolarisPrincipal.class); + + Resolver resolver = mock(Resolver.class); + ResolverFactory resolverFactory = mock(ResolverFactory.class); + when(resolverFactory.createResolver(principal, "test-catalog")).thenReturn(resolver); + + PolarisResolutionManifest manifest = + new PolarisResolutionManifest( + diagnostics, realmContext, resolverFactory, principal, "test-catalog"); + + assertThrows(NullPointerException.class, () -> manifest.getResolvedPath("key")); + assertThrows(NullPointerException.class, () -> manifest.getResolvedReferenceCatalogEntity()); + assertThrows(NullPointerException.class, () -> manifest.getLeafSubType("key")); + assertThrows( + NullPointerException.class, + () -> manifest.getResolvedTopLevelEntity("catalog", PolarisEntityType.CATALOG)); + } +} From 0bfbb550e49c1b97deddffdd8a8ac5c9336cf450 Mon Sep 17 00:00:00 2001 From: Yufei Date: Tue, 18 Nov 2025 20:36:04 -0800 Subject: [PATCH 2/3] Resolve comments --- .../resolver/PolarisResolutionManifest.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java index 7f5d39bbaa..4f38758dbe 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifest.java @@ -68,12 +68,12 @@ public class PolarisResolutionManifest implements PolarisResolutionManifestCatal // Set when resolveAll is called private ResolverStatus primaryResolverStatus = null; - private ResolverStatus requirePrimaryResolverStatus() { + private boolean isResolveAllSucceeded() { diagnostics.checkNotNull( primaryResolverStatus, "resolver_not_run_before_access", "resolveAll() must be called before reading resolution results"); - return primaryResolverStatus; + return primaryResolverStatus.getStatus() == ResolverStatus.StatusEnum.SUCCESS; } public PolarisResolutionManifest( @@ -264,7 +264,7 @@ public Set getAllActivatedPrincipalRoleEntities() { } private @Nullable ResolvedPolarisEntity getResolvedRootContainerEntity() { - if (requirePrimaryResolverStatus().getStatus() != ResolverStatus.StatusEnum.SUCCESS) { + if (!isResolveAllSucceeded()) { return null; } ResolvedPolarisEntity resolvedEntity = @@ -285,7 +285,6 @@ public PolarisResolvedPathWrapper getResolvedRootContainerEntityAsPath() { public PolarisResolvedPathWrapper getResolvedReferenceCatalogEntity( boolean prependRootContainer) { - requirePrimaryResolverStatus(); // This is a server error instead of being able to legitimately return null, since this means // a callsite failed to incorporate a reference catalog into its authorization flow but is // still trying to perform operations on the (nonexistence) reference catalog. @@ -307,7 +306,6 @@ public PolarisResolvedPathWrapper getResolvedReferenceCatalogEntity( } public PolarisEntitySubType getLeafSubType(Object key) { - requirePrimaryResolverStatus(); diagnostics.check( pathLookup.containsKey(key), "never_registered_key_for_resolved_path", @@ -330,7 +328,6 @@ public PolarisEntitySubType getLeafSubType(Object key) { * "optional" */ public PolarisResolvedPathWrapper getResolvedPath(Object key, boolean prependRootContainer) { - requirePrimaryResolverStatus(); diagnostics.check( pathLookup.containsKey(key), "never_registered_key_for_resolved_path", @@ -338,7 +335,7 @@ public PolarisResolvedPathWrapper getResolvedPath(Object key, boolean prependRoo key, pathLookup); - if (primaryResolverStatus.getStatus() != ResolverStatus.StatusEnum.SUCCESS) { + if (!isResolveAllSucceeded()) { return null; } int index = pathLookup.get(key); @@ -395,7 +392,6 @@ public PolarisResolvedPathWrapper getResolvedPath( public PolarisResolvedPathWrapper getResolvedTopLevelEntity( String entityName, PolarisEntityType entityType) { - requirePrimaryResolverStatus(); // For now, all top-level entities will have the root container prepended so we don't have // a variation of this method that allows specifying whether to prepend the root container. diagnostics.check( @@ -406,7 +402,7 @@ public PolarisResolvedPathWrapper getResolvedTopLevelEntity( entityType, addedTopLevelNames); - if (primaryResolverStatus.getStatus() != ResolverStatus.StatusEnum.SUCCESS) { + if (!isResolveAllSucceeded()) { return null; } From 1e2161d307b0b33d37de1c2426a31e201f165ba1 Mon Sep 17 00:00:00 2001 From: Yufei Date: Tue, 18 Nov 2025 21:09:33 -0800 Subject: [PATCH 3/3] Resolve comments --- .../PolarisResolutionManifestTest.java | 55 ------------------- 1 file changed, 55 deletions(-) delete mode 100644 polaris-core/src/test/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifestTest.java diff --git a/polaris-core/src/test/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifestTest.java b/polaris-core/src/test/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifestTest.java deleted file mode 100644 index 23f6d8e2b6..0000000000 --- a/polaris-core/src/test/java/org/apache/polaris/core/persistence/resolver/PolarisResolutionManifestTest.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * 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.core.persistence.resolver; - -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; -import org.apache.polaris.core.PolarisDiagnostics; -import org.apache.polaris.core.auth.PolarisPrincipal; -import org.apache.polaris.core.context.RealmContext; -import org.apache.polaris.core.entity.PolarisEntityType; -import org.junit.jupiter.api.Test; - -class PolarisResolutionManifestTest { - - @Test - void accessingResultsBeforeResolveAllFailsFast() { - PolarisDiagnostics diagnostics = new PolarisDefaultDiagServiceImpl(); - RealmContext realmContext = () -> "test-realm"; - PolarisPrincipal principal = mock(PolarisPrincipal.class); - - Resolver resolver = mock(Resolver.class); - ResolverFactory resolverFactory = mock(ResolverFactory.class); - when(resolverFactory.createResolver(principal, "test-catalog")).thenReturn(resolver); - - PolarisResolutionManifest manifest = - new PolarisResolutionManifest( - diagnostics, realmContext, resolverFactory, principal, "test-catalog"); - - assertThrows(NullPointerException.class, () -> manifest.getResolvedPath("key")); - assertThrows(NullPointerException.class, () -> manifest.getResolvedReferenceCatalogEntity()); - assertThrows(NullPointerException.class, () -> manifest.getLeafSubType("key")); - assertThrows( - NullPointerException.class, - () -> manifest.getResolvedTopLevelEntity("catalog", PolarisEntityType.CATALOG)); - } -}