From 415b25d41ab298d5c737d8893f63e8fcbd598d05 Mon Sep 17 00:00:00 2001
From: Mirco Nasuti <mirco.nasuti@chuv.ch>
Date: Fri, 11 Nov 2016 11:06:17 +0100
Subject: [PATCH] clean code + fix shared caching problem

---
 .../eu/hbp/mip/controllers/ArticlesApi.java   | 10 ++++---
 .../eu/hbp/mip/controllers/ExperimentApi.java | 17 +++++-------
 .../eu/hbp/mip/controllers/ModelsApi.java     | 26 +++++++++----------
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/src/main/java/eu/hbp/mip/controllers/ArticlesApi.java b/src/main/java/eu/hbp/mip/controllers/ArticlesApi.java
index 569f30eb8..71c3f1460 100644
--- a/src/main/java/eu/hbp/mip/controllers/ArticlesApi.java
+++ b/src/main/java/eu/hbp/mip/controllers/ArticlesApi.java
@@ -16,6 +16,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.cache.annotation.CacheEvict;
 import org.springframework.cache.annotation.CachePut;
 import org.springframework.cache.annotation.Cacheable;
+import org.springframework.context.annotation.Scope;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.*;
@@ -30,6 +31,7 @@ import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
 @RestController
 @RequestMapping(value = "/articles", produces = {APPLICATION_JSON_VALUE})
 @Api(value = "/articles", description = "the articles API")
