Skip to content
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

Jedis test plan coverage for CSC #3918

Merged
merged 11 commits into from
Aug 15, 2024
13 changes: 0 additions & 13 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,6 @@
</dependency>

<!-- Optional dependencies -->
<!-- Client-side caching -->
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>33.2.1-jre</version>
<optional>true</optional>
</dependency>
<dependency>
<groupId>com.github.ben-manes.caffeine</groupId>
<artifactId>caffeine</artifactId>
<version>2.9.3</version>
<optional>true</optional>
</dependency>

<!-- UNIX socket connection support -->
<dependency>
Expand Down
37 changes: 32 additions & 5 deletions src/main/java/redis/clients/jedis/CommandArguments.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;

import redis.clients.jedis.annots.Internal;
import redis.clients.jedis.args.Rawable;
import redis.clients.jedis.args.RawableFactory;
import redis.clients.jedis.commands.ProtocolCommand;
Expand All @@ -14,14 +17,20 @@
public class CommandArguments implements Iterable<Rawable> {

private final ArrayList<Rawable> args;
private final ArrayList<Object> keys;

private List<Object> keys;

private boolean blocking;

private CommandArguments() {
sazzad16 marked this conversation as resolved.
Show resolved Hide resolved
throw new InstantiationError();
}

public CommandArguments(ProtocolCommand command) {
args = new ArrayList<>();
args.add(command);
keys = new ArrayList<>();

keys = Collections.emptyList();
}

public ProtocolCommand getCommand() {
Expand Down Expand Up @@ -113,10 +122,25 @@ public CommandArguments key(Object key) {
} else {
throw new IllegalArgumentException("\"" + key.toString() + "\" is not a valid argument.");
}
keys.add(key);

addKeyInKeys(key);

return this;
}

private void addKeyInKeys(Object key) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are the sample cases we get the benefit from a collections.list??

Copy link
Collaborator Author

@sazzad16 sazzad16 Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most Redis commands have single key. this way we are avoiding creation of an array just for an entry. so saving memory.
moreover, there are more keyless commands than multikey commands. this is saving in those cases too.

if (keys.isEmpty()) {
keys = Collections.singletonList(key);
} else if (keys.size() == 1) {
List oldKeys = keys;
keys = new ArrayList();
keys.addAll(oldKeys);
keys.add(key);
} else {
keys.add(key);
}
}

