Skip to content

Commit

Permalink
refactor(cats-core): Remove Cache.StoreType (#4050)
Browse files Browse the repository at this point in the history
We don't need to leak this information to clients. They don't care what
the storage type is; they care whether or not getAllByApplication works,
so just let them ask that.
  • Loading branch information
plumpy authored and cfieber committed Sep 27, 2019
1 parent b234aca commit 757b6f6
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 38 deletions.
Expand Up @@ -23,11 +23,6 @@

/** Cache provides view access to data keyed by type and identifier. */
public interface Cache {
enum StoreType {
REDIS,
IN_MEMORY,
SQL
}

/**
* Gets a single item from the cache by type and id
Expand Down Expand Up @@ -122,10 +117,18 @@ Collection<CacheData> getAll(
*/
Collection<CacheData> getAll(String type, String... identifiers);

/** Returns whether or not the three {@code getAllByApplication} methods are supported */
default boolean supportsGetAllByApplication() {
return false;
}

/**
* Retrieves all items for the specified type associated with the provided application. Requires a
* storeType with secondary indexes and support in the type's caching agent.
*
* <p>Clients should check {@link #supportsGetAllByApplication()} to check if this method is
* supported before calling it.
*
* @param type the type for which to retrieve items
* @param application the application name
* @return the matching items, keyed by type
Expand All @@ -138,6 +141,9 @@ default Map<String, Collection<CacheData>> getAllByApplication(String type, Stri
* Retrieves all items for the specified type associated with the provided application. Requires a
* storeType with secondary indexes and support in the type's caching agent.
*
* <p>Clients should check {@link #supportsGetAllByApplication()} to check if this method is
* supported before calling it.
*
* @param type the type for which to retrieve items
* @param application the application name
* @param cacheFilter the cacheFilter to govern which relationships to fetch
Expand All @@ -152,6 +158,9 @@ default Map<String, Collection<CacheData>> getAllByApplication(
* Retrieves all items for the specified type associated with the provided application. Requires a
* storeType with secondary indexes and support in the type's caching agent.
*
* <p>Clients should check {@link #supportsGetAllByApplication()} to check if this method is
* supported before calling it.
*
* @param types collection of types for which to retrieve items
* @param application the application name
* @param cacheFilters cacheFilters to govern which relationships to fetch, as type to filter
Expand All @@ -161,13 +170,4 @@ default Map<String, Collection<CacheData>> getAllByApplication(
Collection<String> types, String application, Map<String, CacheFilter> cacheFilters) {
throw new UnsupportedCacheMethodException("Method only implemented for StoreType.SQL");
}

/**
* Get backing store type for Cache implementation
*
* @return the backing StoreType
*/
default StoreType storeType() {
return StoreType.REDIS;
}
}
Expand Up @@ -17,7 +17,6 @@
package com.netflix.spinnaker.cats.cache;

import java.util.*;
import java.util.stream.Collectors;

/** A cache that provides a unified view of multiples, merging items from each cache together. */
public class CompositeCache implements Cache {
Expand All @@ -28,8 +27,9 @@ public CompositeCache(Collection<? extends Cache> caches) {
this.caches = caches;
}

public Set<StoreType> getStoreTypes() {
return caches.stream().map(c -> ((Cache) c).storeType()).collect(Collectors.toSet());
@Override
public boolean supportsGetAllByApplication() {
return caches.stream().allMatch(Cache::supportsGetAllByApplication);
}

@Override
Expand Down
Expand Up @@ -39,11 +39,6 @@ public class InMemoryCache implements WriteableCache {
private ConcurrentMap<String, ConcurrentMap<String, CacheData>> typeMap =
new ConcurrentHashMap<>();

@Override
public StoreType storeType() {
return StoreType.IN_MEMORY;
}

@Override
public void merge(String type, CacheData cacheData) {
merge(getOrCreate(type, cacheData.getId()), cacheData);
Expand Down
@@ -1,8 +1,6 @@
package com.netflix.spinnaker.cats.sql

import com.netflix.spinnaker.cats.agent.CacheResult
import com.netflix.spinnaker.cats.cache.Cache.StoreType
import com.netflix.spinnaker.cats.cache.Cache.StoreType.SQL
import com.netflix.spinnaker.cats.cache.CacheData
import com.netflix.spinnaker.cats.cache.CacheFilter
import com.netflix.spinnaker.cats.cache.DefaultCacheData
Expand Down Expand Up @@ -30,8 +28,6 @@ class SqlProviderCache(private val backingStore: WriteableCache) : ProviderCache
}
}

override fun storeType(): StoreType = SQL

/**
* Filters the supplied list of identifiers to only those that exist in the cache.
*
Expand Down Expand Up @@ -82,6 +78,10 @@ class SqlProviderCache(private val backingStore: WriteableCache) : ProviderCache
return backingStore.getAll(type, identifiers, cacheFilter)
}

override fun supportsGetAllByApplication(): Boolean {
return true
}

override fun getAllByApplication(type: String, application: String): Map<String, MutableCollection<CacheData>> {
return getAllByApplication(type, application, null)
}
Expand Down
@@ -1,8 +1,6 @@
package com.netflix.spinnaker.cats.sql.cache

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.cats.cache.Cache.StoreType
import com.netflix.spinnaker.cats.cache.Cache.StoreType.SQL
import com.netflix.spinnaker.cats.cache.CacheData
import com.netflix.spinnaker.cats.cache.CacheFilter
import com.netflix.spinnaker.cats.cache.DefaultCacheData
Expand Down Expand Up @@ -76,8 +74,6 @@ class SqlCache(
log.info("Configured for $name")
}

override fun storeType(): StoreType = SQL

/**
* Only evicts cache records but not relationship rows
*/
Expand Down Expand Up @@ -267,6 +263,10 @@ class SqlCache(
return getAll(type, ids)
}

override fun supportsGetAllByApplication(): Boolean {
return true
}

override fun getAllByApplication(
type: String,
application: String,
Expand Down
Expand Up @@ -21,7 +21,6 @@ import com.netflix.frigga.ami.AppVersion
import com.netflix.spinnaker.cats.cache.Cache
import com.netflix.spinnaker.cats.cache.CacheData
import com.netflix.spinnaker.cats.cache.CacheFilter
import com.netflix.spinnaker.cats.cache.CompositeCache
import com.netflix.spinnaker.cats.cache.RelationshipCacheFilter
import com.netflix.spinnaker.clouddriver.aws.AmazonCloudProvider
import com.netflix.spinnaker.clouddriver.aws.data.Keys
Expand All @@ -34,7 +33,6 @@ import org.springframework.beans.factory.annotation.Autowired
import org.springframework.beans.factory.annotation.Value
import org.springframework.stereotype.Component

import static com.netflix.spinnaker.cats.cache.Cache.StoreType.SQL
import static com.netflix.spinnaker.clouddriver.core.provider.agent.Namespace.*

@Component
Expand Down Expand Up @@ -259,12 +257,7 @@ class AmazonClusterProvider implements ClusterProvider<AmazonCluster>, ServerGro

Collection<AmazonCluster> clusters

// TODO: remove special casing for sql vs. redis; possibly via dropping redis support in the future
if ((includeDetails && sqlEnabled) &&
((cacheView instanceof CompositeCache &&
(cacheView as CompositeCache).getStoreTypes().every { (it == SQL) }) ||
(cacheView.storeType() == SQL))
) {
if (includeDetails && cacheView.supportsGetAllByApplication()) {
clusters = allClustersByApplication(applicationName)
} else {
clusters = translateClusters(resolveRelationshipData(application, CLUSTERS.ns), includeDetails)
Expand Down

0 comments on commit 757b6f6

Please sign in to comment.