-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Adding CacheConfig #3919
Adding CacheConfig #3919
Changes from all commits
f27bb1c
2685c9b
c4e740e
2136d1b
0a828b3
5896984
b047881
8a96f0a
059eb4d
aa5f27f
4fe1c56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -235,10 +235,8 @@ public static void readPushes(final RedisInputStream is, final Cache cache, bool | |||||
is.readByte(); | ||||||
processPush(is, cache); | ||||||
} | ||||||
} catch (JedisConnectionException e) { | ||||||
// TODO handle it properly | ||||||
} catch (IOException e) { | ||||||
// TODO handle it properly | ||||||
throw new JedisConnectionException("Failed to read pending buffer for push messages !", e); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
} else { | ||||||
while (is.peek(GREATER_THAN_BYTE)) { | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
package redis.clients.jedis.csc; | ||
|
||
public class CacheConfig { | ||
|
||
private int maxSize; | ||
private Cacheable cacheable; | ||
private EvictionPolicy evictionPolicy; | ||
|
||
public int getMaxSize() { | ||
return maxSize; | ||
} | ||
|
||
public Cacheable getCacheable() { | ||
return cacheable; | ||
} | ||
|
||
public EvictionPolicy getEvictionPolicy() { | ||
return evictionPolicy; | ||
} | ||
|
||
public static Builder builder() { | ||
return new Builder(); | ||
} | ||
|
||
public static class Builder { | ||
private int maxSize; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Set the default value. |
||
private Cacheable cacheable = DefaultCacheable.INSTANCE; | ||
private EvictionPolicy evictionPolicy; | ||
|
||
public Builder maxSize(int maxSize) { | ||
this.maxSize = maxSize; | ||
return this; | ||
} | ||
|
||
public Builder evictionPolicy(EvictionPolicy policy) { | ||
this.evictionPolicy = policy; | ||
return this; | ||
} | ||
|
||
public Builder cacheable(Cacheable cacheable) { | ||
this.cacheable = cacheable; | ||
return this; | ||
} | ||
|
||
public CacheConfig build() { | ||
CacheConfig cacheConfig = new CacheConfig(); | ||
cacheConfig.maxSize = this.maxSize; | ||
cacheConfig.cacheable = this.cacheable; | ||
cacheConfig.evictionPolicy = this.evictionPolicy; | ||
return cacheConfig; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package redis.clients.jedis.csc; | ||
|
||
import java.util.HashMap; | ||
|
||
public class CacheProvider { | ||
atakavci marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should rename this class. IMHO, the way the term Provider is used in this repo doesn't go along with the usage/behavior of this class. (Suggestions: CacheManager, CacheUtil, CacheConfigManager, CacheConfigHandler.) |
||
|
||
public Cache getCache(CacheConfig config) { | ||
return getCache(config, new HashMap<CacheKey, CacheEntry>()); | ||
} | ||
|
||
public Cache getCache(CacheConfig config, HashMap<CacheKey, CacheEntry> map) { | ||
return new DefaultCache(config.getMaxSize(), map, config.getCacheable(), | ||
getEvictionPolicy(config)); | ||
} | ||
Comment on lines
+7
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just asking/discussing: How about renaming BTW, if this class is rename to CacheManager or CacheUtil, createCache or simply |
||
|
||
private EvictionPolicy getEvictionPolicy(CacheConfig config) { | ||
if (config.getEvictionPolicy() == null) { | ||
// It will be default to LRUEviction, until we have other eviction implementations | ||
return new LRUEviction(config.getMaxSize()); | ||
} | ||
return config.getEvictionPolicy(); | ||
} | ||
} |
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.