Changeset 3596

Timestamp:

Jul 24, 2007, 2:14:09 PM (16 years ago)

Author:

Nicklas Nordborg

Message:

Fixes #654: Batch deletion of reporters

Location:

trunk

Files:

• doc/src/docbook/userdoc/reporters.xml (modified) (1 diff)
• src/core/net/sf/basedb/core/Install.java (modified) (1 diff)
• src/core/net/sf/basedb/core/ItemInUseException.java (modified) (1 diff)
• src/plugins/core/net/sf/basedb/plugins/ReporterFlatFileImporter.java (modified) (18 diffs)
• src/test/TestReporterFlatFileImporter.java (modified) (6 diffs)
• src/test/TestReporterMapFlatFileImporter.java (modified) (1 diff)

trunk/doc/src/docbook/userdoc/reporters.xml

@@ -322 +322 @@
       
       <para>
-        Reporters which has been referenced to from raw data, array
+        Reporters which has been referenced to from reporter lists, raw data, array
         designs, plates or any other item cannot be deleted.
       </para>
+      
+      <sect3 id="reporter.delete.batch">
+        <title>Batch deletion</title>
+        <para>
+          A common problem is to delete reporters that has been accidentaly
+          created. The regular web interface is usually no good since it
+          only allows you to select at most 99 reporters at a time. To solve
+          this problem the reporter import plug-in can be used in delete mode.
+          You can use the same file as you used when importing. Just select
+          the <userinput>delete</userinput> option for the <guilabel>mode</guilabel>
+          parameter in the configuration wizard and continue as usual. 
+          If the plug-in is used in delete mode from a reporter list it will
+          only remove the reporters from the reporter list. The reporters are not
+          deleted from the database.
+        </para>
+        
+        <note>
+          <para>
+          It may be a bit confusing to delete things from an import plug-in. But
+          since plug-ins can only belong to one category and we wanted to re-use
+          existing file format definitions this was our only option.
+          </para>
+        </note>
+      </sect3>
+      
     </sect2>
 

trunk/src/core/net/sf/basedb/core/Install.java

@@ -1923 +1923 @@
       {
         int version = schemaVersion.getSchemaVersion();
-        pd.loadPluginInformation(jarPath, className, version < 20);
+        pd.loadPluginInformation(jarPath, className, true);
         if (version < 21) pd.setAllowImmediateExecution(allowImmediateExecution);
         dc.commit();

trunk/src/core/net/sf/basedb/core/ItemInUseException.java

@@ -48 +48 @@
     super("The item "+what+" is used by another item.");
   }
+  
+  /**
+    Creates a new <code>ItemInUseException</code> with a cause. The error
+    message produced will look like:
+    <code>Permission denied. The item User[ID=325] is used by another item.</code>
+  
+    @param what A description of what already exists, for 
+      example User[Id=325]
+    @param cause The cause of the error
+    @since 2.4
+  */
+  public ItemInUseException(String what, Throwable cause)
+  {
+    super("The item "+what+" is used by another item.", cause);
+  }
+  
 }

trunk/src/plugins/core/net/sf/basedb/plugins/ReporterFlatFileImporter.java

@@ -25 +25 @@
 
 import net.sf.basedb.core.FileType;
+import net.sf.basedb.core.HibernateUtil;
 import net.sf.basedb.core.InvalidUseOfNullException;
 import net.sf.basedb.core.ItemContext;
+import net.sf.basedb.core.ItemInUseException;
 import net.sf.basedb.core.Permission;
 import net.sf.basedb.core.ReporterList;
@@ -37 +39 @@
 import net.sf.basedb.core.ItemParameterType;
 import net.sf.basedb.core.StringParameterType;
-import net.sf.basedb.core.BooleanParameterType;
 import net.sf.basedb.core.BaseException;
 import net.sf.basedb.core.ItemNotFoundException;
@@ -61 +62 @@
 import net.sf.basedb.core.plugin.GuiContext;
 
+import net.sf.basedb.util.error.SimpleErrorHandler;
 import net.sf.basedb.util.parser.ConfigureByExample;
 import net.sf.basedb.util.parser.ConstantMapper;
