Skip to content

Commit 342b6fd

Browse files
committed
Revert "Includes the following changes: 1. Fixing a few types 2. removing the redundant usage of object mapper customizer in AwsCloudWatchEventListener and related changes."
This reverts commit 99d8a5d.
1 parent 99d8a5d commit 342b6fd

File tree

5 files changed

+29
-79
lines changed

5 files changed

+29
-79
lines changed

runtime/defaults/src/main/resources/application.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ polaris.event-listener.type=no-op
145145
# polaris.event-listener.aws-cloudwatch.log-stream=polaris-cloudwatch-default-stream
146146
# polaris.event-listener.aws-cloudwatch.region=us-east-1
147147
# polaris.event-listener.aws-cloudwatch.synchronous-mode=false
148-
# polaris.event-listener.aws-cloudwatch.event-types= // the absence of this property would result in processing all Polaris event types.
148+
# polaris.event-listener.aws-cloudwatch.event-types= // the absence of this property would result if processing all Polaris event types.
149149

150150
polaris.log.request-id-header-name=Polaris-Request-Id
151151
# polaris.log.mdc.aid=polaris

runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventType.java

Lines changed: 0 additions & 63 deletions
This file was deleted.

runtime/service/src/main/java/org/apache/polaris/service/events/listeners/AllEventsForwardingListener.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
import org.apache.polaris.service.events.PrincipalsServiceEvents;
3232

