Completed and tested alias cache. See #73

basilapi · Oct 11, 2018 · 23d18b8 · 23d18b8
1 parent eb1da52
commit 23d18b8
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 27 deletions.
diff --git a/basil/src/main/java/uk/ac/open/kmi/basil/alias/AliasMemCache.java b/basil/src/main/java/uk/ac/open/kmi/basil/alias/AliasMemCache.java
@@ -1,8 +1,10 @@
 package uk.ac.open.kmi.basil.alias;
 
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -11,6 +13,7 @@ public class AliasMemCache implements AliasCache {
 	private Map<String, String> cache;
 	private int limit;
 	private static Logger log = LoggerFactory.getLogger(AliasMemCache.class);
+
 	public AliasMemCache() {
 		this(1000);
 	}
@@ -22,7 +25,7 @@ public AliasMemCache(int size) {
 
 			@Override
 			protected boolean removeEldestEntry(Map.Entry<String, String> eldest) {
-				if(log.isDebugEnabled() && size() > limit) {
+				if (log.isDebugEnabled() && size() > limit) {
 					log.debug("removing older cache entry");
 					return true;
 				}
@@ -69,10 +72,13 @@ public String getId(String alias) {
 	 */
 	@Override
 	public void removeAll(String id) {
+		Set<String> remove = new HashSet<String>();
 		for (Entry<String, String> entry : cache.entrySet()) {
 			if (entry.getValue().equals(id)) {
-				cache.remove(entry.getKey());
+				remove.add(entry.getKey());
 			}
 		}
+		for (String key : remove)
+			cache.remove(key);
 	}
 }
diff --git a/basil/src/main/java/uk/ac/open/kmi/basil/mysql/MySQLStore.java b/basil/src/main/java/uk/ac/open/kmi/basil/mysql/MySQLStore.java
@@ -699,7 +699,7 @@ public Set<String> alias() {
 	public String getIdByAlias(String alias) throws IOException {
 		log.trace("get id by alias: {}", alias);
 		try {
-			String q = "SELECT API.NICKNAME FROM ALIAS INNER JOIN APIS ON ALIAS.API = APIS.ID WHERE ALIAS = ? LIMIT 1";
+			String q = "SELECT APIS.NICKNAME FROM ALIAS INNER JOIN APIS ON ALIAS.API = APIS.ID WHERE ALIAS.ALIAS = ? LIMIT 1";
 			Class.forName("com.mysql.jdbc.Driver");
 			try (Connection connect = DriverManager.getConnection(jdbcUri)) {
 				try (PreparedStatement stmt = connect.prepareStatement(q)) {

diff --git a/basil/src/main/java/uk/ac/open/kmi/basil/rest/BasilApplication.java b/basil/src/main/java/uk/ac/open/kmi/basil/rest/BasilApplication.java
@@ -15,6 +15,7 @@
 import com.wordnik.swagger.jersey.listing.JerseyApiDeclarationProvider;
 import com.wordnik.swagger.jersey.listing.JerseyResourceListingProvider;
 
+import uk.ac.open.kmi.basil.alias.AliasMemCache;
 import uk.ac.open.kmi.basil.core.auth.JDBCUserManager;
 import uk.ac.open.kmi.basil.invoke.DirectExecutor;
 import uk.ac.open.kmi.basil.invoke.QueryExecutor;
@@ -60,6 +61,7 @@ public void contextInitialized(ServletContextEvent arg0) {
 		MySQLStore store = new MySQLStore(environment.getJdbcConnectionUrl());
 		ctx.setAttribute(Registry.Store, store);
 		ctx.setAttribute(Registry.SearchProvider, store);
+		ctx.setAttribute(Registry.AliasCache, new AliasMemCache(10000));
 		QueryExecutor exec;
 		try {
 			exec = environment.getQueryExecutorClass().newInstance();
@@ -79,5 +81,6 @@ public final static class Registry {
 		public final static String UserManager = "_UserManager";
 		public final static String JdbcUri = "_JdbcUri";
 		public final static String SearchProvider = "_SearchProvider";
+		public static final String AliasCache = "_AliasCache";
 	}
 }
diff --git a/basil/src/main/java/uk/ac/open/kmi/basil/rest/core/AbstractResource.java b/basil/src/main/java/uk/ac/open/kmi/basil/rest/core/AbstractResource.java
@@ -52,34 +52,25 @@ protected ApiManager getApiManager() {
 		return apiManager;
 	}
 
-	protected AliasCache getAliasCache() {
-		if (aliasCache == null) {
-			// XXX That't the only implementation at the moment.
-			aliasCache = new AliasMemCache();
-		}
-		return aliasCache;
-	}
-
 	protected String getApiId(String idOrAlias) throws IOException {
 		// Try alias in cache
 		if(getAliasCache().containsAlias(idOrAlias)) {
+			log.trace("alias from cache: {}", idOrAlias);
 			return getAliasCache().getId(idOrAlias);
 		}
 		// Try id
-		try {
-			if(getApiManager().existsSpec(idOrAlias)) {
-				return idOrAlias;
-			}else {
-				// Try Alias
-				String id = getApiManager().byAlias(idOrAlias);
-				// Save in cache
-				getAliasCache().set(id, idOrAlias);
-				return id;
-			}
-		} catch (IOException e) {
-
+
+		if(getApiManager().existsSpec(idOrAlias)) {
+			log.trace("id is id: {}", idOrAlias);
+			return idOrAlias;
+		}else {
+			// Try Alias
+			String id = getApiManager().byAlias(idOrAlias);
+			log.trace("id from alias: {}={}", idOrAlias, id);
+			// Save in cache
+			getAliasCache().set(id, idOrAlias);
+			return id;
 		}
-		throw new IOException("This should never happen");
 	}
 
 	protected boolean isAuthenticated(){
@@ -94,6 +85,14 @@ protected QueryExecutor getQueryExecutor() {
 		return (QueryExecutor) context.getAttribute(BasilApplication.Registry.QueryExecutor);
 	}
 
+	protected AliasCache getAliasCache() {
+		if (aliasCache == null) {
+			// XXX That't the only implementation at the moment.
+			aliasCache = (AliasCache) context.getAttribute(BasilApplication.Registry.AliasCache);
+		}
+		return aliasCache;
+	}
+
 	protected final ResponseBuilder packError(ResponseBuilder builder, String message){
 		return builder.header(Headers.Error, message).entity(new ErrorMessage(message).asJSON());
 	}

diff --git a/basil/src/main/java/uk/ac/open/kmi/basil/rest/core/AliasResource.java b/basil/src/main/java/uk/ac/open/kmi/basil/rest/core/AliasResource.java
@@ -100,9 +100,6 @@ public Response delete(@PathParam("id") String id, @Auth Subject subject) {
 			}
 
 			subject.checkRole(id); // is the creator
-			if (getApiManager().getSpecification(id) == null) {
-				return Response.status(404).build();
-			}
 			boolean success = getApiManager().deleteAlias(id);
 			if(success) {
 				// Clear alias cache