From ea89dd2cfce00a33fdcad009202ef8b87ea72f4b Mon Sep 17 00:00:00 2001
From: Tor-Einar Skog <tor-einar.skog@bioforsk.no>
Date: Mon, 24 Aug 2015 19:16:03 +0200
Subject: [PATCH] Giving sensible error message (BAD REQUEST) when requesting
 non-existing modelId

---
 .../nibio/vips/core/VIPSCoreApplication.java  |  1 +
 .../vips/core/service/ModelResourceImpl.java  | 77 +++++++++++++++----
 .../vips/model/factory/ModelFactory.java      | 58 ++++++++++----
 .../model/factory/NoSuchModelException.java   | 42 ++++++++++
 4 files changed, 147 insertions(+), 31 deletions(-)
 create mode 100644 src/main/java/no/nibio/vips/model/factory/NoSuchModelException.java

diff --git a/src/main/java/no/nibio/vips/core/VIPSCoreApplication.java b/src/main/java/no/nibio/vips/core/VIPSCoreApplication.java
index 0597d0c..cdcf561 100644
--- a/src/main/java/no/nibio/vips/core/VIPSCoreApplication.java
+++ b/src/main/java/no/nibio/vips/core/VIPSCoreApplication.java
@@ -56,5 +56,6 @@ public class VIPSCoreApplication extends Application
      */
     private void addRestResourceClasses(Set<Class<?>> resources) {
         resources.add(no.nibio.vips.core.config.JacksonConfig.class);
+        resources.add(no.nibio.vips.core.service.ModelResourceImpl.class);
     }
 }
\ No newline at end of file
diff --git a/src/main/java/no/nibio/vips/core/service/ModelResourceImpl.java b/src/main/java/no/nibio/vips/core/service/ModelResourceImpl.java
index 402596c..cd51810 100644
--- a/src/main/java/no/nibio/vips/core/service/ModelResourceImpl.java
+++ b/src/main/java/no/nibio/vips/core/service/ModelResourceImpl.java
@@ -19,7 +19,6 @@
 
 package no.nibio.vips.core.service;
 
-import no.nibio.vips.core.service.ModelResource;
 import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -38,6 +37,7 @@ import no.nibio.vips.model.Model;
 import no.nibio.vips.model.ModelExcecutionException;
 import no.nibio.vips.model.factory.DuplicateModelIdException;
 import no.nibio.vips.model.factory.ModelFactory;