3333
/**
34-
* Base class for event listeners that wish to generically forward all {@link PolarisEvent
34+
* Base class for event listeners that with to generically forward all {@link PolarisEvent
3535
* PolarisEvents} to an external sink.
3636
*
3737
* <p>This design follows the Template Method pattern, centralizing shared control flow in the base

runtime/service/src/main/java/org/apache/polaris/service/events/listeners/aws/cloudwatch/AwsCloudWatchEventListener.java

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.util.function.Supplier;
4040
import org.apache.polaris.core.auth.PolarisPrincipal;
4141
import org.apache.polaris.core.context.CallContext;
42+
import org.apache.polaris.service.config.PolarisIcebergObjectMapperCustomizer;
4243
import org.apache.polaris.service.events.PolarisEvent;
4344
import org.apache.polaris.service.events.listeners.AllEventsForwardingListener;
4445
import org.slf4j.Logger;
@@ -75,17 +76,22 @@ public class AwsCloudWatchEventListener extends AllEventsForwardingListener {
7576

7677
@Inject
7778
public AwsCloudWatchEventListener(
78-
AwsCloudWatchConfiguration config, Clock clock, ObjectMapper mapper) {
79+
AwsCloudWatchConfiguration config,
80+
Clock clock,
81+
PolarisIcebergObjectMapperCustomizer customizer,
82+
ObjectMapper mapper) {
7983
this.logStream = config.awsCloudWatchLogStream();
8084
this.logGroup = config.awsCloudWatchLogGroup();
8185
this.region = Region.of(config.awsCloudWatchRegion());
8286
this.synchronousMode = config.synchronousMode();
8387
this.clock = clock;
8488
this.objectMapper = mapper;
85-
this.allowedEventTypes = config.eventTypes().orElse(Set.of());
89+
customizer.customize(this.objectMapper);
8690
this.listenToAllEvents =
87-
allowedEventTypes.isEmpty()
88-
|| allowedEventTypes.stream().anyMatch(c -> c == PolarisEvent.class);
91+
config.eventTypes().isEmpty()
92+
|| config.eventTypes().map(Set::isEmpty).orElse(true)
93+
|| config.eventTypes().get().stream().anyMatch(e -> e == PolarisEvent.class);
94+
this.allowedEventTypes = listenToAllEvents ? Set.of() : Set.copyOf(config.eventTypes().get());
8995
}
9096

9197
@Override
@@ -112,6 +118,14 @@ void start() {
112118
ensureLogGroupAndStream();
113119
}
114120

121+
@PostConstruct
122+
void verifyMapper() {
123+
LOGGER.info(
124+
"ObjectMapper hash={}, mixins={}",
125+
System.identityHashCode(objectMapper),
126+
objectMapper.mixInCount());
127+
}
128+
115129
protected CloudWatchLogsAsyncClient createCloudWatchAsyncClient() {
116130
return CloudWatchLogsAsyncClient.builder().region(region).build();
117131
}

runtime/service/src/test/java/org/apache/polaris/service/events/listeners/aws/cloudwatch/AwsCloudWatchEventListenerTest.java

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import java.math.BigInteger;
3232
import java.time.Clock;
3333
import java.time.Duration;
34-
import java.util.Optional;
3534
import java.util.Set;
3635
import java.util.concurrent.ExecutorService;
3736
import java.util.concurrent.Executors;
@@ -93,20 +92,22 @@ class AwsCloudWatchEventListenerTest {
9392

9493
private ExecutorService executorService;
9594
private AutoCloseable mockitoContext;
96-
private static final ObjectMapper objectMapper = new ObjectMapper();
95+
private ObjectMapper objectMapper;
9796

9897
@BeforeEach
9998
void setUp() {
10099
mockitoContext = MockitoAnnotations.openMocks(this);
101100
executorService = Executors.newSingleThreadExecutor();
102-
customizer.customize(objectMapper);
101+
102+
// Build the test mapper and apply the same customizations Quarkus would
103+
objectMapper = new ObjectMapper();
103104

104105
// Configure the mocks
105106
when(config.awsCloudWatchLogGroup()).thenReturn(LOG_GROUP);
106107
when(config.awsCloudWatchLogStream()).thenReturn(LOG_STREAM);
107108
when(config.awsCloudWatchRegion()).thenReturn("us-east-1");
108109
when(config.synchronousMode()).thenReturn(false); // default async
109-
when(config.eventTypes()).thenReturn(Optional.empty()); // handle all events
110+
when(config.eventTypes()).thenReturn(java.util.Optional.empty()); // handle all events
110111
}
111112

112113
@AfterEach
@@ -140,7 +141,7 @@ private CloudWatchLogsAsyncClient createCloudWatchAsyncClient() {
140141

141142
private AwsCloudWatchEventListener createListener(CloudWatchLogsAsyncClient client) {
142143
AwsCloudWatchEventListener listener =
143-
new AwsCloudWatchEventListener(config, clock, objectMapper) {
144+
new AwsCloudWatchEventListener(config, clock, customizer, objectMapper) {
144145
@Override
145146
protected CloudWatchLogsAsyncClient createCloudWatchAsyncClient() {
146147
return client;
@@ -328,9 +329,7 @@ void shouldSendEventInSynchronousMode() {
328329
void ensureObjectMapperCustomizerIsApplied() {
329330

330331
AwsCloudWatchEventListener listener =
331-
new AwsCloudWatchEventListener(config, clock, objectMapper);
332-
333-
assertThat(listener.objectMapper).isSameAs(objectMapper);
332+
new AwsCloudWatchEventListener(config, clock, customizer, objectMapper);
334333

335334
assertThat(listener.objectMapper.getPropertyNamingStrategy())
336335
.isInstanceOf(PropertyNamingStrategies.KebabCaseStrategy.class);
@@ -353,10 +352,10 @@ void ensureObjectMapperCustomizerIsApplied() {
353352
@Test
354353
void shouldListenToAllEventTypesWhenConfigNotProvided() {
355354
// given: config.eventTypes() is empty → listen to all events
356-
when(config.eventTypes()).thenReturn(Optional.empty());
355+
when(config.eventTypes()).thenReturn(java.util.Optional.empty());
357356

358357
AwsCloudWatchEventListener listener =
359-
new AwsCloudWatchEventListener(config, clock, objectMapper);
358+
new AwsCloudWatchEventListener(config, clock, customizer, objectMapper);
360359

361360
// This is any random PolarisEvent — if the listener listens to all types,
362361
// shouldHandle(event) should return true

0 commit comments

Comments
 (0)