+@Scope("session")
 public class ArticlesApi {
 
     private static final Logger LOGGER = Logger.getLogger(ArticlesApi.class);
@@ -41,7 +43,7 @@ public class ArticlesApi {
     private ArticleRepository articleRepository;
 
     @ApiOperation(value = "Get articles", response = Article.class, responseContainer = "List")
-    @Cacheable("Articles")
+    @Cacheable(value = "Articles", key = "(#own != null and #own).toString() + #status + #root.target")
     @RequestMapping(method = RequestMethod.GET)
     public ResponseEntity<Iterable> getArticles(
             @ApiParam(value = "Only ask own articles") @RequestParam(value = "own", required = false) Boolean own,
@@ -79,7 +81,7 @@ public class ArticlesApi {
 
     @ApiOperation(value = "Create an article")
     @ApiResponses(value = { @ApiResponse(code = 201, message = "Article created") })
-    @CachePut("Article")
+    @CachePut(value = "Article", key = "#article.getSlug() + #root.target")
     @CacheEvict(value = "Articles", allEntries = true)
     @RequestMapping(method = RequestMethod.POST)
     public ResponseEntity<Void> addAnArticle(
@@ -142,7 +144,7 @@ public class ArticlesApi {
 
 
     @ApiOperation(value = "Get an article", response = Article.class)
-    @Cacheable("Article")
+    @Cacheable(value = "Article", key = "#slug + #root.target")
     @RequestMapping(value = "/{slug}", method = RequestMethod.GET)
     public ResponseEntity<Article> getAnArticle(
             @ApiParam(value = "slug", required = true) @PathVariable("slug") String slug
@@ -170,7 +172,7 @@ public class ArticlesApi {
 
     @ApiOperation(value = "Update an article")
     @ApiResponses(value = { @ApiResponse(code = 204, message = "Article updated") })
-    @CachePut("Article")
+    @CachePut(value = "Article", key = "#slug + #root.target")
     @CacheEvict(value = "Articles",allEntries = true)
     @RequestMapping(value = "/{slug}", method = RequestMethod.PUT)
     public ResponseEntity<Void> updateAnArticle(
diff --git a/src/main/java/eu/hbp/mip/controllers/ExperimentApi.java b/src/main/java/eu/hbp/mip/controllers/ExperimentApi.java
index 34be9c610..6f8ea38f1 100644
--- a/src/main/java/eu/hbp/mip/controllers/ExperimentApi.java
+++ b/src/main/java/eu/hbp/mip/controllers/ExperimentApi.java
@@ -179,18 +179,18 @@ public class ExperimentApi {
     @ApiOperation(value = "list experiments", response = Experiment.class, responseContainer = "List")
     @RequestMapping(value = "/mine", method = RequestMethod.GET, params = {"maxResultCount"})
     public ResponseEntity<String> listExperiments(
-            @ApiParam(value = "maxResultCount", required = false) @RequestParam int maxResultCount
+            @ApiParam(value = "maxResultCount") @RequestParam int maxResultCount
     ) {
         LOGGER.info("List experiments");
 
-        return doListExperiments(true, maxResultCount, null);
+        return doListExperiments(true, null);
     }
 
     @ApiOperation(value = "list experiments", response = Experiment.class, responseContainer = "List")
     @RequestMapping(method = RequestMethod.GET, params = {"slug", "maxResultCount"})
     public ResponseEntity<String> listExperiments(
-            @ApiParam(value = "slug", required = false) @RequestParam("slug") String modelSlug,
-            @ApiParam(value = "maxResultCount", required = false) @RequestParam("maxResultCount") int maxResultCount
+            @ApiParam(value = "slug") @RequestParam("slug") String modelSlug,
+            @ApiParam(value = "maxResultCount") @RequestParam("maxResultCount") int maxResultCount
     ) {
         LOGGER.info("List experiments");
 
@@ -198,7 +198,7 @@ public class ExperimentApi {
             return new ResponseEntity<>("You must provide at least a slug or a limit of result", HttpStatus.BAD_REQUEST);
         }
 
-        return doListExperiments(false, maxResultCount, modelSlug);
+        return doListExperiments(false, modelSlug);
     }
 
     @ApiOperation(value = "List available methods and validations", response = String.class)
@@ -228,7 +228,6 @@ public class ExperimentApi {
 
     private ResponseEntity<String> doListExperiments(
             boolean mine,
-            int maxResultCount,
             String modelSlug
     ) {
         User user = securityConfiguration.getUser();
@@ -352,10 +351,8 @@ public class ExperimentApi {
         experiment.setResult(e.getMessage());
     }
 
-    private static boolean isExaremeAlgo(ExperimentQuery expQuery)  {
-        if (expQuery.getAlgorithms().size() < 1)
-            return false;
-        return "glm_exareme".equals(expQuery.getAlgorithms().get(0).getCode());
+    private static boolean isExaremeAlgo(ExperimentQuery expQuery) {
+        return expQuery.getAlgorithms().size() >= 1 && "glm_exareme".equals(expQuery.getAlgorithms().get(0).getCode());
     }
 
 }
diff --git a/src/main/java/eu/hbp/mip/controllers/ModelsApi.java b/src/main/java/eu/hbp/mip/controllers/ModelsApi.java
index de8fb4857..2df27612c 100644
--- a/src/main/java/eu/hbp/mip/controllers/ModelsApi.java
+++ b/src/main/java/eu/hbp/mip/controllers/ModelsApi.java
@@ -20,6 +20,7 @@ import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.cache.annotation.CacheEvict;
 import org.springframework.cache.annotation.CachePut;
 import org.springframework.cache.annotation.Cacheable;
+import org.springframework.context.annotation.Scope;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.*;
@@ -32,6 +33,7 @@ import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
 @RestController
 @RequestMapping(value = "/models", produces = {APPLICATION_JSON_VALUE})
 @Api(value = "/models", description = "the models API")
+@Scope("session")
 public class ModelsApi {
 
     private static final Logger LOGGER = Logger.getLogger(ModelsApi.class);
@@ -60,7 +62,7 @@ public class ModelsApi {
 
 
     @ApiOperation(value = "Get models", response = Model.class, responseContainer = "List")
-    @Cacheable("Models")
+    @Cacheable(value = "Models", key = "(#own != null and #own).toString() + (#valid != null and #valid).toString() + #root.target")
     @RequestMapping(method = RequestMethod.GET)
     public ResponseEntity<List> getModels(
             @ApiParam(value = "Only ask own models") @RequestParam(value = "own", required = false) Boolean own,
@@ -70,7 +72,7 @@ public class ModelsApi {
 
         User user = securityConfiguration.getUser();
 
-        Iterable<Model> models = null;
+        Iterable<Model> models;
         if(own != null && own)
         {
             models = modelRepository.findByCreatedByOrderByCreatedAt(user);
@@ -93,21 +95,19 @@ public class ModelsApi {
         }
 
         List<Object> modelsList = new LinkedList<>();
-        for (Iterator<Model> i = models.iterator(); i.hasNext(); )
-        {
-            Model m = i.next();
+        models = models != null ? models : new LinkedList<>();
+        for (Model m : models) {
             m.setDataset(datasetRepository.findOne(m.getDataset().getCode()));
             modelsList.add(getModelWithDataset(m));
         }
 
-        return new ResponseEntity<List<Model>>(HttpStatus.OK).ok(modelsList);
-
+        return ResponseEntity.ok(modelsList);
     }
 
 
     @ApiOperation(value = "Create a model", response = Model.class)
     @ApiResponses(value = { @ApiResponse(code = 201, message = "Model created") })
-    @CachePut("Model")
+    @CachePut(value = "Model", key = "#model.getSlug() + #root.target")
     @CacheEvict(value = "Models", allEntries = true)
     @RequestMapping(method = RequestMethod.POST)
     public ResponseEntity<Model> addAModel(
@@ -143,7 +143,7 @@ public class ModelsApi {
         }
 
         // Create slug from title
-        String slug = null;
+        String slug;
         try {
             slug = new Slugify().slugify(model.getTitle());
         } catch (IOException e) {
@@ -191,11 +191,11 @@ public class ModelsApi {
 
         LOGGER.info("Model saved (also saved model.config and model.query)");
 
-        return new ResponseEntity<Model>(HttpStatus.CREATED).ok(model);
+        return ResponseEntity.status(HttpStatus.CREATED).body(model);
     }
 
     @ApiOperation(value = "Get a model", response = Model.class)
-    @Cacheable("model")
+    @Cacheable(value = "model", key = "#slug + #root.target")
     @RequestMapping(value = "/{slug}", method = RequestMethod.GET)
     public ResponseEntity<Model> getAModel(
             @ApiParam(value = "slug", required = true) @PathVariable("slug") String slug
@@ -221,13 +221,13 @@ public class ModelsApi {
         Collection<String> yAxisVarsColl = new LinkedHashSet<>(yAxisVars);
         model.getConfig().setyAxisVariables(new LinkedList<>(yAxisVarsColl));
 
-        return new ResponseEntity<>(HttpStatus.OK).ok(getModelWithDataset(model));
+        return ResponseEntity.ok(getModelWithDataset(model));
     }
 
 
     @ApiOperation(value = "Update a model", response = Void.class)
     @ApiResponses(value = { @ApiResponse(code = 204, message = "Model updated") })
-    @CachePut("model")
+    @CachePut(value = "model", key = "#slug + #root.target")
     @CacheEvict(value = "Models", allEntries = true)
     @RequestMapping(value = "/{slug}", method = RequestMethod.PUT)
     public ResponseEntity<Void> updateAModel(
-- 
GitLab