From e391b5a495d979fbdb3502a0444d2d9e1341a64d Mon Sep 17 00:00:00 2001
From: Tor-Einar Skog <tor-einar.skog@bioforsk.no>
Date: Thu, 22 Aug 2013 15:47:42 +0200
Subject: [PATCH] Added sane discovery and handling of the situation where two
 or more models found on the classpath have the same modelId.

---
 .../vips/core/VIPSCoreApplication.java        |  1 +
 .../vips/core/service/ModelResourceImpl.java  | 32 +++++++---
 .../factory/DuplicateModelIdException.java    | 27 ++++++++
 .../vips/model/factory/ModelFactory.java      | 62 +++++++++++++++++--
 4 files changed, 110 insertions(+), 12 deletions(-)
 create mode 100644 src/main/java/no/bioforsk/vips/model/factory/DuplicateModelIdException.java

diff --git a/src/main/java/no/bioforsk/vips/core/VIPSCoreApplication.java b/src/main/java/no/bioforsk/vips/core/VIPSCoreApplication.java
index d6b678b..41f57cb 100644
--- a/src/main/java/no/bioforsk/vips/core/VIPSCoreApplication.java
+++ b/src/main/java/no/bioforsk/vips/core/VIPSCoreApplication.java
@@ -37,5 +37,6 @@ public class VIPSCoreApplication extends Application
      */
     private void addRestResourceClasses(Set<Class<?>> resources) {
         resources.add(no.bioforsk.vips.core.config.JacksonConfig.class);
+        resources.add(no.bioforsk.vips.core.service.ModelResource.class);
     }
 }
\ No newline at end of file
diff --git a/src/main/java/no/bioforsk/vips/core/service/ModelResourceImpl.java b/src/main/java/no/bioforsk/vips/core/service/ModelResourceImpl.java
index c05613b..1e84ea0 100644
--- a/src/main/java/no/bioforsk/vips/core/service/ModelResourceImpl.java
+++ b/src/main/java/no/bioforsk/vips/core/service/ModelResourceImpl.java
@@ -9,6 +9,7 @@ import no.bioforsk.vips.entity.ModelConfiguration;
 import no.bioforsk.vips.model.ConfigValidationException;
 import no.bioforsk.vips.model.Model;
 import no.bioforsk.vips.model.ModelExcecutionException;
