GH-3135 : Adding new `ListenerContainerRegistry.getListenerContainersMatchingId(Predicate)` for easier usage. #3137

JooHyukKim · 2024-03-15T16:07:39Z

Resolves #3135

artembilan

Please, add your name to the @author list of all the affected classes.

Also consider to mention this new feature in the whats-new.adoc and in the kafkalistener-lifecycle.adoc.

Thanks

artembilan · 2024-03-15T16:48:26Z

spring-kafka/src/main/java/org/springframework/kafka/config/KafkaListenerEndpointRegistry.java

+			.stream()
+			.filter(entry -> idMatcher.test(entry.getKey()))
+			.map(Map.Entry::getValue)
+			.collect(Collectors.toList());


There is just toList() API.

spring-kafka/src/main/java/org/springframework/kafka/config/KafkaListenerEndpointRegistry.java

spring-kafka/src/main/java/org/springframework/kafka/listener/ListenerContainerRegistry.java

artembilan · 2024-03-15T16:52:46Z

...kafka/src/test/java/org/springframework/kafka/config/KafkaListenerEndpointRegistryTests.java

+        // When
+        Collection<MessageListenerContainer> listeners = registry.getListenerContainersMatching(s -> true);
+        // Then
+        assertThrows(UnsupportedOperationException.class, () -> listeners.add(mock(MessageListenerContainer.class)));


Why don't use assertThatExceptionOfType(UnsupportedOperationException.class) instead?

It also probably going to fail on Checkstyle since we have a limited set of allowed static method imports.

Right, looking now there is no pre-existing usage of assertThatExceptionOfType()

artembilan · 2024-03-15T16:53:05Z

...kafka/src/test/java/org/springframework/kafka/config/KafkaListenerEndpointRegistryTests.java

+		names.forEach(name -> registerListenerWithId(registry, name));
+	}
+
+	private void registerListenerWithId(KafkaListenerEndpointRegistry registry, String id) {


Looks like these both methods can be static.

JooHyukKim · 2024-03-16T01:16:54Z

I've updated the author list and added documentation for the new feature in whats-new.adoc and kafkalistener-lifecycle.adoc.

I noticed there seems to be limited documentation on KafkaListenerEndpointRegistry, particularly methods like getListenerContainers() and getListenerContainer(String). I can
address this in a future PR to enhance docs further. WDYT, @artembilan?

artembilan · 2024-03-16T02:05:39Z

Sure! You can add those couple sentences just in this PR. We have a release on Monday, so it will be reviewed immediately .
Thanks

JooHyukKim · 2024-03-16T06:55:10Z

Added some more doc as discussed 👍🏼 cc. @artembilan

artembilan · 2024-03-18T14:34:24Z

spring-kafka-docs/src/main/antora/modules/ROOT/pages/whats-new.adoc

+== Retrieve MessageListenerContainers with by matching a Predicate
+
+Starting with version 3.2, the `ListenerContainerRegistry` introduces the `getListenerContainersMatching` method.
+This method filters `MessageListenerContainer` instances based on a `Predicate<String>` applied to their IDs.


After your great docs I wonder if we could extend this new feature to something like this BiPredicate<String, MessageListenerContainer>. This way we may filter desired containers not only by their ids, but via some other option form the container itself.

WDYT?

Makes sense 👍🏼
I wonder if we should have both or just BiPredicate<String, MessageListenerContainer>. 🤔
Extended feature sounds very powerful, but might be too verbose/overkill in someway, knowing that only id has been used and is the key to Map containing MessageContainerListener.

Since the String, MessageListenerContainer pair in BiPredicate technically represents the key-value pair Map<String, MessageListenerContainer> listenerContainers stored in the registry, my suggestion is to have both current method and the extended version. I can promise to commit to writing the latter.

But I would follow your call @artembilan 👍🏼

OK. Let's have two of them!

Should we do a separate PR or do everything in this one?

This one is OK.
Thanks

Alright, hopefully it would probably take an hour or two

Trying to build locally, before re-requesting review 😆

artembilan · 2024-03-19T14:20:55Z

spring-kafka-docs/src/main/antora/modules/ROOT/pages/whats-new.adoc

+== Retrieve MessageListenerContainers with by matching a Predicate
+
+Starting with version 3.2, the `ListenerContainerRegistry` introduces the `getListenerContainersMatching` method.
+This method filters `MessageListenerContainer` instances based on a `Predicate<String>` applied to their IDs.


OK. Let's have two of them!

…stenerContainer> idAndContainerMatcher)

artembilan

