Repository: cassandra Updated Branches: refs/heads/trunk cc12665bb -> 960174da6
Make AuthCache easier to subclass Patch by Kurt Greaves; reviewed by Sam Tunnicliffe for CASSANDRA-14662 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/960174da Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/960174da Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/960174da Branch: refs/heads/trunk Commit: 960174da67eb6008c73340e61700ea34ec550a12 Parents: cc12665 Author: kurt <[email protected]> Authored: Sat Sep 1 21:43:58 2018 +0100 Committer: Sam Tunnicliffe <[email protected]> Committed: Sat Sep 1 21:46:11 2018 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/auth/AuthCache.java | 115 +++++++++++----- .../apache/cassandra/auth/AuthCacheMBean.java | 4 +- .../apache/cassandra/auth/NetworkAuthCache.java | 2 +- .../cassandra/auth/PasswordAuthenticator.java | 2 +- .../apache/cassandra/auth/PermissionsCache.java | 2 +- .../cassandra/auth/PermissionsCacheMBean.java | 26 ---- .../org/apache/cassandra/auth/RolesCache.java | 2 +- .../apache/cassandra/auth/RolesCacheMBean.java | 26 ---- .../apache/cassandra/auth/AuthCacheTest.java | 137 +++++++++++++++++++ 10 files changed, 229 insertions(+), 88 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/960174da/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index a7468f4..aca31fe 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.0 + * Make AuthCache more easily extendable (CASSANDRA-14662) * Extend RolesCache to include detailed role info (CASSANDRA-14497) * Add fqltool compare (CASSANDRA-14619) * Add fqltool replay (CASSANDRA-14618) http://git-wip-us.apache.org/repos/asf/cassandra/blob/960174da/src/java/org/apache/cassandra/auth/AuthCache.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/auth/AuthCache.java b/src/java/org/apache/cassandra/auth/AuthCache.java index d6ff0b0..4f36a63 100644 --- a/src/java/org/apache/cassandra/auth/AuthCache.java +++ b/src/java/org/apache/cassandra/auth/AuthCache.java @@ -35,24 +35,40 @@ import com.github.benmanes.caffeine.cache.LoadingCache; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class AuthCache<K, V> implements AuthCacheMBean +import static com.google.common.base.Preconditions.checkNotNull; + +public class AuthCache<K, V> implements AuthCacheMBean<K> { private static final Logger logger = LoggerFactory.getLogger(AuthCache.class); private static final String MBEAN_NAME_BASE = "org.apache.cassandra.auth:type="; - private volatile LoadingCache<K, V> cache; - - private final String name; - private final IntConsumer setValidityDelegate; - private final IntSupplier getValidityDelegate; - private final IntConsumer setUpdateIntervalDelegate; - private final IntSupplier getUpdateIntervalDelegate; - private final IntConsumer setMaxEntriesDelegate; - private final IntSupplier getMaxEntriesDelegate; - private final Function<K, V> loadFunction; - private final BooleanSupplier enableCache; - + /** + * Underlying cache. LoadingCache will call underlying load function on {@link #get} if key is not present + */ + protected volatile LoadingCache<K, V> cache; + + private String name; + private IntConsumer setValidityDelegate; + private IntSupplier getValidityDelegate; + private IntConsumer setUpdateIntervalDelegate; + private IntSupplier getUpdateIntervalDelegate; + private IntConsumer setMaxEntriesDelegate; + private IntSupplier getMaxEntriesDelegate; + private Function<K, V> loadFunction; + private BooleanSupplier enableCache; + + /** + * @param name Used for MBean + * @param setValidityDelegate Used to set cache validity period. See {@link Policy#expireAfterWrite()} + * @param getValidityDelegate Getter for validity period + * @param setUpdateIntervalDelegate Used to set cache update interval. See {@link Policy#refreshAfterWrite()} + * @param getUpdateIntervalDelegate Getter for update interval + * @param setMaxEntriesDelegate Used to set max # entries in cache. See {@link com.github.benmanes.caffeine.cache.Policy.Eviction#setMaximum(long)} + * @param getMaxEntriesDelegate Getter for max entries. + * @param loadFunction Function to load the cache. Called on {@link #get(Object)} + * @param cacheEnabledDelegate Used to determine if cache is enabled. + */ protected AuthCache(String name, IntConsumer setValidityDelegate, IntSupplier getValidityDelegate, @@ -61,23 +77,26 @@ public class AuthCache<K, V> implements AuthCacheMBean IntConsumer setMaxEntriesDelegate, IntSupplier getMaxEntriesDelegate, Function<K, V> loadFunction, - BooleanSupplier enableCache) + BooleanSupplier cacheEnabledDelegate) { - this.name = name; - this.setValidityDelegate = setValidityDelegate; - this.getValidityDelegate = getValidityDelegate; - this.setUpdateIntervalDelegate = setUpdateIntervalDelegate; - this.getUpdateIntervalDelegate = getUpdateIntervalDelegate; - this.setMaxEntriesDelegate = setMaxEntriesDelegate; - this.getMaxEntriesDelegate = getMaxEntriesDelegate; - this.loadFunction = loadFunction; - this.enableCache = enableCache; + this.name = checkNotNull(name); + this.setValidityDelegate = checkNotNull(setValidityDelegate); + this.getValidityDelegate = checkNotNull(getValidityDelegate); + this.setUpdateIntervalDelegate = checkNotNull(setUpdateIntervalDelegate); + this.getUpdateIntervalDelegate = checkNotNull(getUpdateIntervalDelegate); + this.setMaxEntriesDelegate = checkNotNull(setMaxEntriesDelegate); + this.getMaxEntriesDelegate = checkNotNull(getMaxEntriesDelegate); + this.loadFunction = checkNotNull(loadFunction); + this.enableCache = checkNotNull(cacheEnabledDelegate); init(); } + /** + * Do setup for the cache and MBean. + */ protected void init() { - this.cache = initCache(null); + cache = initCache(null); try { MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); @@ -107,6 +126,14 @@ public class AuthCache<K, V> implements AuthCacheMBean return new ObjectName(MBEAN_NAME_BASE + name); } + /** + * Retrieve a value from the cache. Will call {@link LoadingCache#get(Object)} which will + * "load" the value if it's not present, thus populating the key. + * @param k + * @return The current value of {@code K} if cached or loaded. + * + * See {@link LoadingCache#get(Object)} for possible exceptions. + */ public V get(K k) { if (cache == null) @@ -115,17 +142,28 @@ public class AuthCache<K, V> implements AuthCacheMBean return cache.get(k); } + /** + * Invalidate the entire cache. + */ public void invalidate() { cache = initCache(null); } + /** + * Invalidate a key + * @param k key to invalidate + */ public void invalidate(K k) { if (cache != null) cache.invalidate(k); } + /** + * Time in milliseconds that a value in the cache will expire after. + * @param validityPeriod in milliseconds + */ public void setValidity(int validityPeriod) { if (Boolean.getBoolean("cassandra.disable_auth_caches_remote_configuration")) @@ -140,6 +178,10 @@ public class AuthCache<K, V> implements AuthCacheMBean return getValidityDelegate.getAsInt(); } + /** + * Time in milliseconds after which an entry in the cache should be refreshed (it's load function called again) + * @param updateInterval in milliseconds + */ public void setUpdateInterval(int updateInterval) { if (Boolean.getBoolean("cassandra.disable_auth_caches_remote_configuration")) @@ -154,6 +196,10 @@ public class AuthCache<K, V> implements AuthCacheMBean return getUpdateIntervalDelegate.getAsInt(); } + /** + * Set maximum number of entries in the cache. + * @param maxEntries + */ public void setMaxEntries(int maxEntries) { if (Boolean.getBoolean("cassandra.disable_auth_caches_remote_configuration")) @@ -168,7 +214,14 @@ public class AuthCache<K, V> implements AuthCacheMBean return getMaxEntriesDelegate.getAsInt(); } - private LoadingCache<K, V> initCache(LoadingCache<K, V> existing) + /** + * (Re-)initialise the underlying cache. Will update validity, max entries, and update interval if + * any have changed. The underlying {@link LoadingCache} will be initiated based on the provided {@code loadFunction}. + * Note: If you need some unhandled cache setting to be set you should extend {@link AuthCache} and override this method. + * @param existing If not null will only update cache update validity, max entries, and update interval. + * @return New {@link LoadingCache} if existing was null, otherwise the existing {@code cache} + */ + protected LoadingCache<K, V> initCache(LoadingCache<K, V> existing) { if (!enableCache.getAsBoolean()) return null; @@ -181,14 +234,14 @@ public class AuthCache<K, V> implements AuthCacheMBean if (existing == null) { return Caffeine.newBuilder() - .refreshAfterWrite(getUpdateInterval(), TimeUnit.MILLISECONDS) - .expireAfterWrite(getValidity(), TimeUnit.MILLISECONDS) - .maximumSize(getMaxEntries()) - .executor(MoreExecutors.directExecutor()) - .build(loadFunction::apply); + .refreshAfterWrite(getUpdateInterval(), TimeUnit.MILLISECONDS) + .expireAfterWrite(getValidity(), TimeUnit.MILLISECONDS) + .maximumSize(getMaxEntries()) + .executor(MoreExecutors.directExecutor()) + .build(loadFunction::apply); } - // Always set as manditory + // Always set as mandatory cache.policy().refreshAfterWrite().ifPresent(policy -> policy.setExpiresAfter(getUpdateInterval(), TimeUnit.MILLISECONDS)); cache.policy().expireAfterWrite().ifPresent(policy -> http://git-wip-us.apache.org/repos/asf/cassandra/blob/960174da/src/java/org/apache/cassandra/auth/AuthCacheMBean.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/auth/AuthCacheMBean.java b/src/java/org/apache/cassandra/auth/AuthCacheMBean.java index 43fb88e..1416044 100644 --- a/src/java/org/apache/cassandra/auth/AuthCacheMBean.java +++ b/src/java/org/apache/cassandra/auth/AuthCacheMBean.java @@ -18,10 +18,12 @@ package org.apache.cassandra.auth; -public interface AuthCacheMBean +public interface AuthCacheMBean<T> { public void invalidate(); + public void invalidate(T t); + public void setValidity(int validityPeriod); public int getValidity(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/960174da/src/java/org/apache/cassandra/auth/NetworkAuthCache.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/auth/NetworkAuthCache.java b/src/java/org/apache/cassandra/auth/NetworkAuthCache.java index 15b1819..0991889 100644 --- a/src/java/org/apache/cassandra/auth/NetworkAuthCache.java +++ b/src/java/org/apache/cassandra/auth/NetworkAuthCache.java @@ -20,7 +20,7 @@ package org.apache.cassandra.auth; import org.apache.cassandra.config.DatabaseDescriptor; -public class NetworkAuthCache extends AuthCache<RoleResource, DCPermissions> implements AuthCacheMBean +public class NetworkAuthCache extends AuthCache<RoleResource, DCPermissions> { public NetworkAuthCache(INetworkAuthorizer authorizer) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/960174da/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java b/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java index 27a68a0..b10136e 100644 --- a/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java +++ b/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java @@ -228,7 +228,7 @@ public class PasswordAuthenticator implements IAuthenticator } } - private static class CredentialsCache extends AuthCache<String, String> implements CredentialsCacheMBean + private static class CredentialsCache extends AuthCache<String, String> { private CredentialsCache(PasswordAuthenticator authenticator) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/960174da/src/java/org/apache/cassandra/auth/PermissionsCache.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/auth/PermissionsCache.java b/src/java/org/apache/cassandra/auth/PermissionsCache.java index 981ede8..a33f5d1 100644 --- a/src/java/org/apache/cassandra/auth/PermissionsCache.java +++ b/src/java/org/apache/cassandra/auth/PermissionsCache.java @@ -22,7 +22,7 @@ import java.util.Set; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.utils.Pair; -public class PermissionsCache extends AuthCache<Pair<AuthenticatedUser, IResource>, Set<Permission>> implements PermissionsCacheMBean +public class PermissionsCache extends AuthCache<Pair<AuthenticatedUser, IResource>, Set<Permission>> { public PermissionsCache(IAuthorizer authorizer) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/960174da/src/java/org/apache/cassandra/auth/PermissionsCacheMBean.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/auth/PermissionsCacheMBean.java b/src/java/org/apache/cassandra/auth/PermissionsCacheMBean.java deleted file mode 100644 index d370d06..0000000 --- a/src/java/org/apache/cassandra/auth/PermissionsCacheMBean.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.cassandra.auth; - -/** - * Retained since CASSANDRA-7715 for backwards compatibility of MBean interface - * classes. This should be removed in the next major version (4.0) - */ -public interface PermissionsCacheMBean extends AuthCacheMBean -{ -} http://git-wip-us.apache.org/repos/asf/cassandra/blob/960174da/src/java/org/apache/cassandra/auth/RolesCache.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/auth/RolesCache.java b/src/java/org/apache/cassandra/auth/RolesCache.java index cc178ce..d01de63 100644 --- a/src/java/org/apache/cassandra/auth/RolesCache.java +++ b/src/java/org/apache/cassandra/auth/RolesCache.java @@ -23,7 +23,7 @@ import java.util.stream.Collectors; import org.apache.cassandra.config.DatabaseDescriptor; -public class RolesCache extends AuthCache<RoleResource, Set<Role>> implements RolesCacheMBean +public class RolesCache extends AuthCache<RoleResource, Set<Role>> { public RolesCache(IRoleManager roleManager, BooleanSupplier enableCache) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/960174da/src/java/org/apache/cassandra/auth/RolesCacheMBean.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/auth/RolesCacheMBean.java b/src/java/org/apache/cassandra/auth/RolesCacheMBean.java deleted file mode 100644 index 06482d7..0000000 --- a/src/java/org/apache/cassandra/auth/RolesCacheMBean.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.cassandra.auth; - -/** - * Retained since CASSANDRA-7715 for backwards compatibility of MBean interface - * classes. This should be removed in the next major version (4.0) - */ -public interface RolesCacheMBean extends AuthCacheMBean -{ -} http://git-wip-us.apache.org/repos/asf/cassandra/blob/960174da/test/unit/org/apache/cassandra/auth/AuthCacheTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/auth/AuthCacheTest.java b/test/unit/org/apache/cassandra/auth/AuthCacheTest.java new file mode 100644 index 0000000..cc78ebc --- /dev/null +++ b/test/unit/org/apache/cassandra/auth/AuthCacheTest.java @@ -0,0 +1,137 @@ + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.cassandra.auth; + +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.config.DatabaseDescriptor; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +public class AuthCacheTest +{ + private boolean loadFuncCalled = false; + private boolean isCacheEnabled = false; + + @BeforeClass + public static void setup() + { + DatabaseDescriptor.daemonInitialization(); + } + + @Test + public void testCaching() + { + AuthCache<String, String> authCache = new AuthCache<>("TestCache", + DatabaseDescriptor::setCredentialsValidity, + DatabaseDescriptor::getCredentialsValidity, + DatabaseDescriptor::setCredentialsUpdateInterval, + DatabaseDescriptor::getCredentialsUpdateInterval, + DatabaseDescriptor::setCredentialsCacheMaxEntries, + DatabaseDescriptor::getCredentialsCacheMaxEntries, + this::load, + () -> true + ); + + // Test cacheloader is called if set + loadFuncCalled = false; + String result = authCache.get("test"); + assertTrue(loadFuncCalled); + Assert.assertEquals("load", result); + + // value should be fetched from cache + loadFuncCalled = false; + String result2 = authCache.get("test"); + assertFalse(loadFuncCalled); + Assert.assertEquals("load", result2); + + // value should be fetched from cache after complete invalidate + authCache.invalidate(); + loadFuncCalled = false; + String result3 = authCache.get("test"); + assertTrue(loadFuncCalled); + Assert.assertEquals("load", result3); + + // value should be fetched from cache after invalidating key + authCache.invalidate("test"); + loadFuncCalled = false; + String result4 = authCache.get("test"); + assertTrue(loadFuncCalled); + Assert.assertEquals("load", result4); + + // set cache to null and load function should be called + loadFuncCalled = false; + authCache.cache = null; + String result5 = authCache.get("test"); + assertTrue(loadFuncCalled); + Assert.assertEquals("load", result5); + } + + @Test + public void testInitCache() + { + // Test that a validity of <= 0 will turn off caching + DatabaseDescriptor.setCredentialsValidity(0); + AuthCache<String, String> authCache = new AuthCache<>("TestCache2", + DatabaseDescriptor::setCredentialsValidity, + DatabaseDescriptor::getCredentialsValidity, + DatabaseDescriptor::setCredentialsUpdateInterval, + DatabaseDescriptor::getCredentialsUpdateInterval, + DatabaseDescriptor::setCredentialsCacheMaxEntries, + DatabaseDescriptor::getCredentialsCacheMaxEntries, + this::load, + () -> true); + assertNull(authCache.cache); + authCache.setValidity(2000); + authCache.cache = authCache.initCache(null); + assertNotNull(authCache.cache); + + // Test enableCache works as intended + authCache = new AuthCache<>("TestCache3", + DatabaseDescriptor::setCredentialsValidity, + DatabaseDescriptor::getCredentialsValidity, + DatabaseDescriptor::setCredentialsUpdateInterval, + DatabaseDescriptor::getCredentialsUpdateInterval, + DatabaseDescriptor::setCredentialsCacheMaxEntries, + DatabaseDescriptor::getCredentialsCacheMaxEntries, + this::load, + () -> isCacheEnabled); + assertNull(authCache.cache); + isCacheEnabled = true; + authCache.cache = authCache.initCache(null); + assertNotNull(authCache.cache); + + // Ensure at a minimum these policies have been initialised by default + assertTrue(authCache.cache.policy().expireAfterWrite().isPresent()); + assertTrue(authCache.cache.policy().refreshAfterWrite().isPresent()); + assertTrue(authCache.cache.policy().eviction().isPresent()); + } + + private String load(String test) + { + loadFuncCalled = true; + return "load"; + } + +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