+import no.nibio.vips.model.factory.NoSuchModelException;
 
 /**
  * The available resources in this system
@@ -119,7 +119,7 @@ public class ModelResourceImpl implements ModelResource{
                 }
                 retVal.append("\"").append("}");
                 
-            } catch (InstantiationException | IllegalAccessException ex) {
+            } catch (InstantiationException | NoSuchModelException | IllegalAccessException ex) {
                 Logger.getLogger(ModelResourceImpl.class.getName()).log(Level.SEVERE, null, ex);
             }
             
@@ -145,7 +145,7 @@ public class ModelResourceImpl implements ModelResource{
         {
             try {
                 retVal.append(key).append(" ").append(mF.getModelInstance(key).getModelName(language)).append("\n");
-            } catch (InstantiationException | IllegalAccessException | DuplicateModelIdException ex) {
+            } catch (InstantiationException | IllegalAccessException | NoSuchModelException | DuplicateModelIdException ex) {
                 retVal.append(key).append(" ").append(ex.getMessage()).append("\n");
                 Logger.getLogger(ModelResourceImpl.class.getName()).log(Level.SEVERE, null, ex);
             }
@@ -161,8 +161,15 @@ public class ModelResourceImpl implements ModelResource{
 
     @Override
     public Response printModelDescription(String modelId, String language) {
-        String description = ModelFactory.getInstance().getModelDescription(modelId,language);
-        return Response.ok().entity(description).build();
+        try
+        {
+            String description = ModelFactory.getInstance().getModelDescription(modelId,language);
+            return Response.ok().entity(description).build();
+        }
+        catch(NoSuchModelException ex)
+        {
+            return Response.status(Response.Status.BAD_REQUEST).entity(ex.getMessage()).build();
+        }
     }
   
     /**
@@ -177,8 +184,15 @@ public class ModelResourceImpl implements ModelResource{
     @Override
     public Response printModelUsage(@PathParam("modelId") String modelId, @PathParam("language") String language)
     {
-        String usage = ModelFactory.getInstance().getModelUsage(modelId, language);
-        return Response.ok().entity(usage).build();
+        try
+        {
+            String usage = ModelFactory.getInstance().getModelUsage(modelId, language);
+            return Response.ok().entity(usage).build();
+        }
+        catch(NoSuchModelException ex)
+        {
+            return Response.status(Response.Status.BAD_REQUEST).entity(ex.getMessage()).build();
+        }
     }
     
     /**
@@ -213,7 +227,7 @@ public class ModelResourceImpl implements ModelResource{
             {
                  sampleConfig = ModelFactory.getInstance().getModelInstance(modelId).getSampleConfig();
             }
-            catch(DuplicateModelIdException ex)
+            catch(NoSuchModelException | DuplicateModelIdException ex)
             {
                 sampleConfig = ex.getMessage();
             }
@@ -238,7 +252,7 @@ public class ModelResourceImpl implements ModelResource{
             calledModel.setConfiguration(config);
             return Response.ok().entity(calledModel.getResult()).build();
         }
-        catch(InstantiationException | IllegalAccessException | ConfigValidationException | ModelExcecutionException | DuplicateModelIdException ex)
+        catch(InstantiationException | IllegalAccessException | ConfigValidationException | ModelExcecutionException | NoSuchModelException | DuplicateModelIdException ex)
         {
             Logger.getLogger(ModelResourceImpl.class.getName()).log(Level.SEVERE, null, ex);
             return Response.serverError().entity(ex.getMessage()).build();
@@ -282,20 +296,42 @@ public class ModelResourceImpl implements ModelResource{
 
     @Override
     public Response printModelName(String modelId, String language) {
-        String name = ModelFactory.getInstance().getModelName(modelId, language);
-        return Response.ok().entity(name).build();
+        try
+        {
+            String name = ModelFactory.getInstance().getModelName(modelId, language);
+            return Response.ok().entity(name).build();
+        }
+        catch(NoSuchModelException ex)
+        {
+            return Response.status(Response.Status.BAD_REQUEST).entity(ex.getMessage()).build();
+        }
+        
     }
 
     @Override
     public Response printModelLicense(String modelId) {
-        String license = ModelFactory.getInstance().getModelLicense(modelId);
-        return Response.ok().entity(license).build();
+        try
+        {
+            String license = ModelFactory.getInstance().getModelLicense(modelId);
+            return Response.ok().entity(license).build();
+        }
+        catch(NoSuchModelException ex)
+        {
+            return Response.status(Response.Status.BAD_REQUEST).entity(ex.getMessage()).build();
+        }
     }
     
     @Override
     public Response printModelCopyright(String modelId) {
-        String copyright = ModelFactory.getInstance().getModelCopyright(modelId);
-        return Response.ok().entity(copyright).build();
+        try
+        {
+            String copyright = ModelFactory.getInstance().getModelCopyright(modelId);
+            return Response.ok().entity(copyright).build();
+        }
+        catch(NoSuchModelException ex)
+        {
+            return Response.status(Response.Status.BAD_REQUEST).entity(ex.getMessage()).build();
+        }
     }
 
     @Override
@@ -305,8 +341,15 @@ public class ModelResourceImpl implements ModelResource{
 
     @Override
     public Response printModelWarningStatusInterpretation(String modelId, String language) {
-        String interpretation = ModelFactory.getInstance().getModelWarningStatusInterpretation(modelId,language);
-        return Response.ok().entity(interpretation).build();
+        try
+        {
+            String interpretation = ModelFactory.getInstance().getModelWarningStatusInterpretation(modelId,language);
+            return Response.ok().entity(interpretation).build();
+        }
+        catch(NoSuchModelException ex)
+        {
+            return Response.status(Response.Status.BAD_REQUEST).entity(ex.getMessage()).build();
+        }
     }
 
     
diff --git a/src/main/java/no/nibio/vips/model/factory/ModelFactory.java b/src/main/java/no/nibio/vips/model/factory/ModelFactory.java
index a0e1c8f..f78c1f4 100644
--- a/src/main/java/no/nibio/vips/model/factory/ModelFactory.java
+++ b/src/main/java/no/nibio/vips/model/factory/ModelFactory.java
@@ -151,13 +151,11 @@ public class ModelFactory {
                    else
                    {
                        this.addDuplicateModelId(model.getModelId().toString());
-                       StringBuilder message = new StringBuilder()
-                               .append("Duplicate model Ids found for at least two models: ")
-                               .append("ModelId \"").append(model.getModelId()).append("\" is assigned to ")
-                               .append(model.getClass().getName())
-                               .append(" and ")
-                               .append(models.get(model.getModelId().toString()).getClass().getName());
-                       throw new DuplicateModelIdException(message.toString());
+                       String message = "Duplicate model Ids found for at least two models: ModelId \"" 
+                               + model.getModelId() + "\" is assigned to " 
+                               + model.getClass().getName() + " and " 
+                               + models.get(model.getModelId().toString()).getClass().getName();
+                       throw new DuplicateModelIdException(message);
                    }
                    //System.out.println("Model " + model.getModelName() + " with id=" + model.getModelId().toString() + " was found");
                } catch (    InstantiationException | IllegalAccessException | DuplicateModelIdException ex) {
@@ -190,12 +188,16 @@ public class ModelFactory {
      * @param language
      * @return 
      */