The API is great: only some cosmetic concerns in regards of our code style rules.
Thanks

artembilan · 2024-03-19T19:46:28Z

spring-kafka/src/main/java/org/springframework/kafka/listener/ListenerContainerRegistry.java

+	 * @see #getListenerContainerIds()
+	 * @see #getListenerContainersMatching(Predicate)
+	 */
+	Collection<MessageListenerContainer> getListenerContainersMatching(BiPredicate<String, MessageListenerContainer> idAndContainerMatcher);


Let's see if you can follow the rule no more than 120 symbols per line!

artembilan · 2024-03-19T19:49:34Z

spring-kafka-docs/src/main/antora/modules/ROOT/pages/whats-new.adoc

+[[get-listener-containers-matching]]
+== Enhanced Retrieval of MessageListenerContainers
+
+Introduced in version 3.2, the `ListenerContainerRegistry` provides advanced methods for dynamically filtering `MessageListenerContainer` instances, enhancing management capabilities.


This file is fully specific to particular version we mention in the beginning of the file.
More over the content of the file is just a list of topics with links to details.
You do show enough in the kafkalistener-lifecycle.adoc, but that all must not be duplicated in What's New file.

True, thank you for pointing out!
Modified the part to only show the essence.

...kafka/src/test/java/org/springframework/kafka/config/KafkaListenerEndpointRegistryTests.java

artembilan · 2024-03-20T18:20:20Z

@JooHyukKim ,

thank you very much for the contribution; looking forward for more!

JooHyukKim added 3 commits March 16, 2024 01:01

Implement new getListenerContainersMatching() method

f525e08

Add tests

d40a318

Add @DisplayName for a test

9aa2850

JooHyukKim mentioned this pull request Mar 15, 2024

Adding new KafkaListenerEndpointRegistry.getListenerContainersMatchingId(predicate: Predicate<String>) #3135

Closed

artembilan requested changes Mar 15, 2024

View reviewed changes

JooHyukKim added 3 commits March 16, 2024 09:38

Apply review

97d4fba

Add documentation

d049ca1

Trim doc

2b4ed2a

JooHyukKim added 3 commits March 16, 2024 15:07

format : Fix CheckStyle

2b0f09f

Cover more getter documentation

77b9fde

Fix checkstyle

76e6484

JooHyukKim requested a review from artembilan March 16, 2024 06:49

artembilan requested changes Mar 18, 2024

View reviewed changes

artembilan requested changes Mar 19, 2024

View reviewed changes

JooHyukKim added 4 commits March 19, 2024 23:51

Doc : add doc for BiPredicate matcher

c4d8337

Feature : getListenerContainersMatching(BiPredicate<String, MessageLi…

78e863e

…stenerContainer> idAndContainerMatcher)

Doc : Final touch

21cf9c4

Refactor : Remove unnccesary changes

faa929f

JooHyukKim requested a review from artembilan March 19, 2024 15:22

artembilan requested changes Mar 19, 2024

View reviewed changes

JooHyukKim added 2 commits March 20, 2024 08:01

Refactor : fix codestyle as per note

a2f719e

Doc : Remove duplication in whats'new adoc

e91612f

JooHyukKim requested a review from artembilan March 19, 2024 23:09

artembilan merged commit d8e7299 into spring-projects:main Mar 20, 2024

JooHyukKim deleted the GH-3135 branch March 21, 2024 15:01

JooHyukKim mentioned this pull request Mar 24, 2024

Add checkstyle rule on empty-line-separater #3162

Merged

GH-3135 : Adding new ListenerContainerRegistry.getListenerContainersMatchingId(Predicate) for easier usage. #3137

GH-3135 : Adding new ListenerContainerRegistry.getListenerContainersMatchingId(Predicate) for easier usage. #3137

Uh oh!

Conversation

JooHyukKim commented Mar 15, 2024

Uh oh!

artembilan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

JooHyukKim commented Mar 16, 2024

Uh oh!

artembilan commented Mar 16, 2024

Uh oh!

JooHyukKim commented Mar 16, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JooHyukKim Mar 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JooHyukKim Mar 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

JooHyukKim Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

artembilan commented Mar 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

GH-3135 : Adding new `ListenerContainerRegistry.getListenerContainersMatchingId(Predicate)` for easier usage. #3137

GH-3135 : Adding new `ListenerContainerRegistry.getListenerContainersMatchingId(Predicate)` for easier usage. #3137

JooHyukKim Mar 18, 2024 •

edited

Loading

JooHyukKim Mar 18, 2024 •

edited

Loading

JooHyukKim Mar 19, 2024 •

edited

Loading