dschneider-pivotal commented on a change in pull request #6814:
URL: https://github.com/apache/geode/pull/6814#discussion_r703743113
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/pubsub/UnsubscribeExecutor.java
##########
@@ -15,60 +15,56 @@
package org.apache.geode.redis.internal.executor.pubsub;
-import static org.apache.geode.redis.internal.pubsub.Subscription.Type.CHANNEL;
+import static java.util.Collections.singletonList;
+import static
org.apache.geode.redis.internal.netty.StringBytesGlossary.bUNSUBSCRIBE;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;
import org.apache.geode.redis.internal.executor.AbstractExecutor;
import org.apache.geode.redis.internal.executor.RedisResponse;
+import org.apache.geode.redis.internal.netty.Client;
import org.apache.geode.redis.internal.netty.Command;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
public class UnsubscribeExecutor extends AbstractExecutor {
@Override
public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
-
List<byte[]> channelNames = extractChannelNames(command);
- if (channelNames.isEmpty()) {
- channelNames =
context.getPubSub().findSubscriptionNames(context.getClient(), CHANNEL);
- }
-
Collection<Collection<?>> response = unsubscribe(context, channelNames);
-
return RedisResponse.flattenedArray(response);
}
private List<byte[]> extractChannelNames(Command command) {
Review comment:
I think what all the executors want from Command in regards to the
List<byte[]> is the arguments. Most all of them probably never look at the
first element of the list returned from getProcessedCommand() since it is
always the name of the command which if you cared about you could get by
calling Command.getCommandType. So I decided to add a method on Command named
getCommandArguments. I didn't change anything else about Command but the nice
thing about getCommandArguments is it uses List.subList which on an ArrayList
does not make a copy of the array. I'll submit a future improvement store that
we should try to remove getProcessedCommand and then we could change the
internals of Command to just keep a reference to the sublist. Does that sound
good?
--
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]
