Skip to content

Commit fa9767b

Browse files
msalinas-setombentley
msalinas-se
and
tombentley
authored
refactor: Fix ErrorProne StaticAssignmentOfThrowable warnings (kroxylicious#3742)
* refactor: Fix ErrorProne StaticAssignmentOfThrowable warnings Replaces static Throwable fields with factory methods so each exception is created at the actual throw site, giving meaningful stack traces. Promotes StaticAssignmentOfThrowable to ERROR in pom.xml. * refactor: rename FILTER_RUNTIME_EXCEPTION to camelCase (instance field) Instance fields should not follow UPPER_CASE naming convention reserved for static final constants. Closes kroxylicious#3741 Signed-off-by: msalinas-se <116117013+msalinas-se@users.noreply.github.com> Co-authored-by: Tom Bentley <tombentley@users.noreply.github.com> Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 60261b0 commit fa9767b

6 files changed

Lines changed: 24 additions & 18 deletions

File tree

‎kroxylicious-filter-test-support/src/main/java/io/kroxylicious/test/context/MockFilterContext.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,10 @@ public class MockFilterContext implements FilterContext {
6161
public static final String DEFAULT_SNI_HOSTNAME = "sniHostname";
6262
public static final String DEFAULT_VIRTUAL_CLUSTER_NAME = "virtualCluster";
6363
public static final Subject DEFAULT_AUTHENTICATED_SUBJECT = Subject.anonymous();
64-
private static final TopicNameMappingException NOT_CONFIGURED_EXCEPTION = new TopicNameMappingException(Errors.UNKNOWN_SERVER_ERROR,
65-
"no mapping for topicId configured in MockFilterContext");
64+
65+
private static TopicNameMappingException notConfiguredException() {
66+
return new TopicNameMappingException(Errors.UNKNOWN_SERVER_ERROR, "no mapping for topicId configured in MockFilterContext");
67+
}
6668

6769
private final ApiMessage header;
6870
private final ApiMessage message;
@@ -341,7 +343,7 @@ public <M extends ApiMessage> CompletionStage<M> sendRequest(RequestHeaderData h
341343
public CompletionStage<TopicNameMapping> topicNames(Collection<Uuid> topicIds) {
342344
Map<Boolean, List<Uuid>> partitioned = topicIds.stream().collect(Collectors.partitioningBy(topicNames::containsKey));
343345
Map<Uuid, TopicNameMappingException> errors = partitioned.get(false).stream()
344-
.collect(Collectors.toMap(topicId -> topicId, topicId -> topicNameFailures.getOrDefault(topicId, NOT_CONFIGURED_EXCEPTION)));
346+
.collect(Collectors.toMap(topicId -> topicId, topicId -> topicNameFailures.getOrDefault(topicId, notConfiguredException())));
345347
Map<Uuid, String> names = partitioned.get(true).stream().collect(Collectors.toMap(topicId -> topicId, topicNames::get));
346348
return CompletableFuture.completedStage(
347349
new MockTopicNameMapping(!errors.isEmpty(), names, errors));

‎kroxylicious-filters/kroxylicious-oauthbearer-validation/src/main/java/io/kroxylicious/filter/oauthbearer/OauthBearerValidationFilter.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ public class OauthBearerValidationFilter
6767
SaslAuthenticateResponseFilter {
6868

6969
private static final Logger LOGGER = LoggerFactory.getLogger(OauthBearerValidationFilter.class);
70-
private static final SaslAuthenticationException INVALID_SASL_STATE_EXCEPTION = new SaslAuthenticationException("invalid sasl state");
7170
private final ScheduledExecutorService executorService;
7271
private final BackoffStrategy strategy;
7372
private final LoadingCache<String, AtomicInteger> rateLimiter;
@@ -137,7 +136,7 @@ public CompletionStage<RequestFilterResult> onSaslAuthenticateRequest(short apiV
137136
LOGGER.atDebug()
138137
.addKeyValue("saslState", "ILLEGAL_SASL_STATE")
139138
.log("SASL invalid state");
140-
notifyThrowable(context, INVALID_SASL_STATE_EXCEPTION);
139+
notifyThrowable(context, invalidSaslStateException());
141140
return context.requestFilterResultBuilder().shortCircuitResponse(failedResponse).withCloseConnection().completed();
142141
}
143142
this.saslServer = null;
@@ -178,6 +177,10 @@ public CompletionStage<RequestFilterResult> onSaslAuthenticateRequest(short apiV
178177
}
179178
}
180179

180+
private static SaslAuthenticationException invalidSaslStateException() {
181+
return new SaslAuthenticationException("invalid sasl state");
182+
}
183+
181184
private void notifyThrowable(FilterContext context, Throwable e) {
182185
if (e instanceof Exception ex) {
183186
context.clientSaslAuthenticationFailure(OAUTHBEARER_MECHANISM, authorizationId, ex);

‎kroxylicious-integration-tests/src/test/java/io/kroxylicious/it/testplugins/ShortCircuitErrorResponse.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,28 +31,30 @@
3131
@Plugin(configType = ShortCircuitErrorResponse.Config.class)
3232
public class ShortCircuitErrorResponse implements FilterFactory<ShortCircuitErrorResponse.Config, ShortCircuitErrorResponse.ResponseMechanism> {
3333

34-
private static final UnknownServerException EXCEPTION = new UnknownServerException(ShortCircuitErrorResponse.class.getName() + ": responding error to all requests");
34+
private static UnknownServerException exception() {
35+
return new UnknownServerException(ShortCircuitErrorResponse.class.getName() + ": responding error to all requests");
36+
}
3537

3638
public enum ResponseMechanism implements RequestFilter {
3739
ERROR {
3840
@Override
3941
public CompletionStage<RequestFilterResult> onRequest(ApiKeys apiKey, short apiVersion, RequestHeaderData header, ApiMessage request, FilterContext context) {
4042
return context.requestFilterResultBuilder()
41-
.errorResponse(header, request, EXCEPTION)
43+
.errorResponse(header, request, exception())
4244
.completed();
4345
}
4446
},
4547
SHORTCIRCUIT_MESSAGE {
4648
@Override
4749
public CompletionStage<RequestFilterResult> onRequest(ApiKeys apiKey, short apiVersion, RequestHeaderData header, ApiMessage request, FilterContext context) {
48-
final AbstractResponse errorResponseMessage = KafkaProxyExceptionMapper.errorResponseForMessage(header, request, EXCEPTION);
50+
final AbstractResponse errorResponseMessage = KafkaProxyExceptionMapper.errorResponseForMessage(header, request, exception());
4951
return context.requestFilterResultBuilder().shortCircuitResponse(errorResponseMessage.data()).completed();
5052
}
5153
},
5254
SHORTCIRCUIT_MESSAGE_AND_HEADER {
5355
@Override
5456
public CompletionStage<RequestFilterResult> onRequest(ApiKeys apiKey, short apiVersion, RequestHeaderData header, ApiMessage request, FilterContext context) {
55-
final AbstractResponse errorResponseMessage = KafkaProxyExceptionMapper.errorResponseForMessage(header, request, EXCEPTION);
57+
final AbstractResponse errorResponseMessage = KafkaProxyExceptionMapper.errorResponseForMessage(header, request, exception());
5658
final ResponseHeaderData responseHeaders = new ResponseHeaderData();
5759
responseHeaders.setCorrelationId(header.correlationId());
5860
return context.requestFilterResultBuilder().shortCircuitResponse(responseHeaders, errorResponseMessage.data()).completed();

‎kroxylicious-runtime/src/test/java/io/kroxylicious/proxy/internal/KafkaProxyExceptionMapperTest.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,13 @@
3030

3131
class KafkaProxyExceptionMapperTest {
3232

33-
private static final SSLHandshakeException HANDSHAKE_EXCEPTION = new SSLHandshakeException("it went wrong");
34-
3533
@ParameterizedTest
3634
@MethodSource({ "decodedFrameSourceLatestVersion", "decodedFrameSourceOldestVersion" })
3735
void shouldGenerateErrorResponseApiKey(DecodedRequestFrame<?> request) {
3836
// Given
3937
// When
40-
final AbstractResponse response = KafkaProxyExceptionMapper.errorResponse(request, new BrokerNotAvailableException("handshake failure", HANDSHAKE_EXCEPTION));
38+
final AbstractResponse response = KafkaProxyExceptionMapper.errorResponse(request,
39+
new BrokerNotAvailableException("handshake failure", new SSLHandshakeException("it went wrong")));
4140

4241
// Then
4342
assertThat(response)

‎kroxylicious-runtime/src/test/java/io/kroxylicious/proxy/internal/filter/RequestFilterResultBuilderTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232

3333
class RequestFilterResultBuilderTest {
3434

35-
private static final UnknownServerException FILTER_RUNTIME_EXCEPTION = new UnknownServerException("Filter says yeah, nah!");
35+
private final UnknownServerException filterRuntimeException = new UnknownServerException("Filter says yeah, nah!");
3636
private final RequestFilterResultBuilder builder = new RequestFilterResultBuilderImpl();
3737

3838
@Test
@@ -152,7 +152,7 @@ void shouldBuildErrorResponse(RequestFactory.ApiMessageVersion versionedMessage)
152152
header.setCorrelationId(23456);
153153

154154
// When
155-
var future = builder.errorResponse(header, versionedMessage.apiMessage(), FILTER_RUNTIME_EXCEPTION).completed();
155+
var future = builder.errorResponse(header, versionedMessage.apiMessage(), filterRuntimeException).completed();
156156

157157
// Then
158158
assertThat(future)
@@ -176,7 +176,7 @@ void shouldBuildErrorResponseForIllegitimateLeaveGroupRequest() {
176176
header.setCorrelationId(23456);
177177

178178
// When
179-
var future = builder.errorResponse(header, new LeaveGroupRequestData(), FILTER_RUNTIME_EXCEPTION).completed();
179+
var future = builder.errorResponse(header, new LeaveGroupRequestData(), filterRuntimeException).completed();
180180

181181
// Then
182182
assertThat(future)
@@ -199,7 +199,7 @@ void shouldErrorResponseShouldNotInvokeRemainingFilterChain() {
199199
header.setCorrelationId(23456);
200200

201201
// When
202-
var future = builder.errorResponse(header, request.apiMessage(), FILTER_RUNTIME_EXCEPTION).completed();
202+
var future = builder.errorResponse(header, request.apiMessage(), filterRuntimeException).completed();
203203

204204
// Then
205205
assertThat(future)
@@ -220,7 +220,7 @@ void shouldErrorResponseShouldNotCloseConnection() {
220220
header.setCorrelationId(23456);
221221

222222
// When
223-
var future = builder.errorResponse(header, request.apiMessage(), FILTER_RUNTIME_EXCEPTION).completed();
223+
var future = builder.errorResponse(header, request.apiMessage(), filterRuntimeException).completed();
224224

225225
// Then
226226
assertThat(future)

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1351,7 +1351,7 @@
13511351
<arg>-XDcompilePolicy=simple</arg>
13521352
<arg>--should-stop=ifError=FLOW</arg>
13531353
<!-- The following <arg> cannot be split over multiple lines :-( -->
1354-
<arg>-Xplugin:ErrorProne -XepDisableWarningsInGeneratedCode -XepExcludedPaths:.*/target/generated-sources/.* -Xep:MissingOverride:ERROR -Xep:MutablePublicArray:ERROR -Xep:MathAbsoluteNegative:ERROR -Xep:EffectivelyPrivate:ERROR -Xep:ByteBufferBackingArray:ERROR -Xep:UnnecessaryMethodReference:ERROR -Xep:OperatorPrecedence:ERROR -Xep:ImmutableEnumChecker:ERROR -Xep:StatementSwitchToExpressionSwitch:ERROR</arg>
1354+
<arg>-Xplugin:ErrorProne -XepDisableWarningsInGeneratedCode -XepExcludedPaths:.*/target/generated-sources/.* -Xep:MissingOverride:ERROR -Xep:MutablePublicArray:ERROR -Xep:MathAbsoluteNegative:ERROR -Xep:EffectivelyPrivate:ERROR -Xep:ByteBufferBackingArray:ERROR -Xep:UnnecessaryMethodReference:ERROR -Xep:OperatorPrecedence:ERROR -Xep:ImmutableEnumChecker:ERROR -Xep:StatementSwitchToExpressionSwitch:ERROR -Xep:StaticAssignmentOfThrowable:ERROR</arg>
13551355
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
13561356
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
13571357
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>

0 commit comments

Comments
 (0)