You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "HonahX (via GitHub)" <gi...@apache.org> on 2026/05/15 20:42:16 UTC
[PR] Handle failed identity resolution gracefully in PolarisEventMetadataFactory [polaris]
HonahX opened a new pull request, #4459:
URL: https://github.com/apache/polaris/pull/4459
`PolarisEventMetadataFactory.create()` calls `getDeferredIdentity().subscribeAsCompletionStage().getNow(null)` to resolve the current user for event metadata. When called before authentication has completed
(e.g., in pre-authentication filters), this can trigger the auth pipeline, which may fail with `CompletionException` wrapping an `AuthenticationFailedException`.
Previously, this exception would propagate to the caller. For example, when the rate limiter filter emits an event before rejecting a request, the leaked
Completion exception gets mapped to 500 instead of the intended 429.
In general, since `user` itself is an optional field, proceed the event dispatching with a null user instead of failing directly could increase the stability and while we can still see/notice the actual error through debug log.
## Changes
- `PolarisEventMetadataFactory.java`: Refactor `getUser()` to use new `getSecurityIdentity()` helper with `CompletionException` handling and debug logging
- `PolarisEventMetadataFactoryTest.java`: New unit test covering resolved identity, anonymous identity, not-yet-resolved identity, and failed deferred resolution
## Test plan
- [x] New unit test `PolarisEventMetadataFactoryTest` passes (4 cases)
<!--
๐ Describe what changes you're proposing, especially breaking or user-facing changes.
๐ See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
-->
## Checklist
- [ ] ๐ก๏ธ Don't disclose security issues! (contact security@apache.org)
- [ ] ๐ Clearly explained why the changes are needed, or linked related issues: Fixes #
- [ ] ๐งช Added/updated tests with good coverage, or manually tested (and explained how)
- [ ] ๐ก Added comments for complex logic
- [ ] ๐งพ Updated `CHANGELOG.md` (if needed)
- [ ] ๐ Updated documentation in `site/content/in-dev/unreleased` (if needed)
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle failed identity resolution gracefully in PolarisEventMetadataFactory [polaris]
Posted by "HonahX (via GitHub)" <gi...@apache.org>.
HonahX commented on PR #4459:
URL: https://github.com/apache/polaris/pull/4459#issuecomment-4502563715
@snazy Thanks for the review! I've updated to suppress `AuthenticationFailedException` only and re-throw CompletionException otherwise, and also added a test. Please take a look : )
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle failed identity resolution gracefully in PolarisEventMetadataFactory [polaris]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on code in PR #4459:
URL: https://github.com/apache/polaris/pull/4459#discussion_r3257175355
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventMetadataFactoryTest.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.events;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
+
+import io.quarkus.security.identity.CurrentIdentityAssociation;
+import io.quarkus.security.identity.SecurityIdentity;
+import io.smallrye.mutiny.Uni;
+import jakarta.enterprise.inject.Instance;
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
+import org.apache.polaris.core.auth.ImmutablePolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.context.RealmContext;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class PolarisEventMetadataFactoryTest {
+
+ private static final Instant FIXED_INSTANT = Instant.parse("2025-01-01T00:00:00Z");
+ private static final String TEST_REALM = "test-realm";
+
+ @Mock private CurrentIdentityAssociation currentIdentityAssociation;
+ @Mock private Instance<RealmContext> realmContext;
+ @Mock private RealmContext realmContextInstance;
+ @Mock private SecurityIdentity securityIdentity;
+
+ @InjectMocks private PolarisEventMetadataFactory factory;
+
+ @BeforeEach
+ void setUp() {
+ factory.clock = Clock.fixed(FIXED_INSTANT, ZoneId.of("UTC"));
+ when(realmContext.isResolvable()).thenReturn(true);
+ when(realmContext.get()).thenReturn(realmContextInstance);
+ when(realmContextInstance.getRealmIdentifier()).thenReturn(TEST_REALM);
+ }
+
+ @Test
+ void createReturnsMetadataWithUserWhenIdentityResolved() {
+ PolarisPrincipal principal = ImmutablePolarisPrincipal.builder().name("test-user").build();
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(securityIdentity));
+ when(securityIdentity.isAnonymous()).thenReturn(false);
+ when(securityIdentity.getPrincipal(PolarisPrincipal.class)).thenReturn(principal);
+
+ PolarisEventMetadata metadata = factory.create();
+
+ assertThat(metadata.realmId()).isEqualTo(TEST_REALM);
+ assertThat(metadata.timestamp()).isEqualTo(FIXED_INSTANT);
+ assertThat(metadata.user()).contains(principal);
+ }
+
+ @Test
+ void createReturnsEmptyUserWhenIdentityIsAnonymous() {
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(securityIdentity));
+ when(securityIdentity.isAnonymous()).thenReturn(true);
+
+ PolarisEventMetadata metadata = factory.create();
+
+ assertThat(metadata.user()).isEmpty();
Review Comment:
nit: Can we make sure that the `realmId` and `timestamp` assertions are added to this function and the one below as well?
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventMetadataFactoryTest.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.events;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
+
+import io.quarkus.security.identity.CurrentIdentityAssociation;
+import io.quarkus.security.identity.SecurityIdentity;
+import io.smallrye.mutiny.Uni;
+import jakarta.enterprise.inject.Instance;
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
+import org.apache.polaris.core.auth.ImmutablePolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.context.RealmContext;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class PolarisEventMetadataFactoryTest {
+
+ private static final Instant FIXED_INSTANT = Instant.parse("2025-01-01T00:00:00Z");
+ private static final String TEST_REALM = "test-realm";
+
+ @Mock private CurrentIdentityAssociation currentIdentityAssociation;
+ @Mock private Instance<RealmContext> realmContext;
+ @Mock private RealmContext realmContextInstance;
+ @Mock private SecurityIdentity securityIdentity;
+
+ @InjectMocks private PolarisEventMetadataFactory factory;
+
+ @BeforeEach
+ void setUp() {
+ factory.clock = Clock.fixed(FIXED_INSTANT, ZoneId.of("UTC"));
+ when(realmContext.isResolvable()).thenReturn(true);
+ when(realmContext.get()).thenReturn(realmContextInstance);
+ when(realmContextInstance.getRealmIdentifier()).thenReturn(TEST_REALM);
+ }
+
+ @Test
+ void createReturnsMetadataWithUserWhenIdentityResolved() {
+ PolarisPrincipal principal = ImmutablePolarisPrincipal.builder().name("test-user").build();
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(securityIdentity));
+ when(securityIdentity.isAnonymous()).thenReturn(false);
+ when(securityIdentity.getPrincipal(PolarisPrincipal.class)).thenReturn(principal);
+
+ PolarisEventMetadata metadata = factory.create();
+
+ assertThat(metadata.realmId()).isEqualTo(TEST_REALM);
+ assertThat(metadata.timestamp()).isEqualTo(FIXED_INSTANT);
+ assertThat(metadata.user()).contains(principal);
+ }
+
+ @Test
+ void createReturnsEmptyUserWhenIdentityIsAnonymous() {
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(securityIdentity));
+ when(securityIdentity.isAnonymous()).thenReturn(true);
+
+ PolarisEventMetadata metadata = factory.create();
+
+ assertThat(metadata.user()).isEmpty();
+ }
+
+ @Test
+ void createReturnsEmptyUserWhenIdentityNotYetResolved() {
+ // Simulates getNow(null) returning null because the future hasn't completed
Review Comment:
Also, I think this is true, but would be nice if someone can confirm that this behavior hasn't changed as part of this code change.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle failed identity resolution gracefully in PolarisEventMetadataFactory [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4459:
URL: https://github.com/apache/polaris/pull/4459#discussion_r3277231994
##########
runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventMetadataFactory.java:
##########
@@ -92,11 +97,35 @@ private String getRealmId() {
* taken place, e.g. in pre-authentication filters.
*/
private Optional<PolarisPrincipal> getUser() {
- SecurityIdentity identity =
- currentIdentityAssociation.getDeferredIdentity().subscribeAsCompletionStage().getNow(null);
- return identity == null || identity.isAnonymous()
- ? Optional.empty()
- : Optional.of(identity.getPrincipal(PolarisPrincipal.class));
+ return getSecurityIdentity()
+ .filter(identity -> !identity.isAnonymous())
+ .map(identity -> identity.getPrincipal(PolarisPrincipal.class));
+ }
+
+ /**
+ * Resolves the current security identity if available.
+ *
+ * <p>Returns {@link Optional#empty()} if the identity has not been resolved yet, or if the
+ * deferred identity resolution failed with an {@link AuthenticationFailedException} (e.g. on
+ * unauthenticated endpoints where the auth pipeline throws when triggered).
+ *
+ * <p>Other exceptions (e.g. {@link org.apache.iceberg.exceptions.ServiceFailureException}) are
+ * not suppressed and will propagate to the caller.
+ */
+ private Optional<SecurityIdentity> getSecurityIdentity() {
+ try {
+ return Optional.ofNullable(
+ currentIdentityAssociation
+ .getDeferredIdentity()
+ .subscribeAsCompletionStage()
+ .getNow(null));
+ } catch (CompletionException e) {
+ if (e.getCause() instanceof AuthenticationFailedException) {
Review Comment:
How about a holder approach?
We could start with `PolarisPrincipalHolder` but alter it to always receive the `PolarisPrincipal` after authentication completes... that is in `AuthenticatingAugmentor`.
The normal producer method would assert that the principal is set.
The events factory could deal with `PolarisPrincipalHolder` and get an optional `PolarisPrincipal` value via another getter.
This way most of the processing logic does not have to deal with async aspects.
WDYT?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle failed identity resolution gracefully in PolarisEventMetadataFactory [polaris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4459: Handle failed identity resolution gracefully in PolarisEventMetadataFactory
URL: https://github.com/apache/polaris/pull/4459
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle failed identity resolution gracefully in PolarisEventMetadataFactory [polaris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4459:
URL: https://github.com/apache/polaris/pull/4459#issuecomment-4756036888
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle failed identity resolution gracefully in PolarisEventMetadataFactory [polaris]
Posted by "HonahX (via GitHub)" <gi...@apache.org>.
HonahX commented on code in PR #4459:
URL: https://github.com/apache/polaris/pull/4459#discussion_r3277094584
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventMetadataFactoryTest.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.events;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
+
+import io.quarkus.security.identity.CurrentIdentityAssociation;
+import io.quarkus.security.identity.SecurityIdentity;
+import io.smallrye.mutiny.Uni;
+import jakarta.enterprise.inject.Instance;
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
+import org.apache.polaris.core.auth.ImmutablePolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.context.RealmContext;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class PolarisEventMetadataFactoryTest {
+
+ private static final Instant FIXED_INSTANT = Instant.parse("2025-01-01T00:00:00Z");
+ private static final String TEST_REALM = "test-realm";
+
+ @Mock private CurrentIdentityAssociation currentIdentityAssociation;
+ @Mock private Instance<RealmContext> realmContext;
+ @Mock private RealmContext realmContextInstance;
+ @Mock private SecurityIdentity securityIdentity;
+
+ @InjectMocks private PolarisEventMetadataFactory factory;
+
+ @BeforeEach
+ void setUp() {
+ factory.clock = Clock.fixed(FIXED_INSTANT, ZoneId.of("UTC"));
+ when(realmContext.isResolvable()).thenReturn(true);
+ when(realmContext.get()).thenReturn(realmContextInstance);
+ when(realmContextInstance.getRealmIdentifier()).thenReturn(TEST_REALM);
+ }
+
+ @Test
+ void createReturnsMetadataWithUserWhenIdentityResolved() {
+ PolarisPrincipal principal = ImmutablePolarisPrincipal.builder().name("test-user").build();
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(securityIdentity));
+ when(securityIdentity.isAnonymous()).thenReturn(false);
+ when(securityIdentity.getPrincipal(PolarisPrincipal.class)).thenReturn(principal);
+
+ PolarisEventMetadata metadata = factory.create();
+
+ assertThat(metadata.realmId()).isEqualTo(TEST_REALM);
+ assertThat(metadata.timestamp()).isEqualTo(FIXED_INSTANT);
+ assertThat(metadata.user()).contains(principal);
+ }
+
+ @Test
+ void createReturnsEmptyUserWhenIdentityIsAnonymous() {
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(securityIdentity));
+ when(securityIdentity.isAnonymous()).thenReturn(true);
+
+ PolarisEventMetadata metadata = factory.create();
+
+ assertThat(metadata.user()).isEmpty();
Review Comment:
Done!
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle failed identity resolution gracefully in PolarisEventMetadataFactory [polaris]
Posted by "HonahX (via GitHub)" <gi...@apache.org>.
HonahX commented on code in PR #4459:
URL: https://github.com/apache/polaris/pull/4459#discussion_r3277104806
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventMetadataFactoryTest.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.events;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
+
+import io.quarkus.security.identity.CurrentIdentityAssociation;
+import io.quarkus.security.identity.SecurityIdentity;
+import io.smallrye.mutiny.Uni;
+import jakarta.enterprise.inject.Instance;
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
+import org.apache.polaris.core.auth.ImmutablePolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.context.RealmContext;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class PolarisEventMetadataFactoryTest {
+
+ private static final Instant FIXED_INSTANT = Instant.parse("2025-01-01T00:00:00Z");
+ private static final String TEST_REALM = "test-realm";
+
+ @Mock private CurrentIdentityAssociation currentIdentityAssociation;
+ @Mock private Instance<RealmContext> realmContext;
+ @Mock private RealmContext realmContextInstance;
+ @Mock private SecurityIdentity securityIdentity;
+
+ @InjectMocks private PolarisEventMetadataFactory factory;
+
+ @BeforeEach
+ void setUp() {
+ factory.clock = Clock.fixed(FIXED_INSTANT, ZoneId.of("UTC"));
+ when(realmContext.isResolvable()).thenReturn(true);
+ when(realmContext.get()).thenReturn(realmContextInstance);
+ when(realmContextInstance.getRealmIdentifier()).thenReturn(TEST_REALM);
+ }
+
+ @Test
+ void createReturnsMetadataWithUserWhenIdentityResolved() {
+ PolarisPrincipal principal = ImmutablePolarisPrincipal.builder().name("test-user").build();
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(securityIdentity));
+ when(securityIdentity.isAnonymous()).thenReturn(false);
+ when(securityIdentity.getPrincipal(PolarisPrincipal.class)).thenReturn(principal);
+
+ PolarisEventMetadata metadata = factory.create();
+
+ assertThat(metadata.realmId()).isEqualTo(TEST_REALM);
+ assertThat(metadata.timestamp()).isEqualTo(FIXED_INSTANT);
+ assertThat(metadata.user()).contains(principal);
+ }
+
+ @Test
+ void createReturnsEmptyUserWhenIdentityIsAnonymous() {
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(securityIdentity));
+ when(securityIdentity.isAnonymous()).thenReturn(true);
+
+ PolarisEventMetadata metadata = factory.create();
+
+ assertThat(metadata.user()).isEmpty();
+ }
+
+ @Test
+ void createReturnsEmptyUserWhenIdentityNotYetResolved() {
+ // Simulates getNow(null) returning null because the future hasn't completed
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(() -> null));
Review Comment:
Good catch!
```
Uni.createFrom().nothing()
```
should do the trick. Updated the test to use that!
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Handle failed identity resolution gracefully in PolarisEventMetadataFactory [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4459:
URL: https://github.com/apache/polaris/pull/4459#discussion_r3258097269
##########
runtime/service/src/test/java/org/apache/polaris/service/events/PolarisEventMetadataFactoryTest.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.events;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
+
+import io.quarkus.security.identity.CurrentIdentityAssociation;
+import io.quarkus.security.identity.SecurityIdentity;
+import io.smallrye.mutiny.Uni;
+import jakarta.enterprise.inject.Instance;
+import java.time.Clock;
+import java.time.Instant;
+import java.time.ZoneId;
+import org.apache.polaris.core.auth.ImmutablePolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisPrincipal;
+import org.apache.polaris.core.context.RealmContext;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class PolarisEventMetadataFactoryTest {
+
+ private static final Instant FIXED_INSTANT = Instant.parse("2025-01-01T00:00:00Z");
+ private static final String TEST_REALM = "test-realm";
+
+ @Mock private CurrentIdentityAssociation currentIdentityAssociation;
+ @Mock private Instance<RealmContext> realmContext;
+ @Mock private RealmContext realmContextInstance;
+ @Mock private SecurityIdentity securityIdentity;
+
+ @InjectMocks private PolarisEventMetadataFactory factory;
+
+ @BeforeEach
+ void setUp() {
+ factory.clock = Clock.fixed(FIXED_INSTANT, ZoneId.of("UTC"));
+ when(realmContext.isResolvable()).thenReturn(true);
+ when(realmContext.get()).thenReturn(realmContextInstance);
+ when(realmContextInstance.getRealmIdentifier()).thenReturn(TEST_REALM);
+ }
+
+ @Test
+ void createReturnsMetadataWithUserWhenIdentityResolved() {
+ PolarisPrincipal principal = ImmutablePolarisPrincipal.builder().name("test-user").build();
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(securityIdentity));
+ when(securityIdentity.isAnonymous()).thenReturn(false);
+ when(securityIdentity.getPrincipal(PolarisPrincipal.class)).thenReturn(principal);
+
+ PolarisEventMetadata metadata = factory.create();
+
+ assertThat(metadata.realmId()).isEqualTo(TEST_REALM);
+ assertThat(metadata.timestamp()).isEqualTo(FIXED_INSTANT);
+ assertThat(metadata.user()).contains(principal);
+ }
+
+ @Test
+ void createReturnsEmptyUserWhenIdentityIsAnonymous() {
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(securityIdentity));
+ when(securityIdentity.isAnonymous()).thenReturn(true);
+
+ PolarisEventMetadata metadata = factory.create();
+
+ assertThat(metadata.user()).isEmpty();
+ }
+
+ @Test
+ void createReturnsEmptyUserWhenIdentityNotYetResolved() {
+ // Simulates getNow(null) returning null because the future hasn't completed
+ when(currentIdentityAssociation.getDeferredIdentity())
+ .thenReturn(Uni.createFrom().item(() -> null));
Review Comment:
This is not accurately testing the scenario of a future that isn't completed. You should return instead a `Uni` that never completes.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org