You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by sn...@apache.org on 2026/05/28 05:08:12 UTC
(polaris) branch main updated: NoSQL: Scope authz Jackson privilege resolution to each mapper (#4544)
This is an automated email from the ASF dual-hosted git repository.
snazy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 3401adf96e NoSQL: Scope authz Jackson privilege resolution to each mapper (#4544)
3401adf96e is described below
commit 3401adf96ec69f56ff9d939ecc3bb9b4145452f5
Author: Robert Stupp <sn...@snazy.de>
AuthorDate: Thu May 28 07:08:08 2026 +0200
NoSQL: Scope authz Jackson privilege resolution to each mapper (#4544)
Stop sharing mutable privilege resolution state across Jackson mappers.
The previous setup leaked deserializer and serializer context between mappers
and made tests depend on mapper construction order. Keep privilege resolution
local to the module/mapper setup.
---
.../nosql/authz/impl/AclDeserializer.java | 13 +++-
.../nosql/authz/impl/JacksonPrivilegesModule.java | 89 +++++++++++-----------
.../nosql/authz/impl/PrivilegeSetDeserializer.java | 25 ++++--
.../nosql/authz/impl/PrivilegeSetSerializer.java | 13 +++-
.../nosql/authz/impl/TestAclEntryImpl.java | 7 +-
.../persistence/nosql/authz/impl/TestAclImpl.java | 3 +-
.../nosql/authz/impl/TestPrivilegeCheckImpl.java | 1 -
.../nosql/authz/impl/TestPrivilegeSetImpl.java | 16 +++-
8 files changed, 100 insertions(+), 67 deletions(-)
diff --git a/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/AclDeserializer.java b/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/AclDeserializer.java
index 6bef3cb976..5ac63539a2 100644
--- a/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/AclDeserializer.java
+++ b/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/AclDeserializer.java
@@ -18,23 +18,34 @@
*/
package org.apache.polaris.persistence.nosql.authz.impl;
+import static java.util.Objects.requireNonNull;
+
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonMappingException;
import java.io.IOException;
+import java.util.function.Supplier;
import org.apache.polaris.persistence.nosql.authz.api.Acl;
import org.apache.polaris.persistence.nosql.authz.api.AclEntry;
+import org.apache.polaris.persistence.nosql.authz.api.Privileges;
class AclDeserializer extends JsonDeserializer<Acl> {
+ private final Supplier<Privileges> privilegesResolver;
+
+ AclDeserializer(Supplier<Privileges> privilegesResolver) {
+ this.privilegesResolver =
+ requireNonNull(privilegesResolver, "privilegesResolver must not be null");
+ }
+
@Override
public Acl deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
if (p.currentToken() != JsonToken.START_OBJECT) {
throw new JsonMappingException(p, "Unexpected token " + p.currentToken());
}
- var privileges = JacksonPrivilegesModule.currentPrivileges();
+ var privileges = privilegesResolver.get();
var builder = AclImpl.builder(privileges);
for (var t = p.nextToken(); t != JsonToken.END_OBJECT; t = p.nextToken()) {
if (t == JsonToken.FIELD_NAME) {
diff --git a/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/JacksonPrivilegesModule.java b/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/JacksonPrivilegesModule.java
index b5beb49ac0..30667dfc94 100644
--- a/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/JacksonPrivilegesModule.java
+++ b/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/JacksonPrivilegesModule.java
@@ -18,72 +18,71 @@
*/
package org.apache.polaris.persistence.nosql.authz.impl;
-import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
import com.fasterxml.jackson.databind.module.SimpleModule;
-import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.event.Observes;
+import jakarta.enterprise.event.Shutdown;
+import jakarta.enterprise.event.Startup;
import jakarta.enterprise.inject.spi.CDI;
-import java.util.function.Function;
+import jakarta.inject.Inject;
+import java.util.function.Supplier;
import org.apache.polaris.persistence.nosql.authz.api.Acl;
import org.apache.polaris.persistence.nosql.authz.api.PrivilegeSet;
import org.apache.polaris.persistence.nosql.authz.api.Privileges;
public class JacksonPrivilegesModule extends SimpleModule {
+ @SuppressWarnings("unused")
public JacksonPrivilegesModule() {
- addDeserializer(PrivilegeSet.class, new PrivilegeSetDeserializer());
- addSerializer(PrivilegeSet.class, new PrivilegeSetSerializer());
- addDeserializer(Acl.class, new AclDeserializer());
- addSerializer(Acl.class, new AclSerializer());
+ this(PrivilegesViaCDI::privileges);
}
- static Privileges currentPrivileges() {
- return cdiResolve(Privileges.class);
+ JacksonPrivilegesModule(Supplier<Privileges> privilegesResolver) {
+ requireNonNull(privilegesResolver, "privilegesResolver must not be null");
+ addDeserializer(PrivilegeSet.class, new PrivilegeSetDeserializer(privilegesResolver));
+ addSerializer(PrivilegeSet.class, new PrivilegeSetSerializer(privilegesResolver));
+ addDeserializer(Acl.class, new AclDeserializer(privilegesResolver));
+ addSerializer(Acl.class, new AclSerializer());
}
- // TODO the following is the same as in AbstractTypeIdResolver
-
/**
- * Resolve the given type via {@link CDI#current() CDI.current()}. For tests the resolution via
- * CDI can be {@linkplain CDIResolver#setResolver(Function) routed to a custom function}.
+ * Helper to memoize the {@link Privileges} instance in a CDI environment. This class bridges the
+ * gap between CDI and Jackson module lifecycles.
+ *
+ * <p>The implementation attempts to infer the {@link Privileges} instance from the CDI container
+ * by observing the {@link Startup} event. However, other beans that also observe the {@link
+ * Startup} event can trigger a call to the {@link #privileges()} method before this bean's {@link
+ * #init(Startup)} function is called.
*/
- private static <R> R cdiResolve(Class<R> type) {
- // TODO instead of doing the 'CDIResolver' dance, we could (should?) have an attribute in the
- // `DatabindContext` holding a reference to the CDI instance (referred to as `Instance<?>`).
- var resolved = CDIResolver.resolver.apply(type);
- @SuppressWarnings("unchecked")
- var r = (R) resolved;
- return r;
- }
+ @ApplicationScoped
+ static class PrivilegesViaCDI {
+ @Inject Privileges privileges;
- public static final class CDIResolver {
- static Function<Class<?>, ?> resolver = CDIResolver::resolveViaCurrentCDI;
+ private static volatile Privileges privilegesForJackson;
- /**
- * The helper function {@link #cdiResolve(Class)} is used by {@link JacksonPrivilegesModule}
- * implementations to resolve the {@link Privileges} instance, and the default implementation of
- * {@link #cdiResolve(Class)} relies on {@link CDI#current() CDI.current()} to resolve against a
- * "singleton" {@link CDI} instance. Some tests do not use CDI. Setting a custom resolver
- * function helps in such scenarios.
- */
- @SuppressWarnings("unused")
- public static void setResolver(Function<Class<?>, ?> resolver) {
- CDIResolver.resolver = resolver;
+ void init(@Observes Startup startup) {
+ checkState(
+ privilegesForJackson == null || privilegesForJackson == privileges,
+ "A CDI container has already been started");
+ privilegesForJackson = privileges;
}
- /**
- * Manually reset a custom {@linkplain #setResolver(Function) CDI resolver}. This is usually
- * performed automatically after each test case.
- */
- @SuppressWarnings("unused")
- public static void resetResolver() {
- resolver = CDIResolver::resolveViaCurrentCDI;
+ void dispose(@Observes Shutdown shutdown) {
+ privilegesForJackson = null;
}
- private static Object resolveViaCurrentCDI(Class<?> type) {
- Instance<?> selected = CDI.current().select(type);
- checkArgument(selected.isResolvable(), "Cannot resolve %s", type);
- checkArgument(!selected.isAmbiguous(), "Ambiguous type %s", type);
- return selected.get();
+ static Privileges privileges() {
+ var privs = privilegesForJackson;
+ if (privs != null) {
+ // fast path
+ return privs;
+ }
+ // slow path
+ privs = CDI.current().select(Privileges.class).get();
+ privilegesForJackson = privs;
+ return privs;
}
}
}
diff --git a/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/PrivilegeSetDeserializer.java b/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/PrivilegeSetDeserializer.java
index a41635ddaf..49ca7c9241 100644
--- a/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/PrivilegeSetDeserializer.java
+++ b/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/PrivilegeSetDeserializer.java
@@ -18,7 +18,7 @@
*/
package org.apache.polaris.persistence.nosql.authz.impl;
-import static org.apache.polaris.persistence.nosql.authz.impl.JacksonPrivilegesModule.currentPrivileges;
+import static java.util.Objects.requireNonNull;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
@@ -26,30 +26,39 @@ import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonMappingException;
import java.io.IOException;
+import java.util.function.Supplier;
import org.apache.polaris.persistence.nosql.authz.api.PrivilegeSet;
+import org.apache.polaris.persistence.nosql.authz.api.Privileges;
class PrivilegeSetDeserializer extends JsonDeserializer<PrivilegeSet> {
+ private final Supplier<Privileges> privilegesResolver;
+
+ PrivilegeSetDeserializer(Supplier<Privileges> privilegesResolver) {
+ this.privilegesResolver =
+ requireNonNull(privilegesResolver, "privilegesResolver must not be null");
+ }
+
@Override
public PrivilegeSet deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
switch (p.currentToken()) {
case VALUE_NULL:
- return new PrivilegeSetImpl(currentPrivileges(), new byte[0]);
+ return new PrivilegeSetImpl(privilegesResolver.get(), new byte[0]);
case VALUE_STRING:
// Internal, storage serialization format.
var bytes = p.getBinaryValue();
- return new PrivilegeSetImpl(currentPrivileges(), bytes);
+ return new PrivilegeSetImpl(privilegesResolver.get(), bytes);
case START_ARRAY:
// External/REST serialization format using privilege names.
- var privileges = currentPrivileges();
+ var privileges = privilegesResolver.get();
var builder = PrivilegeSetImpl.builder(privileges);
for (var t = p.nextToken(); ; t = p.nextToken()) {
- // Note: switch(t) lets checkstyle fail
- if (t == JsonToken.VALUE_STRING) {
- builder.addPrivilege(privileges.byName(p.getText()));
- }
if (t == JsonToken.END_ARRAY) {
break;
}
+ if (t != JsonToken.VALUE_STRING) {
+ throw new JsonMappingException(p, "Unexpected JSON token " + t + " in privilege array");
+ }
+ builder.addPrivilege(privileges.byName(p.getText()));
}
return builder.build();
default:
diff --git a/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/PrivilegeSetSerializer.java b/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/PrivilegeSetSerializer.java
index 1e7d5b65aa..3aa8227f1d 100644
--- a/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/PrivilegeSetSerializer.java
+++ b/persistence/nosql/authz/impl/src/main/java/org/apache/polaris/persistence/nosql/authz/impl/PrivilegeSetSerializer.java
@@ -18,15 +18,24 @@
*/
package org.apache.polaris.persistence.nosql.authz.impl;
-import static org.apache.polaris.persistence.nosql.authz.impl.JacksonPrivilegesModule.currentPrivileges;
+import static java.util.Objects.requireNonNull;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.SerializerProvider;
import java.io.IOException;
+import java.util.function.Supplier;
import org.apache.polaris.persistence.nosql.authz.api.PrivilegeSet;
+import org.apache.polaris.persistence.nosql.authz.api.Privileges;
public class PrivilegeSetSerializer extends JsonSerializer<PrivilegeSet> {
+ private final Supplier<Privileges> privilegesResolver;
+
+ PrivilegeSetSerializer(Supplier<Privileges> privilegesResolver) {
+ this.privilegesResolver =
+ requireNonNull(privilegesResolver, "privilegesResolver must not be null");
+ }
+
@Override
public void serialize(PrivilegeSet value, JsonGenerator gen, SerializerProvider serializers)
throws IOException {
@@ -45,7 +54,7 @@ public class PrivilegeSetSerializer extends JsonSerializer<PrivilegeSet> {
// Otherwise, for external/REST, use the privilege names from the "global" set of
// privileges.
gen.writeStartArray();
- var collapsed = value.collapseComposites(currentPrivileges());
+ var collapsed = value.collapseComposites(privilegesResolver.get());
for (var privilege : collapsed) {
gen.writeString(privilege.name());
}
diff --git a/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestAclEntryImpl.java b/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestAclEntryImpl.java
index 270409b4a0..09525d481a 100644
--- a/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestAclEntryImpl.java
+++ b/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestAclEntryImpl.java
@@ -43,15 +43,14 @@ public class TestAclEntryImpl {
@BeforeAll
static void setUp() {
+ privileges =
+ new PrivilegesImpl(Stream.of(new PrivilegesTestProvider()), new PrivilegesTestRepository());
mapper =
JsonMapper.builder()
- .findAndAddModules()
+ .addModule(new JacksonPrivilegesModule(() -> privileges))
.disable(FAIL_ON_UNKNOWN_PROPERTIES)
.enable(DEFAULT_VIEW_INCLUSION)
.build();
- privileges =
- new PrivilegesImpl(Stream.of(new PrivilegesTestProvider()), new PrivilegesTestRepository());
- JacksonPrivilegesModule.CDIResolver.setResolver(x -> privileges);
}
@ParameterizedTest
diff --git a/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestAclImpl.java b/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestAclImpl.java
index ff29dfbd73..c3d28dd811 100644
--- a/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestAclImpl.java
+++ b/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestAclImpl.java
@@ -42,10 +42,9 @@ public class TestAclImpl {
@BeforeAll
static void setUp() {
- mapper = JsonMapper.builder().findAndAddModules().build();
privileges =
new PrivilegesImpl(Stream.of(new PrivilegesTestProvider()), new PrivilegesTestRepository());
- JacksonPrivilegesModule.CDIResolver.setResolver(x -> privileges);
+ mapper = JsonMapper.builder().addModule(new JacksonPrivilegesModule(() -> privileges)).build();
}
@ParameterizedTest
diff --git a/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestPrivilegeCheckImpl.java b/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestPrivilegeCheckImpl.java
index 47084d0593..759c8e1af5 100644
--- a/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestPrivilegeCheckImpl.java
+++ b/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestPrivilegeCheckImpl.java
@@ -51,7 +51,6 @@ public class TestPrivilegeCheckImpl {
static void setUp() {
privileges =
new PrivilegesImpl(Stream.of(new PrivilegesTestProvider()), new PrivilegesTestRepository());
- JacksonPrivilegesModule.CDIResolver.setResolver(x -> privileges);
}
static PrivilegeSet privilegeSet(Privilege... values) {
diff --git a/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestPrivilegeSetImpl.java b/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestPrivilegeSetImpl.java
index 90f43fe255..403212ed2a 100644
--- a/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestPrivilegeSetImpl.java
+++ b/persistence/nosql/authz/impl/src/test/java/org/apache/polaris/persistence/nosql/authz/impl/TestPrivilegeSetImpl.java
@@ -23,6 +23,7 @@ import static com.fasterxml.jackson.databind.MapperFeature.DEFAULT_VIEW_INCLUSIO
import static java.util.Collections.singleton;
import static org.junit.jupiter.params.provider.Arguments.arguments;
+import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.json.JsonMapper;
@@ -38,6 +39,7 @@ import org.assertj.core.api.SoftAssertions;
import org.assertj.core.api.junit.jupiter.InjectSoftAssertions;
import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension;
import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
@@ -57,15 +59,14 @@ public class TestPrivilegeSetImpl {
@BeforeAll
static void setUp() {
+ privileges =
+ new PrivilegesImpl(Stream.of(new PrivilegesTestProvider()), new PrivilegesTestRepository());
mapper =
JsonMapper.builder()
- .findAndAddModules()
+ .addModule(new JacksonPrivilegesModule(() -> privileges))
.disable(FAIL_ON_UNKNOWN_PROPERTIES)
.enable(DEFAULT_VIEW_INCLUSION)
.build();
- privileges =
- new PrivilegesImpl(Stream.of(new PrivilegesTestProvider()), new PrivilegesTestRepository());
- JacksonPrivilegesModule.CDIResolver.setResolver(x -> privileges);
}
@SuppressWarnings("RedundantCollectionOperation")
@@ -168,6 +169,13 @@ public class TestPrivilegeSetImpl {
Stream.of(privileges.newPrivilegesSetBuilder().addPrivileges(privileges.all()).build()));
}
+ @Test
+ public void nameDeserializationRejectsNonStringArrayMembers() {
+ soft.assertThatThrownBy(() -> mapper.readValue("[\"zero\",1]", PrivilegeSet.class))
+ .isInstanceOf(JsonMappingException.class)
+ .hasMessageContaining("Unexpected JSON token VALUE_NUMBER_INT in privilege array");
+ }
+
@ParameterizedTest
@MethodSource
public void compositeByNameSerialization(