@@ -76 +78 @@
 import java.util.HashMap;
 import java.util.Set;
+import java.util.TreeSet;
 
 /**
@@ -112 +115 @@
   private static final StringParameterType requiredColumnMapping = new StringParameterType(255, null, true);
   private static final StringParameterType optionalColumnMapping = new StringParameterType(255, null, false);
-  private static final BooleanParameterType updateExistingType = new BooleanParameterType(false, true);
   
   private static final PluginParameter<String> nameColumnMapping = new PluginParameter<String>(
@@ -161 +163 @@
     );
 
-  private static final PluginParameter<Boolean> updateExistingParameter = new PluginParameter<Boolean>(
-    "updateExisting",
-    "Update existing reporters",
-    "If this option is selected, already existing reporters will be updated with the information "+
-    "in the file. If this option isn't selected existing reporters are left untouched.",
-    updateExistingType
-    );
+  protected static final PluginParameter<String> reporterIsUsedErrorParameter = new PluginParameter<String>(
+      "reporterIsUsedError",
+      "Reporter is used",
+      "How to handle errors that are caused by a reporter beeing used by other items" +
+      "and can't be deleted. This option is used in DELETE mode only. " +
+      "If not specified the default error handling is used.\n\n" +
+      "skip = Skip the current data line and continue\n"+
+      "fail = Stop with an error message",
+      new StringParameterType(255, "skip", false, 1, 0, 0, 
+          Arrays.asList( new String[] { "skip", "fail"} ))
+      );  
   
   private PluginParameter<ReporterType> reporterTypeParameter;
@@ -204 +210 @@
   }
   /**
-    Request create and write access to Reporter:s, write access to ReporterList:s
+    Request create, write and delete access to Reporter:s, write access to ReporterList:s
     and read access to File:s and ReporterType:s.
   */
