slub.team team mailing list archive

[Ralf Claussnitzer <ralf.claussnitzer@xxxxxxxxxxxxxxx>]

Ralf Claussnitzer has proposed merging lp:~slub.team/goobi-production/bug-1034373 into lp:goobi-production.

Requested reviews:
  Henning Gerhardt (henning-gerhardt)
Related bugs:
  Bug #1034373 in Goobi.Production: "meta data editor overwrites data on save"
  https://bugs.launchpad.net/goobi-production/+bug/1034373

For more details, see:
https://code.launchpad.net/~slub.team/goobi-production/bug-1034373/+merge/123036
-- 
https://code.launchpad.net/~slub.team/goobi-production/bug-1034373/+merge/123036
Your team Saxon State Library Team is subscribed to branch lp:goobi-production.

=== modified file 'config/messages_de.properties'
--- config/messages_de.properties	2012-07-17 14:05:47 +0000
+++ config/messages_de.properties	2012-09-06 09:19:18 +0000
@@ -418,6 +418,7 @@
 metadatenBildAnzeigen=Bild anzeigen
 metadatenBildAusblenden=Bild ausblenden
 metadatenDennochBearbeiten=Metadaten dennoch bearbeiten
+metadatenEditorThreadLock=Ein anderer Vorgang wurde im Hintergrund geöffnet. Bitte versuchen Sie es in einigen Sekunden erneut.
 metadatenFuerDMSExportieren=Metadaten f\u00FCr DMS exportieren
 metadatenImportieren=Metadaten importieren
 metadatenKopieren=Metadaten kopieren

=== modified file 'config/messages_en.properties'
--- config/messages_en.properties	2012-08-02 14:15:19 +0000
+++ config/messages_en.properties	2012-09-06 09:19:18 +0000
@@ -418,6 +418,7 @@
 metadatenBildAnzeigen=Show image
 metadatenBildAusblenden=Hide image
 metadatenDennochBearbeiten=Edit metadata anyway
+metadatenEditorThreadLock=Another file gets processed in the background. Please try again after some seconds.
 metadatenFuerDMSExportieren=Export metadata for DMS
 metadatenImportieren=Import metadata
 metadatenKopieren=Copy metadata

=== modified file 'src/de/sub/goobi/metadaten/Metadaten.java'
--- src/de/sub/goobi/metadaten/Metadaten.java	2012-07-03 14:01:09 +0000
+++ src/de/sub/goobi/metadaten/Metadaten.java	2012-09-06 09:19:18 +0000
@@ -35,6 +35,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.StringTokenizer;
+import java.util.concurrent.locks.ReentrantLock;
 
 import javax.faces.context.FacesContext;
 import javax.faces.model.SelectItem;
@@ -170,6 +171,8 @@
 	private String pagesEnd="";
 	private HashMap<String, Boolean> treeProperties;
 
+	ReentrantLock xmlReadingLock = new ReentrantLock();
+
 	/**
 	 * Konstruktor ================================================================
 	 */
@@ -508,65 +511,63 @@
 	 * ## ##################################################### ####################################################
 	 */
 
-	/**
-	 * Metadaten Einlesen
-	 * 
-	 */
+
 	public String XMLlesen() {
-
-		/*
-		 * re-reading the ruleset.xml file
-		 */
-		ConfigDispayRules.getInstance().refresh();
-
-		Modes.setBindState(BindState.edit);
-		try {
-			myProzess = new ProzessDAO().get(new Integer(Helper.getRequestParameter("ProzesseID")));
-		} catch (NumberFormatException e1) {
-			Helper.setFehlerMeldung("error while loading process data" + e1.getMessage());
-			return Helper.getRequestParameter("zurueck");
-		} catch (DAOException e1) {
-			Helper.setFehlerMeldung("error while loading process data" + e1.getMessage());
-			return Helper.getRequestParameter("zurueck");
-		}
-		myBenutzerID = Helper.getRequestParameter("BenutzerID");
-		alleSeitenAuswahl_ersteSeite = "";
-		alleSeitenAuswahl_letzteSeite = "";
-		zurueck = Helper.getRequestParameter("zurueck");
-		nurLesenModus = Helper.getRequestParameter("nurLesen").equals("true") ? true : false;
-		neuesElementWohin = "1";
-		tree3 = null;
-		try {
-			XMLlesenStart();
-		} catch (SwapException e) {
-			Helper.setFehlerMeldung(e);
-			return Helper.getRequestParameter("zurueck");
-		} catch (ReadException e) {
-			Helper.setFehlerMeldung("error while loading metadata" + e.getMessage());
-			return Helper.getRequestParameter("zurueck");
-		} catch (PreferencesException e) {
-			Helper.setFehlerMeldung("error while loading metadata" + e.getMessage());
-			return Helper.getRequestParameter("zurueck");
-		} catch (WriteException e) {
-			Helper.setFehlerMeldung("error while loading metadata" + e.getMessage());
-			return Helper.getRequestParameter("zurueck");
-		} catch (IOException e) {
-			Helper.setFehlerMeldung("error while loading metadata" + e.getMessage());
-			return Helper.getRequestParameter("zurueck");
-		} catch (InterruptedException e) {
-			Helper.setFehlerMeldung("error while loading metadata" + e.getMessage());
-			return Helper.getRequestParameter("zurueck");
-		} catch (DAOException e) {
-			Helper.setFehlerMeldung("error while loading metadata" + e.getMessage());
-			return Helper.getRequestParameter("zurueck");
-		}
-
-		TreeExpand();
-		sperrung.setLocked(myProzess.getId().intValue(), myBenutzerID);
-		return "Metadaten";
+		String result = "";
+
+		if (xmlReadingLock.tryLock()) {
+			try {
+				result = readXmlAndBuildTree();
+			} catch (RuntimeException rte) {
+				throw rte;
+			} finally {
+				xmlReadingLock.unlock();
+			}
+		} else {
+			Helper.setFehlerMeldung("metadatenEditorThreadLock");
+		}
+	
+		return result;
 	}
 
