DonalEvans commented on a change in pull request #6814:
URL: https://github.com/apache/geode/pull/6814#discussion_r703905062
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/pubsub/Subscriptions.java
##########
@@ -96,104 +66,84 @@ boolean exists(Object channelOrPattern, Client client) {
* @param pattern the glob pattern to search for
*/
public List<byte[]> findChannelNames(byte[] pattern) {
-
- GlobPattern globPattern = new GlobPattern(bytesToString(pattern));
-
- return findChannelNames()
- .stream()
- .filter(name -> globPattern.matches(bytesToString(name)))
- .collect(Collectors.toList());
+ return channelSubscriptions.getIds(pattern);
}
/**
- * Return a list consisting of pairs {@code channelName, subscriptionCount}.
- *
- * @param names a list of the names to consider. This should not include any
patterns.
+ * Return a count of all pattern subscriptions including duplicates.
*/
- public List<Object> findNumberOfSubscribersPerChannel(List<byte[]> names) {
- List<Object> result = new ArrayList<>();
-
- names.forEach(name -> {
- Long subscriptionCount = findSubscriptions(name)
- .stream()
- .filter(subscription -> subscription instanceof ChannelSubscription)
- .count();
-
- result.add(name);
- result.add(subscriptionCount);
- });
+ public int getPatternSubscriptionCount() {
+ return patternSubscriptions.getSubscriptionCount();
+ }
- return result;
+ @VisibleForTesting
+ int getChannelSubscriptionCount() {
+ return channelSubscriptions.getSubscriptionCount();
}
- /**
- * Return a count of all pattern subscriptions including duplicates.
- */
- public long findNumberOfPatternSubscriptions() {
- return subscriptions.stream()
- .filter(subscription -> subscription instanceof PatternSubscription)
- .count();
+ void add(ChannelSubscription subscription) {
+ channelSubscriptions.add(subscription);
}
- /**
- * Add a new subscription
- */
- @VisibleForTesting
- void add(Subscription subscription) {
- subscriptions.add(subscription);
+ void add(PatternSubscription subscription) {
+ patternSubscriptions.add(subscription);
}
/**
* Remove all subscriptions for a given client
*/
public void remove(Client client) {
- subscriptions.removeIf(subscription -> subscription.matchesClient(client));
- }
-
- /**
- * Remove a single subscription
- */
- @VisibleForTesting
- boolean remove(Object channel, Client client) {
- return subscriptions.removeIf(subscription ->
subscription.isEqualTo(channel, client));
+ channelSubscriptions.remove(client);
+ patternSubscriptions.remove(client);
+ client.clearSubscriptions();
}
/**
* @return the total number of all local subscriptions
*/
@VisibleForTesting
int size() {
- return subscriptions.size();
+ // this is only used by tests so performance is not an issue
+ return getChannelSubscriptionCount() + getPatternSubscriptionCount();
}
- public synchronized SubscribeResult subscribe(byte[] channel,
ExecutionHandlerContext context,
- Client client) {
- Subscription createdSubscription = null;
- if (!exists(channel, client)) {
- createdSubscription = new ChannelSubscription(client, channel, context,
this);
+ public SubscribeResult subscribe(byte[] channel, ExecutionHandlerContext
context) {
+ final Client client = context.getClient();
+ ChannelSubscription createdSubscription = null;
+ if (client.addChannelSubscription(channel)) {
+ createdSubscription = new ChannelSubscription(channel, context, this);
add(createdSubscription);
}
- long channelCount = findSubscriptions(client).size();
+ long channelCount = client.getSubscriptionCount();
return new SubscribeResult(createdSubscription, channelCount, channel);
}
- public SubscribeResult psubscribe(byte[] patternBytes,
ExecutionHandlerContext context,
- Client client) {
- GlobPattern pattern = new GlobPattern(bytesToString(patternBytes));
- Subscription createdSubscription = null;
- synchronized (this) {
- if (!exists(pattern, client)) {
- createdSubscription = new PatternSubscription(client, pattern,
context, this);
+ public SubscribeResult psubscribe(byte[] patternBytes,
ExecutionHandlerContext context) {
+ final Client client = context.getClient();
+ PatternSubscription createdSubscription = null;
+ if (client.addPatternSubscription(patternBytes)) {
+ boolean added = false;
+ try {
+ createdSubscription = new PatternSubscription(patternBytes, context,
this);
add(createdSubscription);
+ added = true;
+ } finally {
+ if (!added) {
+ // Must have had a problem parsing the pattern
Review comment:
Ah, nice catch. I just did some testing and interestingly, when using
the Redis CLI, a PSUBSCRIBE pattern of `\C` gets treated as `\\C` but a pattern
of `"\C"` gets treated as `C`, so there might be some trickiness here if we
want to behave exactly the same as native Redis.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
