From cd27e96cec34e75209ee0bde041717c20e9bc265 Mon Sep 17 00:00:00 2001 From: Stuart Lewis Date: Tue, 5 Jul 2011 02:11:03 +0000 Subject: [PATCH] [DS-834] - CSV import dialog doesn't handle csv errors gracefully git-svn-id: http://scm.dspace.org/svn/repo/dspace/trunk@6452 9c30dcfa-912a-0410-8fc2-9e0234be79fd --- .../org/dspace/app/bulkedit/DSpaceCSV.java | 21 ++++++++- .../dspace/app/bulkedit/DSpaceCSVLine.java | 2 +- .../dspace/app/bulkedit/MetadataImport.java | 45 +++++++++++++++++-- ...MetadataImportInvalidHeadingException.java | 35 ++++++++++++--- 4 files changed, 91 insertions(+), 12 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSV.java b/dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSV.java index ae13c8456b..c9c04abf29 100644 --- a/dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSV.java +++ b/dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSV.java @@ -97,8 +97,11 @@ public class DSpaceCSV implements Serializable // Read the heading line String head = input.readLine(); String[] headingElements = head.split(escapedFieldSeparator); + int columnCounter = 0; for (String element : headingElements) { + columnCounter++; + // Remove surrounding quotes if there are any if ((element.startsWith("\"")) && (element.endsWith("\""))) { @@ -116,6 +119,13 @@ public class DSpaceCSV implements Serializable // Verify that the heading is valid in the metadata registry String[] clean = element.split("\\["); String[] parts = clean[0].split("\\."); + + if (parts.length < 2) { + throw new MetadataImportInvalidHeadingException(element, + MetadataImportInvalidHeadingException.ENTRY, + columnCounter); + } + String metadataSchema = parts[0]; String metadataElement = parts[1]; String metadataQualifier = null; @@ -127,7 +137,8 @@ public class DSpaceCSV implements Serializable MetadataSchema foundSchema = MetadataSchema.find(c, metadataSchema); if (foundSchema == null) { throw new MetadataImportInvalidHeadingException(clean[0], - MetadataImportInvalidHeadingException.SCHEMA); + MetadataImportInvalidHeadingException.SCHEMA, + columnCounter); } // Check that the metadata element exists in the schema @@ -135,7 +146,8 @@ public class DSpaceCSV implements Serializable MetadataField foundField = MetadataField.findByElement(c, schemaID, metadataElement, metadataQualifier); if (foundField == null) { throw new MetadataImportInvalidHeadingException(clean[0], - MetadataImportInvalidHeadingException.ELEMENT); + MetadataImportInvalidHeadingException.ELEMENT, + columnCounter); } // Store the heading @@ -474,6 +486,11 @@ public class DSpaceCSV implements Serializable } // Make sure we register that this column was there + if (headings.size() < i) { + throw new MetadataImportInvalidHeadingException("", + MetadataImportInvalidHeadingException.MISSING, + i + 1); + } csvLine.add(headings.get(i - 1), null); String[] elements = part.split(escapedValueSeparator); for (String element : elements) diff --git a/dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSVLine.java b/dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSVLine.java index f7135aa52a..666fa3f3ae 100644 --- a/dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSVLine.java +++ b/dspace-api/src/main/java/org/dspace/app/bulkedit/DSpaceCSVLine.java @@ -29,7 +29,7 @@ public class DSpaceCSVLine /** * Create a new CSV line * - * @param id The item ID of the line + * @param itemId The item ID of the line */ public DSpaceCSVLine(int itemId) { diff --git a/dspace-api/src/main/java/org/dspace/app/bulkedit/MetadataImport.java b/dspace-api/src/main/java/org/dspace/app/bulkedit/MetadataImport.java index 8efe5d147b..38c3b814b9 100644 --- a/dspace-api/src/main/java/org/dspace/app/bulkedit/MetadataImport.java +++ b/dspace-api/src/main/java/org/dspace/app/bulkedit/MetadataImport.java @@ -9,10 +9,12 @@ package org.dspace.app.bulkedit; import org.apache.commons.cli.*; +import org.apache.log4j.Logger; import org.dspace.content.*; import org.dspace.core.Context; import org.dspace.core.Constants; import org.dspace.authorize.AuthorizeException; +import org.dspace.core.LogManager; import org.dspace.handle.HandleManager; import org.dspace.eperson.EPerson; import org.dspace.workflow.WorkflowManager; @@ -38,6 +40,10 @@ public class MetadataImport /** The lines to import */ List toImport; + /** Logger */ + private static final Logger log = Logger.getLogger(MetadataImport.class); + + /** * Create an instance of the metadata importer. Requires a context and an array of CSV lines * to examine. @@ -65,9 +71,9 @@ public class MetadataImport * @throws MetadataImportException if something goes wrong */ public List runImport(boolean change, - boolean useWorkflow, - boolean workflowNotify, - boolean useTemplate) throws MetadataImportException + boolean useWorkflow, + boolean workflowNotify, + boolean useTemplate) throws MetadataImportException { // Store the changes ArrayList changes = new ArrayList(); @@ -288,6 +294,16 @@ public class MetadataImport private void compare(Item item, String[] fromCSV, boolean change, String md, BulkEditChange changes) throws SQLException, AuthorizeException { + // Log what metadata element we're looking at + String all = ""; + for (String part : fromCSV) + { + all += part + ","; + } + all = all.substring(0, all.length()); + log.debug(LogManager.getHeader(c, "metadata_import", + "item_id=" + item.getID() + ",fromCSV=" + all)); + // Don't compare collections if ("collection".equals(md)) { @@ -321,6 +337,12 @@ public class MetadataImport qualifier = qualifier.substring(0, qualifier.indexOf('[')); } } + log.debug(LogManager.getHeader(c, "metadata_import", + "item_id=" + item.getID() + ",fromCSV=" + all + + ",looking_for_schema=" + schema + + ",looking_for_element=" + element + + ",looking_for_qualifier=" + qualifier + + ",looking_for_language=" + language)); DCValue[] current = item.getMetadata(schema, element, qualifier, language); String[] dcvalues = new String[current.length]; @@ -329,6 +351,9 @@ public class MetadataImport { dcvalues[i] = dcv.value; i++; + log.debug(LogManager.getHeader(c, "metadata_import", + "item_id=" + item.getID() + ",fromCSV=" + all + + ",found=" + dcv.value)); } // Compare from csv->current @@ -344,6 +369,12 @@ public class MetadataImport if ((value != null) && (!"".equals(value)) && (!contains(value, fromCSV))) { // Remove it + log.debug(LogManager.getHeader(c, "metadata_import", + "item_id=" + item.getID() + ",fromCSV=" + all + + ",removing_schema=" + schema + + ",removing_element=" + element + + ",removing_qualifier=" + qualifier + + ",removing_language=" + language)); changes.registerRemove(dcv); } else @@ -587,7 +618,7 @@ public class MetadataImport * @param md The element to compare * @param changes The changes object to populate * - * @throws SQLException when an SQL error has occured (querying DSpace) + * @throws SQLException when an SQL error has occurred (querying DSpace) * @throws AuthorizeException If the user can't make the changes */ private void add(String[] fromCSV, String md, BulkEditChange changes) @@ -986,6 +1017,12 @@ public class MetadataImport { csv = new DSpaceCSV(new File(filename), c); } + catch (MetadataImportInvalidHeadingException miihe) + { + System.err.println(miihe.getMessage()); + System.exit(1); + return; + } catch (Exception e) { System.err.println("Error reading file: " + e.getMessage()); diff --git a/dspace-api/src/main/java/org/dspace/app/bulkedit/MetadataImportInvalidHeadingException.java b/dspace-api/src/main/java/org/dspace/app/bulkedit/MetadataImportInvalidHeadingException.java index 27a338fafe..14a1341f81 100644 --- a/dspace-api/src/main/java/org/dspace/app/bulkedit/MetadataImportInvalidHeadingException.java +++ b/dspace-api/src/main/java/org/dspace/app/bulkedit/MetadataImportInvalidHeadingException.java @@ -20,12 +20,21 @@ public class MetadataImportInvalidHeadingException extends Exception /** The bad heading */ private String badHeading; + /** The column number */ + private int column; + /** Error with the schema */ public static final int SCHEMA = 0; /** Error with the element */ public static final int ELEMENT = 1; + /** Error with a missing header */ + public static final int MISSING = 98; + + /** Error with the whole entry */ + public static final int ENTRY = 99; + /** * Instantiate a new MetadataImportInvalidHeadingException @@ -33,11 +42,12 @@ public class MetadataImportInvalidHeadingException extends Exception * @param message the error message * @param theType the type of the error */ - public MetadataImportInvalidHeadingException(String message, int theType) + public MetadataImportInvalidHeadingException(String message, int theType, int theColumn) { super(message); badHeading = message; type = theType; + column = theColumn; } /** @@ -60,6 +70,16 @@ public class MetadataImportInvalidHeadingException extends Exception return badHeading; } + /** + * Get the column number that was invalid + * + * @return the invalid column number + */ + public int getColumn() + { + return column; + } + /** * Get the exception message * @@ -69,11 +89,16 @@ public class MetadataImportInvalidHeadingException extends Exception { if (type == SCHEMA) { - return "Unknown metadata schema in heading: " + badHeading; - } - else + return "Unknown metadata schema in row " + column + ": " + badHeading; + } else if (type == ELEMENT) { - return "Unknown metadata element in heading: " + badHeading; + return "Unknown metadata element in row " + column + ": " + badHeading; + } else if (type == MISSING) + { + return "Row with missing header: Row " + column; + } else + { + return "Bad metadata declaration in row " + column + ": " + badHeading; } } } \ No newline at end of file