+import no.bioforsk.vips.model.factory.DuplicateModelIdException;
 import no.bioforsk.vips.model.factory.ModelFactory;
 
 /**
@@ -62,9 +63,17 @@ public class ModelResourceImpl implements ModelResource{
             try {
                 if(first) first=false;
                 else retVal.append(",");
-                retVal.append("{");
-                retVal.append("\"modelId\":\"").append(key).append("\", \"modelName\":\"").append(mF.getModelInstance(key).getModelName(language)).append("\"");
-                retVal.append("}");
+                retVal.append("{")
+                    .append("\"modelId\":\"").append(key).append("\", \"modelName\":\"");
+                try
+                { 
+                    retVal.append(mF.getModelInstance(key).getModelName(language));
+                }
+                catch(DuplicateModelIdException ex)
+                {
+                    retVal.append(ex.getMessage());
+                }
+                retVal.append("\"").append("}");
                 
             } catch (InstantiationException | IllegalAccessException ex) {
                 Logger.getLogger(ModelResourceImpl.class.getName()).log(Level.SEVERE, null, ex);
@@ -92,7 +101,8 @@ public class ModelResourceImpl implements ModelResource{
         {
             try {
                 retVal.append(key).append(" ").append(mF.getModelInstance(key).getModelName(language)).append("\n");
-            } catch (InstantiationException | IllegalAccessException ex) {
+            } catch (InstantiationException | IllegalAccessException | DuplicateModelIdException ex) {
+                retVal.append(key).append(" ").append(ex.getMessage()).append("\n");
                 Logger.getLogger(ModelResourceImpl.class.getName()).log(Level.SEVERE, null, ex);
             }
             
@@ -154,7 +164,15 @@ public class ModelResourceImpl implements ModelResource{
     public Response printModelSampleConfig(@PathParam("modelId") String modelId)
     {
         try {
-            String sampleConfig = ModelFactory.getInstance().getModelInstance(modelId).getSampleConfig();
+            String sampleConfig;
+            try
+            {
+                 sampleConfig = ModelFactory.getInstance().getModelInstance(modelId).getSampleConfig();
+            }
+            catch(DuplicateModelIdException ex)
+            {
+                sampleConfig = ex.getMessage();
+            }
             return Response.ok().entity(sampleConfig).build();
         } catch (InstantiationException | IllegalAccessException ex) {
             Logger.getLogger(ModelResourceImpl.class.getName()).log(Level.SEVERE, null, ex);
@@ -180,7 +198,7 @@ public class ModelResourceImpl implements ModelResource{
             calledModel.setConfiguration(config);
             return Response.ok().entity(calledModel.getResult()).build();
         }
-        catch(InstantiationException | IllegalAccessException | ConfigValidationException | ModelExcecutionException ex)
+        catch(InstantiationException | IllegalAccessException | ConfigValidationException | ModelExcecutionException | DuplicateModelIdException ex)
         {
             Logger.getLogger(ModelResourceImpl.class.getName()).log(Level.SEVERE, null, ex);
             return Response.serverError().entity(ex.getMessage()).build();
@@ -206,7 +224,7 @@ public class ModelResourceImpl implements ModelResource{
             calledModel.setConfiguration(config);
             return Response.ok().entity(calledModel.getResult()).build();
         }
-        catch(InstantiationException | IllegalAccessException | ConfigValidationException | ModelExcecutionException ex)
+        catch(InstantiationException | IllegalAccessException | ConfigValidationException | ModelExcecutionException | DuplicateModelIdException ex)
         {
             Logger.getLogger(ModelResourceImpl.class.getName()).log(Level.SEVERE, null, ex);
             if(ex instanceof ConfigValidationException){
diff --git a/src/main/java/no/bioforsk/vips/model/factory/DuplicateModelIdException.java b/src/main/java/no/bioforsk/vips/model/factory/DuplicateModelIdException.java
new file mode 100644
index 0000000..e353f87
--- /dev/null
+++ b/src/main/java/no/bioforsk/vips/model/factory/DuplicateModelIdException.java
@@ -0,0 +1,27 @@
+package no.bioforsk.vips.model.factory;
+
+/**
+ * Thrown if ModelFactory discovers that the system contains different models
+ * with identical modelIds
+ * @copyright 2013 <a href="http://www.bioforsk.no/">Bioforsk</a>
+ * @author Tor-Einar Skog <tor-einar.skog@bioforsk.no>
+ */
+public class DuplicateModelIdException extends Exception {
+
+    /**
+     * Creates a new instance of
+     * <code>DuplicateModelIdException</code> without detail message.
+     */
+    public DuplicateModelIdException() {
+    }
+
+    /**
+     * Constructs an instance of
+     * <code>DuplicateModelIdException</code> with the specified detail message.
+     *
+     * @param msg the detail message.
+     */
+    public DuplicateModelIdException(String msg) {
+        super(msg);
+    }
+}
diff --git a/src/main/java/no/bioforsk/vips/model/factory/ModelFactory.java b/src/main/java/no/bioforsk/vips/model/factory/ModelFactory.java
index f069882..cbdf097 100644
--- a/src/main/java/no/bioforsk/vips/model/factory/ModelFactory.java
+++ b/src/main/java/no/bioforsk/vips/model/factory/ModelFactory.java
@@ -8,6 +8,7 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.reflections.Reflections;
 import no.bioforsk.vips.model.Model;