@@ -211 +217 @@
     if (permissions.size() == 0)
     {
-      permissions.add(new Permissions(Item.REPORTER, null, EnumSet.of(Permission.CREATE, Permission.WRITE)));
+      permissions.add(new Permissions(Item.REPORTER, null, EnumSet.of(Permission.CREATE, Permission.DELETE)));
       permissions.add(new Permissions(Item.REPORTERLIST, null, EnumSet.of(Permission.WRITE)));
       permissions.add(new Permissions(Item.REPORTERTYPE, EnumSet.of(Permission.CREATE, Permission.USE), null));
@@ -227 +233 @@
     Return a set containing the context:s [REPORTER, LIST],
     [REPORTERLIST, ITEM], [REPORTERSCORE, LIST]. The first context
-    is for reporter reporters only, and the last two for importing 
+    is for importing reporters only, and the last two for importing 
     reporters to a reporter list.
   */
@@ -346 +352 @@
         storeValue(job, request, ri.getParameter(CHARSET));
         storeValue(job, request, ri.getParameter(DECIMAL_SEPARATOR));
-        storeValue(job, request, updateExistingParameter);
+        storeValue(job, request, ri.getParameter("mode"));
         
         // Error handling parameters
@@ -354 +360 @@
         storeValue(job, request, numberFormatErrorParameter);
         storeValue(job, request, numberOutOfRangeErrorParameter);
+        storeValue(job, request, reporterIsUsedErrorParameter);
         
         response.setDone("Job configuration complete", Job.ExecutionTime.SHORT);
@@ -381 +388 @@
   private ReporterList reporterList;
   private boolean updateExisting;
+  private boolean deleteMode;
   private int numInserted;
   private int numUpdated;
   private int numExists;
   private int numAddedToList;
+  private int numNotFound;
+  private int numUsed;
+  private int numDeleted;
   private Map<String, Float> deferred;
+  private Set<String> deleted;
   private FlatFileParser ffp;
   private NumberFormat numberFormat;
   private boolean nullIfException;
+  private boolean failIfUsed;
   
   /**
@@ -410 +423 @@
       numAddedToList = 0;
     }
-    updateExisting = (Boolean)job.getValue("updateExisting");
+    String mode = (String)job.getValue("mode");
+    updateExisting = "update".equals(mode);
+    deleteMode = "delete".equals(mode);
     this.ffp = ffp;
     this.numberFormat = ffp.getDefaultNumberFormat();
     this.nullIfException = "null".equals(job.getValue("numberFormatError"));
+    String reporterIsUsedError = (String)getErrorOption("reporterIsUsedError");
+    this.failIfUsed = "fail".equals(reporterIsUsedError);
+    if (reporterIsUsedError != null)
+    {
+      addErrorHandler(ItemInUseException.class, new SimpleErrorHandler(!failIfUsed));
+    }
+    if (deleteMode)
+    {
+      // Keep track of already deleted reporters
+      deleted = HibernateUtil.getDbEngine().caseInsensitiveComparison() ?
+        new TreeSet<String>(String.CASE_INSENSITIVE_ORDER) : new HashSet<String>();
+    }
     numInserted = 0;
     numUpdated = 0;
@@ -430 +457 @@
     idMapper = getMapper(ffp, (String)configuration.getValue("reporterIdColumnMapping"), 
       cropStrings ? ReporterData.MAX_EXTERNAL_ID_LENGTH : null, nullMapper);
-    nameMapper = getMapper(ffp, (String)configuration.getValue("nameColumnMapping"), 
-      cropStrings ? ReporterData.MAX_NAME_LENGTH : null, null);
-    symbolMapper = getMapper(ffp, (String)configuration.getValue("symbolColumnMapping"), 
-      cropStrings ? ReporterData.MAX_SYMBOL_LENGTH : null, null);
-    descriptionMapper = getMapper(ffp, (String)configuration.getValue("descriptionColumnMapping"), 
-      cropStrings ? ReporterData.MAX_DESCRIPTION_LENGTH : null, null);
-    reporterTypeMapper = getMapper(ffp, (String)configuration.getValue("reporterTypeColumnMapping"), 
-      cropStrings ? ReporterType.MAX_NAME_LENGTH : null, null);
-    scoreMapper = getMapper(ffp, (String)configuration.getValue("scoreColumnMapping"), null, null);
-    
-    reporterTypes = new HashMap<String, ReporterType>();
-    extendedMappers = new HashMap<ExtendedProperty, Mapper>();
-    List<ExtendedProperty> extendedProperties = ExtendedProperties.getProperties("ReporterData");
-    if (extendedProperties != null)
-    {
-      for (ExtendedProperty ep : extendedProperties)
-      {
-        String name = "extendedColumnMapping." + ep.getName();
-        Mapper m = getMapper(ffp, (String)configuration.getValue(name), 
-          cropStrings && ep.getLength() > 0 ? ep.getLength() : null, null);
-        if (m != null) extendedMappers.put(ep, m);
+    if (!deleteMode)
+    {
+      nameMapper = getMapper(ffp, (String)configuration.getValue("nameColumnMapping"), 
+        cropStrings ? ReporterData.MAX_NAME_LENGTH : null, null);
+      symbolMapper = getMapper(ffp, (String)configuration.getValue("symbolColumnMapping"), 
+        cropStrings ? ReporterData.MAX_SYMBOL_LENGTH : null, null);
+      descriptionMapper = getMapper(ffp, (String)configuration.getValue("descriptionColumnMapping"), 
+        cropStrings ? ReporterData.MAX_DESCRIPTION_LENGTH : null, null);
+      reporterTypeMapper = getMapper(ffp, (String)configuration.getValue("reporterTypeColumnMapping"), 
+        cropStrings ? ReporterType.MAX_NAME_LENGTH : null, null);
+      scoreMapper = getMapper(ffp, (String)configuration.getValue("scoreColumnMapping"), null, null);
+      
+      reporterTypes = new HashMap<String, ReporterType>();
+      extendedMappers = new HashMap<ExtendedProperty, Mapper>();
+      List<ExtendedProperty> extendedProperties = ExtendedProperties.getProperties("ReporterData");
+      if (extendedProperties != null)
+      {
+        for (ExtendedProperty ep : extendedProperties)
+        {
+          String name = "extendedColumnMapping." + ep.getName();
+          Mapper m = getMapper(ffp, (String)configuration.getValue(name), 
+            cropStrings && ep.getLength() > 0 ? ep.getLength() : null, null);
+          if (m != null) extendedMappers.put(ep, m);
+        }
       }
     }
@@ -504 +534 @@
     ReporterData reporter =  null;
     
-    // Is the reporter already queued for insert?
-    if (!batcher.isInInsertQueue(externalId))
-    {
-      // No, it's not... try to load it from the database
-      try
-      {
-        reporter = batcher.getByExternalId(externalId);
-      }
-      catch (ItemNotFoundException ex)
-      {
-        // It wasn't in the database either, create a new reporter
-        reporter = Reporter.getNew(externalId);
-      }
-    }
-
-    // If we have a reporter object, we must set the properties or add it to a reporter list
-    if (reporter != null)
-    {
-      int currentId = reporter.getId();
-      if (reporterList != null)
-      {
-        // Add to reporter list
-        Float score = scoreMapper == null ? 
-            reporterList.getScore(reporter) : scoreMapper.getFloat(data);
-//            (Float)Type.FLOAT.parseString(scoreMapper.getValue(data), numberFormat);
-        if (currentId == 0)
-        {
-          // It is a new reporter, we must wait to add it until the batcher has been flushed
-          deferred.put(externalId, score);
+    if (deleteMode)
+    {
+      if (deleted.add(externalId))
+      {
+        try
+        {
+          reporter = batcher.getByExternalId(externalId);
+        }
+        catch (ItemNotFoundException ex)
+        {
+          numNotFound++;
+        }
+        if (reporter != null)
+        {
+          if (reporterList != null)
+          {
+            reporterList.removeReporter(reporter);
+            numDeleted++;
+          }
+          else
+          {
+            try
+            {
+              batcher.delete(reporter);
+              batcher.flushDelete();
+              numDeleted++;
+            }
+            catch (BaseException ex)
+            {
+              if (failIfUsed) throw new ItemInUseException("Reporter[externalId=" + externalId + "]", ex);
+              numUsed++;
+            }
+          }
+        }
+      }
+    }
+    else
+    {
+      // Is the reporter already queued for insert?
+      if (!batcher.isInInsertQueue(externalId))
+      {
+        // No, it's not... try to load it from the database
+        try
+        {
+          reporter = batcher.getByExternalId(externalId);
+        }
+        catch (ItemNotFoundException ex)
+        {
+          // It wasn't in the database either, create a new reporter
+          reporter = Reporter.getNew(externalId);
+        }
+      }
+  
+      // If we have a reporter object, we must set the properties or add it to a reporter list
+      if (reporter != null)
+      {
+        int currentId = reporter.getId();
+        if (reporterList != null)
+        {
+          // Add to reporter list
+          Float score = scoreMapper == null ? 
+              reporterList.getScore(reporter) : scoreMapper.getFloat(data);
+  //            (Float)Type.FLOAT.parseString(scoreMapper.getValue(data), numberFormat);
+          if (currentId == 0)
+          {
+            // It is a new reporter, we must wait to add it until the batcher has been flushed
+            deferred.put(externalId, score);
+          }
+          else
+          {
+            reporterList.addReporter(reporter, score);
+            numAddedToList++;
+          }
+        }
+        
+        // The actual reporter needs updating or it is a new one
+        if ((updateExisting && currentId != 0) || currentId == 0)
+        {
+          
+          if (reporterType != null) 
+          {
+            Reporter.setReporterType(reporter, reporterType);
+          }
+          else if (reporterTypeMapper != null)
+          {
+            Reporter.setReporterType(reporter, getReporterType(reporterTypeMapper.getValue(data)));
+          }
+          String name = nameMapper == null ? reporter.getName() : nameMapper.getValue(data);
+          reporter.setName(name == null ? externalId : name);
+          
+          if (symbolMapper != null) reporter.setSymbol(symbolMapper.getValue(data));
+          if (descriptionMapper != null) reporter.setDescription(descriptionMapper.getValue(data));
+          for (Map.Entry<ExtendedProperty, Mapper> entry : extendedMappers.entrySet())
+          {
+            ExtendedProperty ep = entry.getKey();
+            Mapper m = entry.getValue();
+            reporter.setExtended(ep.getName(), ep.parseString(m.getValue(data), 
+              numberFormat, nullIfException));
+          }
+          if (currentId != 0)
+          {
+            batcher.update(reporter);
+            numUpdated++;
+          }
+          else
+          {
+            batcher.insert(reporter);
+            numInserted++;
+          }
         }
         else
         {
-          reporterList.addReporter(reporter, score);
-          numAddedToList++;
-        }
-      }
-      
-      // The actual reporter needs updating or it is a new one
-      if ((updateExisting && currentId != 0) || currentId == 0)
-      {
-        
-        if (reporterType != null) 
-        {
-          Reporter.setReporterType(reporter, reporterType);
-        }
-        else if (reporterTypeMapper != null)
-        {
-          Reporter.setReporterType(reporter, getReporterType(reporterTypeMapper.getValue(data)));
-        }
-        String name = nameMapper == null ? reporter.getName() : nameMapper.getValue(data);
-        reporter.setName(name == null ? externalId : name);
-        
-        if (symbolMapper != null) reporter.setSymbol(symbolMapper.getValue(data));
-        if (descriptionMapper != null) reporter.setDescription(descriptionMapper.getValue(data));
-        for (Map.Entry<ExtendedProperty, Mapper> entry : extendedMappers.entrySet())
-        {
-          ExtendedProperty ep = entry.getKey();
-          Mapper m = entry.getValue();
-          reporter.setExtended(ep.getName(), ep.parseString(m.getValue(data), 
-            numberFormat, nullIfException));
-        }
-        if (currentId != 0)
-        {
-          batcher.update(reporter);
-          numUpdated++;
-        }
-        else
-        {
-          batcher.insert(reporter);
-          numInserted++;
-        }
-      }
-      else
-      {
-        numExists++;
+          numExists++;
+        }
       }
     }
@@ -587 +656 @@
   protected String getSuccessMessage(int skippedLines)
   {
-    String msg = numInserted + " new reporter(s)";
-    if (numUpdated > 0) msg += "; " + numUpdated + " updated reporter(s)";
-    if (numExists > 0) msg += "; " + numExists + " reporter(s) skipped (already existed)";  
-    if (reporterList != null) msg += "; " + numAddedToList + " reporter(s) added to list";
+    String msg;
+    if (deleteMode)
+    {
+      msg = numDeleted + " deleted reporter(s)";
+      if (numUsed > 0) msg += "; " + numUsed + " reporter(s) are used and could not be deleted";
+      if (numNotFound > 0) msg += "; " + numNotFound + " reporter(s) ignored because they didn't exist";
+    }
+    else
+    {
+      msg = numInserted + " new reporter(s)";
+      if (numUpdated > 0) msg += "; " + numUpdated + " updated reporter(s)";
+      if (numExists > 0) msg += "; " + numExists + " reporter(s) skipped (already existed)";  
+      if (reporterList != null) msg += "; " + numAddedToList + " reporter(s) added to list";
+    }
     if (skippedLines > 0) msg += "; " + skippedLines + " line(s) skipped due to errors";
     return msg;
@@ -645 +724 @@
       }
       parameters.add(fileParameter);
-      parameters.add(updateExistingParameter);
+      
+      List<String> allowedModes = new ArrayList<String>();
+      String defaultMode = "update";
+      allowedModes.add("create");
+      allowedModes.add("update");
+      if (context.getItem() != Item.REPORTER || sc.hasPermission(Permission.DELETE, Item.REPORTER)) 
+      {
+        allowedModes.add("delete");
+      }
+      StringParameterType modeType = new StringParameterType(255, defaultMode, false, 1, 0, 0, allowedModes);
+      PluginParameter<String> modeParameter = new PluginParameter<String>(
+          "mode",
+          "Mode",
+          "Select the operating mode of the plug-in. Depending on permissions not all modes " +
+          "may be available\n\n" +
+          "create = Only create missing reporters; existing ones are not updated\n" +
+          "update = Update existing reporters and create missing ones\n" +
+          "delete = Delete existing reporters",
+          modeType
+          );
+      
+      parameters.add(modeParameter);
       parameters.add(getCharsetParameter(null, null, (String)configuration.getValue(CHARSET)));
       parameters.add(getDecimalSeparatorParameter(null, null, (String)configuration.getValue(DECIMAL_SEPARATOR)));
@@ -655 +755 @@
       parameters.add(numberFormatErrorParameter);
       parameters.add(numberOutOfRangeErrorParameter);
+      parameters.add(reporterIsUsedErrorParameter);
       
       configureJob = new RequestInformation

trunk/src/test/TestReporterFlatFileImporter.java

@@ -13 +13 @@
 import net.sf.basedb.core.ReporterType;
 import net.sf.basedb.core.RequestInformation;
+import net.sf.basedb.core.plugin.GuiContext;
 import net.sf.basedb.core.plugin.Response;
 
@@ -62 +63 @@
     int pluginConfigurationId = test_create_configuration(pluginDefinitionId, reporterTypeId);
     TestPluginConfiguration.test_load(pluginConfigurationId);
-    int jobId = test_create_job(pluginConfigurationId, fileId);
+    int jobId = test_create_job(pluginConfigurationId, fileId, "update");
     
     // Execute job
@@ -72 +73 @@
     
     if (TestUtil.waitBeforeDelete()) TestUtil.waitForEnter();
-    TestReporter.test_delete();
+    int jobId2 = test_create_job(pluginConfigurationId, fileId, "delete");
+    ok &= TestJob.test_execute(jobId2);
+    
+//    TestReporter.test_delete();
 
     TestJob.test_delete(jobId);
+    TestJob.test_delete(jobId2);
     TestPluginConfiguration.test_delete(pluginConfigurationId);
     TestFile.test_delete(fileId);
@@ -151 +156 @@
   }
 
-  static int test_create_job(int pluginConfigurationId, int fileId)
+  static int test_create_job(int pluginConfigurationId, int fileId, String mode)
   {
     if (pluginConfigurationId == 0 || fileId == 0 || !TestUtil.hasPermission(Permission.CREATE, Item.JOB)) return 0;
@@ -164 +169 @@
       j.setName(pc.getName());
       
-      PluginConfigurationRequest request = j.configure(null);
+      PluginConfigurationRequest request = j.configure(new GuiContext(Item.REPORTER, GuiContext.Type.LIST));
       write_request_information(request.getRequestInformation());
       request.setParameterValue("file", File.getById(dc, fileId));
-      request.setParameterValue("updateExisting", true);
+      request.setParameterValue("mode", mode);
       request.setParameterValue("invalidUseOfNullError", "skip");
       PluginResponse response = request.invoke();
@@ -181 +186 @@
       }
       id = j.getId();
-      write("--Create job for reporter import OK");
+      write("--Create job for reporter import (mode="+mode+") OK");
     }
     catch (Throwable ex)
     {
-      write("--Create job for reporter import FAILED");
+      write("--Create job for reporter import (mode="+mode+") FAILED");
       ex.printStackTrace();
       ok = false;

trunk/src/test/TestReporterMapFlatFileImporter.java

@@ -65 +65 @@
     int reporterPluginDefinitionId  = TestPluginDefinition.test_get("net.sf.basedb.plugins.ReporterFlatFileImporter");
     int reporterPluginConfigurationId = test_create_configuration_reporter(reporterPluginDefinitionId, reporterTypeId);
-    int jobId = TestReporterFlatFileImporter.test_create_job(reporterPluginConfigurationId, fileId);
+    int jobId = TestReporterFlatFileImporter.test_create_job(reporterPluginConfigurationId, fileId, "create");
     
     // Execute job