-    public String getModelName(String modelId, String language)
+    public String getModelName(String modelId, String language) throws NoSuchModelException
     {
         if(this.isDuplicateModelId(modelId))
         {
             return this.getDuplicateModelIdWarning();
         }
+        if(this.models.get(modelId) == null)
+        {
+            throw new NoSuchModelException("ERROR: No model found with id = " + modelId);
+        }
         return language != null ? this.models.get(modelId).getModelName(language) : this.models.get(modelId).getModelName();
     }
     
@@ -204,12 +206,16 @@ public class ModelFactory {
      * @param modelId
      * @return The license for the requested model
      */
-    public String getModelLicense(String modelId)
+    public String getModelLicense(String modelId) throws NoSuchModelException
     {
         if(this.isDuplicateModelId(modelId))
         {
             return this.getDuplicateModelIdWarning();
         }
+        if(this.models.get(modelId) == null)
+        {
+            throw new NoSuchModelException("ERROR: No model found with id = " + modelId);
+        }
         return this.models.get(modelId).getLicense();
     }
     
@@ -218,12 +224,16 @@ public class ModelFactory {
      * @param modelId
      * @return the copyright information for the requested model
      */
-    public String getModelCopyright(String modelId)
+    public String getModelCopyright(String modelId) throws NoSuchModelException
     {
         if(this.isDuplicateModelId(modelId))
         {
             return this.getDuplicateModelIdWarning();
         }
+        if(this.models.get(modelId) == null)
+        {
+            throw new NoSuchModelException("ERROR: No model found with id = " + modelId);
+        }
         return this.models.get(modelId).getCopyright();
     }
     
@@ -233,11 +243,15 @@ public class ModelFactory {
      * @param language language two-letter code (<a href="http://www.loc.gov/standards/iso639-2/php/English_list.php">ISO-639-2</a>)
      * @return Description of model in requested language (Default/fallback is English)
      */
-    public String getModelDescription(String modelId, String language) {
+    public String getModelDescription(String modelId, String language) throws NoSuchModelException {
         if(this.isDuplicateModelId(modelId))
         {
             return this.getDuplicateModelIdWarning();
         }
+        if(this.models.get(modelId) == null)
+        {
+            throw new NoSuchModelException("ERROR: No model found with id = " + modelId);
+        }
         return language != null ? this.models.get(modelId).getModelDescription(language) : this.models.get(modelId).getModelDescription();
     }
     
@@ -246,12 +260,16 @@ public class ModelFactory {
      * @return usage instructions for the requested Model
      * 
      */
-    public String getModelUsage(String modelId, String language)
+    public String getModelUsage(String modelId, String language) throws NoSuchModelException
     {
         if(this.isDuplicateModelId(modelId))
         {
             return this.getDuplicateModelIdWarning();
         }
+        if(this.models.get(modelId) == null)
+        {
+            throw new NoSuchModelException("ERROR: No model found with id = " + modelId);
+        }
         return language != null ? this.models.get(modelId).getModelUsage(language) : this.models.get(modelId).getModelUsage();
     }
     
@@ -262,12 +280,20 @@ public class ModelFactory {
      * @throws InstantiationException
      * @throws IllegalAccessException 
      */
-    public Model getModelInstance(String modelId) throws InstantiationException, IllegalAccessException, DuplicateModelIdException
+    public Model getModelInstance(String modelId) throws 
+                InstantiationException, 
+                IllegalAccessException, 
+                DuplicateModelIdException, 
+                NoSuchModelException
     {
         if(this.isDuplicateModelId(modelId))
         {
             throw new DuplicateModelIdException(this.getDuplicateModelIdWarning());
         }
+        if(this.models.get(modelId) == null)
+        {
+            throw new NoSuchModelException("ERROR: No model found with id = " + modelId);
+        }
         return this.models.get(modelId).getClass().newInstance();
     }
 
@@ -293,11 +319,15 @@ public class ModelFactory {
         return DUPLICATE_MODEL_ID_WARNING;
     }
 
-    public String getModelWarningStatusInterpretation(String modelId, String language) {
+    public String getModelWarningStatusInterpretation(String modelId, String language) throws NoSuchModelException {
         if(this.isDuplicateModelId(modelId))
         {
             return this.getDuplicateModelIdWarning();
         }
+        if(this.models.get(modelId) == null)
+        {
+            throw new NoSuchModelException("ERROR: No model found with id = " + modelId);
+        }
         return language != null ? this.models.get(modelId).getWarningStatusInterpretation(language) : this.models.get(modelId).getWarningStatusInterpretation();
     }
     
diff --git a/src/main/java/no/nibio/vips/model/factory/NoSuchModelException.java b/src/main/java/no/nibio/vips/model/factory/NoSuchModelException.java
new file mode 100644
index 0000000..5008f5a
--- /dev/null
+++ b/src/main/java/no/nibio/vips/model/factory/NoSuchModelException.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2015 NIBIO <http://www.nibio.no/>. 
+ * 
+ * This file is part of VIPSCore.
+ * VIPSCore is free software: you can redistribute it and/or modify
+ * it under the terms of the NIBIO Open Source License as published by 
+ * NIBIO, either version 1 of the License, or (at your option) any
+ * later version.
+ * 
+ * VIPSCore is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * NIBIO Open Source License for more details.
+ * 
+ * You should have received a copy of the NIBIO Open Source License
+ * along with VIPSCore.  If not, see <http://www.nibio.no/licenses/>.
+ * 
+ */
+
+package no.nibio.vips.model.factory;
+
+/**
+ * @copyright 2015 <a href="http://www.bioforsk.no/">Bioforsk</a>
+ * @author Tor-Einar Skog <tor-einar.skog@bioforsk.no>
+ */
+public class NoSuchModelException extends Exception {
+
+    /**
+     * Creates a new instance of <code>NoSuchModelException</code> without detail message.
+     */
+    public NoSuchModelException() {
+    }
+
+
+    /**
+     * Constructs an instance of <code>NoSuchModelException</code> with the specified detail message.
+     * @param msg the detail message.
+     */
+    public NoSuchModelException(String msg) {
+        super(msg);
+    }
+}
-- 
GitLab