+import no.bioforsk.vips.model.ModelId;
 
 /**
  * Provides the models available. Singleton.
@@ -34,6 +35,9 @@ public class ModelFactory {
     private Map<String,Model> models;
     // Internal list of model IDs and names
     private Map<String,String> modelList;
+    // If duplicate ids found, put them here
+    private Set<String> duplicateModelIds;
+    private final static String DUPLICATE_MODEL_ID_WARNING = "ERROR: DUPLICATE MODELS EXIST FOR MODEL WITH THIS ID. CHECK SERVER LOGS.";
     
     /**
      * Declared private to protect from unauthorized instatiation. Part of the
@@ -123,9 +127,23 @@ public class ModelFactory {
                try {
                    
                    Model model = subType.newInstance();
-                   models.put(model.getModelId().toString(), model);
+                   if(models.get(model.getModelId().toString()) == null)
+                   {
+                        models.put(model.getModelId().toString(), model);
+                   }
+                   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());
+                   }
                    //System.out.println("Model " + model.getModelName() + " with id=" + model.getModelId().toString() + " was found");
-               } catch (    InstantiationException | IllegalAccessException ex) {
+               } catch (    InstantiationException | IllegalAccessException | DuplicateModelIdException ex) {
                    Logger.getLogger(ModelFactory.class.getName()).log(Level.SEVERE, null, ex);
                }
         }
@@ -142,7 +160,8 @@ public class ModelFactory {
             this.modelList = new HashMap();
             for(Model model:this.models.values())
             {
-                this.modelList.put(model.getModelId().toString(), model.getModelName());
+                String modelName = this.isDuplicateModelId(model.getModelId().toString()) ? this.getDuplicateModelIdWarning() : model.getModelName();
+                this.modelList.put(model.getModelId().toString(), modelName);
             }
         }
         return this.modelList;
@@ -155,6 +174,10 @@ public class ModelFactory {
      * @return Description of model in requested language (Default/fallback is English)
      */
     public String getModelDescription(String modelId, String language) {
+        if(this.isDuplicateModelId(modelId))
+        {
+            return this.getDuplicateModelIdWarning();
+        }
         return language != null ? this.models.get(modelId).getModelDescription(language) : this.models.get(modelId).getModelDescription();
     }
     
@@ -165,6 +188,10 @@ public class ModelFactory {
      */
     public String getModelUsage(String modelId, String language)
     {
+        if(this.isDuplicateModelId(modelId))
+        {
+            return this.getDuplicateModelIdWarning();
+        }
         return language != null ? this.models.get(modelId).getModelUsage(language) : this.models.get(modelId).getModelUsage();
     }
     
@@ -175,11 +202,36 @@ public class ModelFactory {
      * @throws InstantiationException
      * @throws IllegalAccessException 
      */
-    public Model getModelInstance(String modelId) throws InstantiationException, IllegalAccessException
+    public Model getModelInstance(String modelId) throws InstantiationException, IllegalAccessException, DuplicateModelIdException
     {
-            return this.models.get(modelId).getClass().newInstance();
+        if(this.isDuplicateModelId(modelId))
+        {
+            throw new DuplicateModelIdException(this.getDuplicateModelIdWarning());
+        }
+        return this.models.get(modelId).getClass().newInstance();
     }
 
+    private void addDuplicateModelId(String modelId) {
+        if(this.duplicateModelIds == null)
+        {
+            this.duplicateModelIds = new HashSet();
+        }
+        this.duplicateModelIds.add(modelId);
+    }
+
+    private Set<String> getDuplicateModelIds()
+    {
+        return this.duplicateModelIds;
+    }
+    
+    private boolean isDuplicateModelId(String modelId)
+    {
+        return this.duplicateModelIds == null ? false : this.duplicateModelIds.contains(modelId);
+    }
+
+    private String getDuplicateModelIdWarning() {
+        return DUPLICATE_MODEL_ID_WARNING;
+    }
     
     
     
-- 
GitLab