DonalEvans commented on a change in pull request #6487:
URL: https://github.com/apache/geode/pull/6487#discussion_r634808156



##########
File path: 
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
##########
@@ -340,7 +306,7 @@ long srem(ArrayList<ByteArrayWrapper> membersToRemove, 
Region<RedisKey, RedisDat
    * @return a set containing all the members in this set
    */
   @VisibleForTesting
-  Set<ByteArrayWrapper> smembers() {
+  public Set<byte[]> smembers() {
     return new HashSet<>(members);

Review comment:
       The commit prior to the current head returned an 
`ObjectOpenCustomHashSet` here (and in NullRedisSet) but this caused 
serialization issues in DUnit tests as that class wasn't allowed by the 
serialization filter. After trying to find an alternate solution, I've settled 
on extending `ObjectOpenCustomHashSet` to allow adding it to the appropriate 
sanctioned-serializables file, and to allow proper support of `addAll()` and 
`removeAll()`, since the current implementation of `ObjectOpenCustomHashSet` 
doesn't employ the hashing strategy set in the constructor for those.




-- 
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to