-
Notifications
You must be signed in to change notification settings - Fork 157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple Filter Subjects and Subject validation changes #965
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor stuff
@@ -375,14 +375,14 @@ public interface JetStream { | |||
* <p>See {@link io.nats.client.Connection#createDispatcher(MessageHandler) createDispatcher} for | |||
* information about creating an asynchronous subscription with callbacks. | |||
* | |||
* @param subject the subject to subscribe to | |||
* @param subscribeSubject the subject to subscribe to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document when this can be empty or null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
* Gets the max filter subject of this consumer configuration. | ||
* @return the filter subject. | ||
* Gets the first filter subject of this consumer configuration. Will be null if there are none. | ||
* @return the first filter subject. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make this deprecated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's fine.
// 2. figure out user provided subjects and prepare the userCcFilterSubjects | ||
userSubscribeSubject = emptyAsNull(userSubscribeSubject); | ||
List<String> userCcFilterSubjects = new ArrayList<>(); | ||
if (userCC.getFilterSubject() == null) { // empty filterSubjects gives gives null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
String subj; | ||
if (consumerCreate290Available) { | ||
if (consumerCreate290Available && !multipleFilterSubject) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add a code comment for this block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
1. Support for Multiple Filter Subjects feature in Server 2.10
2. Changes and consistency for Subject validation
3. Changes and consistency for Queue name validation