-	/**
+
+    private String readXmlAndBuildTree() {
+
+        // refresh ruleset in case it has changed
+        ConfigDispayRules.getInstance().refresh();
+
+        Modes.setBindState(BindState.edit);
+
+        zurueck = Helper.getRequestParameter("zurueck");
+
+        try {
+            myProzess = new ProzessDAO().get(new Integer(Helper.getRequestParameter("ProzesseID")));
+            myBenutzerID = Helper.getRequestParameter("BenutzerID");
+            nurLesenModus = Helper.getRequestParameter("nurLesen").equals("true");
+        } catch (Exception e) {
+            Helper.setFehlerMeldung("error while loading process data", e.getMessage());
+            return zurueck;
+        }
+
+        alleSeitenAuswahl_ersteSeite = "";
+        alleSeitenAuswahl_letzteSeite = "";
+        neuesElementWohin = "1";
+        tree3 = null;
+
+        try {
+            XMLlesenStart();
+        } catch (Exception e) {
+            Helper.setFehlerMeldung("error while loading metadata", e.getMessage());
+            return zurueck;
+        }
+
+        TreeExpand();
+
+        sperrung.setLocked(myProzess.getId(), myBenutzerID);
+        return "Metadaten";
+    }
+
+    /**
 	 * Metadaten Einlesen
 	 * 
 	 * @throws ReadException

=== added file 'test/src/de/sub/goobi/metadaten/MetadatenTest.java'
--- test/src/de/sub/goobi/metadaten/MetadatenTest.java	1970-01-01 00:00:00 +0000
+++ test/src/de/sub/goobi/metadaten/MetadatenTest.java	2012-09-06 09:19:18 +0000
@@ -0,0 +1,117 @@
+/*
+ * This file is part of the Goobi Application - a Workflow tool for the support of
+ * mass digitization.
+ *
+ * Visit the websites for more information.
+ *	   - http://gdz.sub.uni-goettingen.de
+ *	   - http://www.goobi.org
+ *	   - http://launchpad.net/goobi-production
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation; either version 2 of the License, or (at your option) any later
+ * version.
+ *
+ * This program 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 GNU General Public License for more details. You
+ * should have received a copy of the GNU General Public License along with this
+ * program; if not, write to the Free Software Foundation, Inc., 59 Temple Place,
+ * Suite 330, Boston, MA 02111-1307 USA
+ */
+
+package de.sub.goobi.metadaten;
+
+import de.sub.goobi.helper.Messages;
+import org.apache.log4j.BasicConfigurator;
+
+import org.goobi.log4j.TestAppender;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
+
+public class MetadatenTest {
+
+	private TestAppender testAppender;
+
+	@Before
+	public void setUpTestAppender() {
+		testAppender = new TestAppender();
+		BasicConfigurator.configure(testAppender);
+	}
+
+	@Test(expected = NullPointerException.class)
+	public void throwsNullPointerExceptionWhenCalledWithoutInitializedHelper() {
+		Metadaten md = new Metadaten();
+		md.XMLlesen();
+	}
+
+	@Test
+	public void xmlReadingLockIsNotAquiredAfterCreation() {
+		Metadaten md = new Metadaten();
+		assertFalse("XML Reading Lock is already aquired but shouldn't be.",
+			md.xmlReadingLock.isLocked());
+	}
+
+	@Test
+	public void returnsEmptyStringIfXmlReadingLockIsAlreadyAquiredByOtherThread()
+	throws InterruptedException {
+		final Metadaten md = new Metadaten();
+		final MutableString result = new MutableString();
+		Thread t = new Thread() {
+			public void run() {
+				result.set(md.XMLlesen());
+			}
+		};
+		
+		md.xmlReadingLock.lock();
+		t.start();
+		t.join();
+
+		assertEquals("Should return empty string is XML Reading Lock is aquired.", result.get(), "");
+	}
+
+	@Test
+	public void logsErrorIfXmlReadingLockIsAlreadyAquiredByOtherThread()
+	throws InterruptedException {
+		final Metadaten md = new Metadaten();
+		Thread t = new Thread() {
+			public void run() {
+				md.XMLlesen();
+			}
+		};
+		
+		md.xmlReadingLock.lock();
+		t.start();
+		t.join();
+
+		assertLogMessageContains(Messages.getString("metadatenEditorThreadLock"));
+	}
+
+	@Test
+	public void freesXmlReadingLockIfExceptionHappens()
+	throws InterruptedException {
+		final Metadaten md = new Metadaten();
+		try {
+			md.XMLlesen();
+		} catch (NullPointerException npe) {
+			assertFalse("Should free XML Reading Lock.", md.xmlReadingLock.isLocked());
+		}
+	}
+
+	private class MutableString {
+		private String s;
+		public void set(String val) { s = val; }
+		public String get() { return s; }
+	}
+
+	private void assertLogMessageContains(String message) {
+		String lastMessage = (String) testAppender.getLastEvent().getMessage();
+		assertTrue("Log message '" + lastMessage + "' does not contain '" + message + "'", lastMessage.contains(message));
+	}
+
+}