GH-2284: Publish Auth Error Events #2317

danielgentes · 2022-06-23T10:56:53Z

Added Events for auth failed
and successful retry of auth
publish of these events
unit testing

* Added Events for auth failed and successful retry of auth * publish of these events * unit testing Closes spring-projects#2284

garyrussell

Thanks!

Just a few minor nits.

garyrussell · 2022-06-23T18:01:32Z

spring-kafka/src/main/java/org/springframework/kafka/event/ConsumerRetryAuthEvent.java

@@ -0,0 +1,77 @@
+/*
+ * Copyright 2018-2022 the original author or authors.


New files only need 2022.

garyrussell · 2022-06-23T18:01:51Z

spring-kafka/src/main/java/org/springframework/kafka/event/ConsumerRetryAuthEvent.java

+package org.springframework.kafka.event;
+
+/**
+ * An event published when authentication or authorization of a consumer fails and is being retried. Contains the triggering throwable.


Please wrap javadocs at 90 chars (code at 120).

garyrussell · 2022-06-23T18:02:44Z

spring-kafka/src/main/java/org/springframework/kafka/event/ConsumerRetryAuthEvent.java

+	}
+
+	/**
+	 * Return the reason why the consumer was stopped.


Looks like copy/paste.

Return the reason for the auth failure.

garyrussell · 2022-06-23T18:03:10Z

spring-kafka/src/main/java/org/springframework/kafka/event/ConsumerRetryAuthEvent.java

+	/**
+	 * Return the reason why the consumer was stopped.
+	 * @return the reason.
+	 * @since 2.5.8


Suggested change

* @since 2.5.8

* @since 2.8.8

garyrussell · 2022-06-23T18:03:28Z

...ng-kafka/src/main/java/org/springframework/kafka/event/ConsumerRetryAuthSuccessfulEvent.java

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2018-2022 the original author or authors.


Implement review suggestions: * new files only 2022 * copy paste error * javadoc wrapping

danielgentes · 2022-06-24T06:16:45Z

Hi @garyrussell,
thank you for the review! I made the mentioned changes in my latest commit.

artembilan

I think this also has to be mention in the docs: https://docs.spring.io/spring-kafka/docs/current/reference/html/#events.
See kafka.adoc file in the project and modify its Application Events section.

artembilan · 2022-06-24T14:32:56Z

spring-kafka/src/main/java/org/springframework/kafka/event/ConsumerRetryAuthEvent.java

+	/**
+	 * Return the reason for the auth failure.
+	 * @return the reason.
+	 * @since 2.8.8


This looks like copy/paste from some other class.
Please, remove all the @since in your new classes, but leave that one on the top class level.
You have just introduced the class in 3.0, so it is @since 3.0 with all of its APIs and so on: no reason to duplicate on every method.

artembilan · 2022-06-24T14:34:37Z

...ng-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java

 		}
 	}

+	private void publishRetryAuthEvent(Throwable throwable) {


Please, add your name to the @author list: you have made some changes to the class, so you deserve some credit.

artembilan · 2022-06-24T14:36:22Z

...fka/src/test/java/org/springframework/kafka/listener/KafkaMessageListenerContainerTests.java

 		given(cf.createConsumer(eq("grp"), eq("clientId"), isNull(), any())).willReturn(consumer);
 		given(cf.getConfigurationProperties()).willReturn(new HashMap<>());
 		CountDownLatch latch = new CountDownLatch(2);
+		CountDownLatch retryEvent = new CountDownLatch(2);


Same here: your name to the @author list.

@SInCE

Implement review suggestions: * remove @SInCE tags not at top level * event documentation * @author on changed classes * corrected javadoc

danielgentes · 2022-06-24T17:36:36Z

Hi @artembilan ,
thanks for the review! I updated the documentation and also made the other changes you suggested.

artembilan · 2022-06-24T17:56:04Z

@danielgentes ,

thank you for contribution; looking forward for more!

spring-projectsGH-2284:

7b75205

* Added Events for auth failed and successful retry of auth * publish of these events * unit testing Closes spring-projects#2284

garyrussell suggested changes Jun 23, 2022

View reviewed changes

spring-projectsGH-2284:

bebbf19

Implement review suggestions: * new files only 2022 * copy paste error * javadoc wrapping

artembilan requested changes Jun 24, 2022

View reviewed changes

spring-projectsGH-2284:

3c07c8a

Implement review suggestions: * remove @SInCE tags not at top level * event documentation * @author on changed classes * corrected javadoc

artembilan merged commit 6fbc6cb into spring-projects:main Jun 24, 2022

		@@ -0,0 +1,77 @@
		/*
		* Copyright 2018-2022 the original author or authors.

		@@ -0,0 +1,44 @@
		/*
		* Copyright 2018-2022 the original author or authors.

GH-2284: Publish Auth Error Events #2317

GH-2284: Publish Auth Error Events #2317

Uh oh!

Conversation

danielgentes commented Jun 23, 2022

Uh oh!

garyrussell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielgentes commented Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

artembilan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielgentes commented Jun 24, 2022

Uh oh!

artembilan commented Jun 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

danielgentes commented Jun 24, 2022 •

edited

Loading