public final CommandArguments keys(Object... keys) {
Arrays.stream(keys).forEach(this::key);
return this;
Expand All @@ -133,6 +157,7 @@ public final CommandArguments addParams(IParams params) {
}

protected CommandArguments processKey(byte[] key) {
// do nothing
return this;
}

Expand All @@ -144,6 +169,7 @@ protected final CommandArguments processKeys(byte[]... keys) {
}

protected CommandArguments processKey(String key) {
// do nothing
return this;
}

Expand All @@ -163,8 +189,9 @@ public Iterator<Rawable> iterator() {
return args.iterator();
}

public Object[] getKeys() {
return keys.toArray();
@Internal
public List<Object> getKeys() {
return keys;
}

public boolean isBlocking() {
Expand Down
24 changes: 11 additions & 13 deletions src/main/java/redis/clients/jedis/csc/AbstractCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,23 @@
import redis.clients.jedis.util.SafeEncoder;

/**
* The class to manage the client-side caching. User can provide any of implementation of this class
* to the client object; e.g. {@link redis.clients.jedis.csc.CaffeineClientSideCache
* CaffeineClientSideCache} or {@link redis.clients.jedis.csc.GuavaClientSideCache
* GuavaClientSideCache} or a custom implementation of their own.
* The class to manage the client-side caching. User can provide an of implementation of this class
* to the client object.
*/
@Experimental
public abstract class AbstractCache implements Cache {

private ClientSideCacheable cacheable;
private Cacheable cacheable;
private final Map<ByteBuffer, Set<CacheKey<?>>> redisKeysToCacheKeys = new ConcurrentHashMap<>();
private final int maximumSize;
private ReentrantLock lock = new ReentrantLock();
private volatile CacheStats stats = new CacheStats();

protected AbstractCache(int maximumSize) {
this(maximumSize, DefaultClientSideCacheable.INSTANCE);
this(maximumSize, DefaultCacheable.INSTANCE);
}

protected AbstractCache(int maximumSize, ClientSideCacheable cacheable) {
protected AbstractCache(int maximumSize, Cacheable cacheable) {
this.maximumSize = maximumSize;
this.cacheable = cacheable;
}
Expand Down Expand Up @@ -89,7 +87,7 @@ public CacheEntry set(CacheKey cacheKey, CacheEntry entry) {
}

@Override
public Boolean delete(CacheKey cacheKey) {
public boolean delete(CacheKey cacheKey) {
lock.lock();
try {
boolean removed = removeFromStore(cacheKey);
Expand Down Expand Up @@ -171,12 +169,12 @@ public int flush() {
}

@Override
public Boolean isCacheable(CacheKey cacheKey) {
return cacheable.isCacheable(cacheKey.getCommand().getArguments().getCommand(), cacheKey.getRedisKeys());
public boolean isCacheable(CacheKey cacheKey) {
return cacheable.isCacheable(cacheKey.getRedisCommand(), cacheKey.getRedisKeys());
}

@Override
public Boolean hasCacheKey(CacheKey cacheKey) {
public boolean hasCacheKey(CacheKey cacheKey) {
return containsKeyInStore(cacheKey);
}

Expand All @@ -202,13 +200,13 @@ public CacheStats getAndResetStats() {

protected abstract CacheEntry putIntoStore(CacheKey cacheKey, CacheEntry entry);

protected abstract Boolean removeFromStore(CacheKey cacheKey);
protected abstract boolean removeFromStore(CacheKey cacheKey);

// protected abstract Collection<CacheKey> remove(Set<CacheKey<?>> commands);

protected abstract void clearStore();

protected abstract Boolean containsKeyInStore(CacheKey cacheKey);
protected abstract boolean containsKeyInStore(CacheKey cacheKey);

// End of abstract methods to be implemented by the concrete classes

Expand Down
10 changes: 5 additions & 5 deletions src/main/java/redis/clients/jedis/csc/Cache.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public interface Cache {
* @param cacheKey The cache key of the entry in the cache
* @return True if the entry could be deleted, false if the entry wasn't found.
*/
Boolean delete(CacheKey cacheKey);
boolean delete(CacheKey cacheKey);

/**
* Delete entries by cache key from the cache
Expand Down Expand Up @@ -82,14 +82,14 @@ public interface Cache {
* @param cacheKey The key of the cache entry
* @return True if the entry is cachable, false otherwise
*/
Boolean isCacheable(CacheKey cacheKey);
boolean isCacheable(CacheKey cacheKey);

/**
*
* @param cacheKey The key of the cache entry
* @return True if the cache already contains the key
*/
Boolean hasCacheKey(CacheKey cacheKey);
boolean hasCacheKey(CacheKey cacheKey);

/**
* @return The eviction policy that is used by the cache
Expand All @@ -99,10 +99,10 @@ public interface Cache {
/**
* @return The statistics of the cache
*/
public CacheStats getStats();
CacheStats getStats();

/**
* @return The statistics of the cache
*/
public CacheStats getAndResetStats();
CacheStats getAndResetStats();
}
6 changes: 1 addition & 5 deletions src/main/java/redis/clients/jedis/csc/CacheEntry.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@
import java.lang.ref.WeakReference;

import redis.clients.jedis.Connection;
import redis.clients.jedis.annots.Internal;
import redis.clients.jedis.exceptions.JedisCacheException;

@Internal
public class CacheEntry<T> {

private final CacheKey<T> cacheKey;
Expand Down Expand Up @@ -52,9 +50,7 @@ private T toObject(byte[] data) {
try (ByteArrayInputStream bais = new ByteArrayInputStream(data);
ObjectInputStream ois = new ObjectInputStream(bais)) {
return (T) ois.readObject();
} catch (IOException e) {
throw new JedisCacheException("Failed to deserialize object", e);
} catch (ClassNotFoundException e) {
} catch (IOException | ClassNotFoundException e) {
throw new JedisCacheException("Failed to deserialize object", e);
}
}
Expand Down
11 changes: 6 additions & 5 deletions src/main/java/redis/clients/jedis/csc/CacheKey.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package redis.clients.jedis.csc;

import java.util.List;
import java.util.Objects;

import redis.clients.jedis.CommandObject;
import redis.clients.jedis.annots.Internal;
import redis.clients.jedis.commands.ProtocolCommand;

@Internal
public class CacheKey<T> {

private final CommandObject<T> command;
Expand All @@ -26,11 +27,11 @@ public boolean equals(Object obj) {
return Objects.equals(this.command, other.command);
}

public Object[] getRedisKeys() {
public List<Object> getRedisKeys() {
return command.getArguments().getKeys();
}

public CommandObject<T> getCommand() {
return command;
public ProtocolCommand getRedisCommand() {
return command.getArguments().getCommand();
}
}
9 changes: 9 additions & 0 deletions src/main/java/redis/clients/jedis/csc/Cacheable.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package redis.clients.jedis.csc;

import java.util.List;
import redis.clients.jedis.commands.ProtocolCommand;

public interface Cacheable {

boolean isCacheable(ProtocolCommand command, List<Object> keys);
}
71 changes: 0 additions & 71 deletions src/main/java/redis/clients/jedis/csc/CaffeineClientSideCache.java

This file was deleted.

This file was deleted.

10 changes: 5 additions & 5 deletions src/main/java/redis/clients/jedis/csc/DefaultCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ public DefaultCache(int maximumSize) {
}

public DefaultCache(int maximumSize, Map<CacheKey, CacheEntry> map) {
this(maximumSize, map, DefaultClientSideCacheable.INSTANCE, new LRUEviction(maximumSize));
this(maximumSize, map, DefaultCacheable.INSTANCE, new LRUEviction(maximumSize));
}

public DefaultCache(int maximumSize, ClientSideCacheable cacheable) {
public DefaultCache(int maximumSize, Cacheable cacheable) {
this(maximumSize, new HashMap<CacheKey, CacheEntry>(), cacheable, new LRUEviction(maximumSize));
}

public DefaultCache(int maximumSize, Map<CacheKey, CacheEntry> map, ClientSideCacheable cacheable, EvictionPolicy evictionPolicy) {
public DefaultCache(int maximumSize, Map<CacheKey, CacheEntry> map, Cacheable cacheable, EvictionPolicy evictionPolicy) {
super(maximumSize, cacheable);
this.cache = map;
this.evictionPolicy = evictionPolicy;
Expand Down Expand Up @@ -54,7 +54,7 @@ public CacheEntry putIntoStore(CacheKey key, CacheEntry entry) {
}

@Override
public Boolean removeFromStore(CacheKey key) {
public boolean removeFromStore(CacheKey key) {
return cache.remove(key) != null;
}

Expand All @@ -64,7 +64,7 @@ protected final void clearStore() {
}

@Override
protected Boolean containsKeyInStore(CacheKey cacheKey) {
protected boolean containsKeyInStore(CacheKey cacheKey) {
return cache.containsKey(cacheKey);
}

Expand Down
Loading
Loading