TaxonomyManager to take care of unique inserts

src/main/java/org/genesys2/server/service/TaxonomyService.java

@@ -33,8 +33,6 @@ public interface TaxonomyService {
 
 	long getTaxonomy2Id(String genus, String species);
 
-	Taxonomy2 ensureTaxonomy2(String genus, String species, String spAuthor, String subtaxa, String subtAuthor);
-
 	long countTaxonomy2();
 
 	Taxonomy2 get(Long id);
@@ -45,4 +43,6 @@ public interface TaxonomyService {
 
 	Taxonomy2 find(String genus, String species, String spAuthor, String subtaxa, String subtAuthor);
 
+	Taxonomy2 internalEnsure(String genus, String species, String spAuthor, String subtaxa, String subtAuthor) throws InterruptedException;
+
 }

src/main/java/org/genesys2/server/service/impl/BatchRESTServiceImpl.java

@@ -84,6 +84,9 @@ public class BatchRESTServiceImpl implements BatchRESTService {
 	@Autowired
 	AccessionCustomRepository accessionCustomRepository;
 
+	@Autowired
+	private TaxonomyManager taxonomyManager;
+
 	@Override
 	@Transactional
 	@PreAuthorize("hasRole('ADMINISTRATOR') or hasPermission(#institute, 'WRITE') or hasPermission(#institute, 'CREATE')")
@@ -684,7 +687,7 @@ public class BatchRESTServiceImpl implements BatchRESTService {
 					+ ") and lower(subtauthor)=lower(" + current.getSubtAuthor() + ")");
 		}
 		if (ensuredTaxonomy == null) {
-			ensuredTaxonomy = taxonomyService.ensureTaxonomy2(current.getGenus(), current.getSpecies(), current.getSpAuthor(), current.getSubtaxa(),
+			ensuredTaxonomy = taxonomyManager.ensureTaxonomy2(current.getGenus(), current.getSpecies(), current.getSpAuthor(), current.getSubtaxa(),
 					current.getSubtAuthor());
 		}
 

src/main/java/org/genesys2/server/service/impl/TaxonomyManager.java

+/**
+ * Copyright 2015 Global Crop Diversity Trust
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ **/
+
+package org.genesys2.server.service.impl;
+
+import java.util.concurrent.TimeUnit;
+
+import javax.annotation.Resource;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.commons.lang3.text.WordUtils;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.genesys2.server.model.genesys.Taxonomy2;
+import org.genesys2.server.service.TaxonomyService;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import com.hazelcast.core.ILock;
+
+/**
+ * Cross JVM taxonomy manager
+ * 
+ * @author matijaobreza
+ *
+ */
+@Component
+public class TaxonomyManager {
+	public static final Log LOG = LogFactory.getLog(TaxonomyManager.class);
+
+	@Autowired
+	private TaxonomyService taxonomyService;
+
+	/**
+	 * This lock ensures only one member of the cluster can enter taxonomy
+	 * update logic
+	 */
+	@Resource
+	private ILock taxonomyUpdateLock;
+
+	public Taxonomy2 ensureTaxonomy2(String genus, String species, String spAuthor, String subtaxa, String subtAuthor) {
+		if (StringUtils.isBlank(genus)) {
+			throw new RuntimeException("Genus can't be blank");
+		}
+
+		// Capitalize
+		genus = WordUtils.capitalizeFully(genus).trim();
+
+		species = StringUtils.defaultIfBlank(species, "sp.");
+		species = species.trim().toLowerCase();
+
+		spAuthor = StringUtils.defaultIfBlank(spAuthor, "").trim();
+		subtaxa = StringUtils.defaultIfBlank(subtaxa, "").trim();
+
+		if (subtaxa != null) {
+			subtaxa = subtaxa.toLowerCase();
+		}
+
+		subtAuthor = StringUtils.defaultIfBlank(subtAuthor, "").trim();
+
+		if (StringUtils.equalsIgnoreCase(species.trim(), "sp")) {
+			species = "sp.";
+		}
+
+		final Taxonomy2 existing = taxonomyService.find(genus, species, spAuthor, subtaxa, subtAuthor);
+		if (existing == null) {
+			try {
+				// LOCK
+				if (taxonomyUpdateLock.tryLock(10, TimeUnit.SECONDS)) {
+					Taxonomy2 taxonomy = taxonomyService.internalEnsure(genus, species, spAuthor, subtaxa, subtAuthor);
+					return taxonomy;
+				} else {
+					throw new RuntimeException("Could not persist Taxonomy2, lock wait timeout occured.");
+				}
+			} catch (InterruptedException e) {
+				LOG.warn("Thread interrupted while waiting for taxonomy lock.", e);
+				throw new RuntimeException(e);
+			} finally {
+				taxonomyUpdateLock.unlock();
+			}
+		}
+		return existing;
+	}
+}

src/main/java/org/genesys2/server/service/impl/TaxonomyServiceImpl.java

@@ -17,12 +17,8 @@
 package org.genesys2.server.service.impl;
 
 import java.util.List;
-import java.util.concurrent.TimeUnit;
-
-import javax.annotation.Resource;
 
 import org.apache.commons.lang.StringUtils;
-import org.apache.commons.lang3.text.WordUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.genesys2.server.model.genesys.Taxonomy2;
@@ -37,8 +33,6 @@ import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
 
-import com.hazelcast.core.ILock;
-
 @Service
 @Transactional(readOnly = true)
 public class TaxonomyServiceImpl implements TaxonomyService {
@@ -50,13 +44,6 @@ public class TaxonomyServiceImpl implements TaxonomyService {
 	@Autowired
 	private CropService cropService;
 
-	/**
-	 * This lock ensures only one member of the cluster can enter taxonomy
-	 * update logic
-	 */
-	@Resource
-	private ILock taxonomyUpdateLock;
-
 	@Override
 	public Taxonomy2 get(Long id) {
 		return taxonomy2Repository.findOne(id);
@@ -95,121 +82,74 @@ public class TaxonomyServiceImpl implements TaxonomyService {
 		return taxonomy2Repository.autocompleteTaxonomy("%" + term + "%", new PageRequest(0, 10));
 	}
 
-	@Override
-	// @Transactional(noRollbackFor = AssertionFailure.class)
-	@Transactional(propagation = Propagation.REQUIRES_NEW)
-	public Taxonomy2 ensureTaxonomy2(String genus, String species, String spAuthor, String subtaxa, String subtAuthor) {
-		if (StringUtils.isBlank(genus)) {
-			throw new RuntimeException("Genus can't be blank");
-		}
-
-		// Capitalize
-		genus = WordUtils.capitalizeFully(genus).trim();
-
-		species = StringUtils.defaultIfBlank(species, "sp.");
-		species = species.trim().toLowerCase();
-
-		spAuthor = StringUtils.defaultIfBlank(spAuthor, "").trim();
-		subtaxa = StringUtils.defaultIfBlank(subtaxa, "").trim();
-
-		if (subtaxa != null) {
-			subtaxa = subtaxa.toLowerCase();
-		}
-
-		subtAuthor = StringUtils.defaultIfBlank(subtAuthor, "").trim();
-
-		if (StringUtils.equalsIgnoreCase(species.trim(), "sp")) {
-			species = "sp.";
-		}
-
-		final Taxonomy2 existing = find(genus, species, spAuthor, subtaxa, subtAuthor);
-		if (existing == null) {
-			try {
-				Taxonomy2 taxonomy = internalEnsure(genus, species, spAuthor, subtaxa, subtAuthor);
-				return taxonomy;
-			} catch (InterruptedException e) {
-				LOG.warn("Thread interrupted while waiting for taxonomy lock.", e);
-				throw new RuntimeException(e);
-			}
-		}
-		return existing;
-	}
-
 	@Override
 	@Cacheable(value = "hibernate.org.genesys2.server.model.impl.Taxonomy2.fullname", key = "#genus + '-' + #species + '-' + #spAuthor + '-' + #subtaxa + '-' + #subtAuthor")
 	public Taxonomy2 find(String genus, String species, String spAuthor, String subtaxa, String subtAuthor) {
 		return taxonomy2Repository.findByGenusAndSpeciesAndSpAuthorAndSubtaxaAndSubtAuthor(genus, species, spAuthor, subtaxa, subtAuthor);
 	}
 
-	private synchronized Taxonomy2 internalEnsure(String genus, String species, String spAuthor, String subtaxa, String subtAuthor) throws InterruptedException {
+	@Override
+	@Transactional(readOnly = false, propagation = Propagation.REQUIRES_NEW)
+	public Taxonomy2 internalEnsure(String genus, String species, String spAuthor, String subtaxa, String subtAuthor) throws InterruptedException {
 		Long taxSpeciesId = null, taxGenusId = null;
 
-		// LOCK
-		if (taxonomyUpdateLock.tryLock(10, TimeUnit.SECONDS)) {
+		// Direct species
+		if (subtaxa.equals("") && spAuthor.equals("") && subtAuthor.equals("")) {
+			if (StringUtils.equals(species, "sp.")) {
+				// Self
+			} else {
+				final Taxonomy2 genusTaxa = internalEnsure(genus, "sp.", "", "", "");
+				taxGenusId = genusTaxa.getId();
+			}
+		} else {
+			final Taxonomy2 speciesTaxa = internalEnsure(genus, species, "", "", "");
+			taxSpeciesId = speciesTaxa.getId();
+			taxGenusId = speciesTaxa.getTaxGenus();
+		}
+
+		Taxonomy2 taxonomy = null;
+		try {
+			taxonomy = find(genus, species, spAuthor, subtaxa, subtAuthor);
+		} catch (final Throwable e) {
+			LOG.info("Taxonomy not found: " + e.getMessage());
+		}
+
+		if (taxonomy != null) {
+			return taxonomy;
+		} else {
+			LOG.info("Adding new taxonomic name: " + genus + " " + species + " " + spAuthor + " " + subtaxa + " " + subtAuthor);
+			taxonomy = new Taxonomy2();
+			taxonomy.setGenus(genus);
+			taxonomy.setSpecies(species);
+			taxonomy.setSpAuthor(spAuthor);
+			taxonomy.setSubtaxa(subtaxa);
+			taxonomy.setSubtAuthor(subtAuthor);
+			taxonomy.setTaxGenus(taxGenusId);
+			taxonomy.setTaxSpecies(taxSpeciesId);
+
 			try {
-				// Direct species
-				if (subtaxa.equals("") && spAuthor.equals("") && subtAuthor.equals("")) {
-					if (StringUtils.equals(species, "sp.")) {
-						// Self
-					} else {
-						final Taxonomy2 genusTaxa = internalEnsure(genus, "sp.", "", "", "");
-						taxGenusId = genusTaxa.getId();
-					}
-				} else {
-					final Taxonomy2 speciesTaxa = internalEnsure(genus, species, "", "", "");
-					taxSpeciesId = speciesTaxa.getId();
-					taxGenusId = speciesTaxa.getTaxGenus();
-				}
+				taxonomy = taxonomy2Repository.save(taxonomy);
 
-				Taxonomy2 taxonomy = null;
-				try {
-					taxonomy = taxonomy2Repository.findByGenusAndSpeciesAndSpAuthorAndSubtaxaAndSubtAuthor(genus, species, spAuthor, subtaxa, subtAuthor);
-				} catch (final Throwable e) {
-					LOG.info("Taxonomy not found: " + e.getMessage());
+				if (taxGenusId == null) {
+					taxonomy.setTaxGenus(taxonomy.getId());
+					taxonomy = taxonomy2Repository.save(taxonomy);
 				}
-
-				if (taxonomy != null) {
-					return taxonomy;
-				} else {
-					LOG.info("Adding new taxonomic name: " + genus + " " + species + " " + spAuthor + " " + subtaxa + " " + subtAuthor);
-					taxonomy = new Taxonomy2();
-					taxonomy.setGenus(genus);
-					taxonomy.setSpecies(species);
-					taxonomy.setSpAuthor(spAuthor);
-					taxonomy.setSubtaxa(subtaxa);
-					taxonomy.setSubtAuthor(subtAuthor);
-					taxonomy.setTaxGenus(taxGenusId);
-					taxonomy.setTaxSpecies(taxSpeciesId);
-
-					try {
-						taxonomy = taxonomy2Repository.save(taxonomy);
-
-						if (taxGenusId == null) {
-							taxonomy.setTaxGenus(taxonomy.getId());
-							taxonomy = taxonomy2Repository.save(taxonomy);
-						}
-						if (taxSpeciesId == null) {
-							taxonomy.setTaxSpecies(taxonomy.getId());
-							taxonomy = taxonomy2Repository.save(taxonomy);
-						}
-
-						// Update crop taxonomy lists
-						cropService.updateCropTaxonomyLists(taxonomy);
-
-						return taxonomy;
-
-					} catch (final Throwable e) {
-						LOG.warn("Error " + e.getMessage() + " :" + taxonomy);
-						throw new RuntimeException(e.getMessage());
-					}
+				if (taxSpeciesId == null) {
+					taxonomy.setTaxSpecies(taxonomy.getId());
+					taxonomy = taxonomy2Repository.save(taxonomy);
 				}
 
-			} finally {
-				taxonomyUpdateLock.unlock();
+				// Update crop taxonomy lists
+				cropService.updateCropTaxonomyLists(taxonomy);
+
+				return taxonomy;
+
+			} catch (final Throwable e) {
+				LOG.warn("Error " + e.getMessage() + " :" + taxonomy);
+				throw new RuntimeException(e.getMessage());
 			}
-		} else {
-			throw new RuntimeException("Could not persist Taxonomy2, lock wait timeout occured.");
 		}
+
 	}
 
 	@Override

src/test/java/org/genesys2/server/service/worker/SGSVUpdateTest.java

@@ -55,6 +55,7 @@ import org.genesys2.server.service.impl.InstituteServiceImpl;
 import org.genesys2.server.service.impl.NonUniqueAccessionException;
 import org.genesys2.server.service.impl.OWASPSanitizer;
 import org.genesys2.server.service.impl.OrganizationServiceImpl;
+import org.genesys2.server.service.impl.TaxonomyManager;
 import org.genesys2.server.service.impl.TaxonomyServiceImpl;
 import org.genesys2.server.service.impl.UserServiceImpl;
 import org.genesys2.server.test.JpaDataConfig;
@@ -182,6 +183,9 @@ public class SGSVUpdateTest {
 	@Autowired
 	private TaskExecutor taskExecutor;
 
+	@Autowired
+	private TaxonomyManager taxonomyManager;
+
 	@Before
 	public void setup() {
 		LOG.info("Setting up");
@@ -211,7 +215,7 @@ public class SGSVUpdateTest {
 		final Accession a = new Accession();
 		a.setInstitute(institute);
 		a.setAccessionName(acceNumb);
-		a.setTaxonomy(taxonomyService.ensureTaxonomy2(genus, species, null, null, null));
+		a.setTaxonomy(taxonomyManager.ensureTaxonomy2(genus, species, null, null, null));
 
 		accessions.add(a);
 		genesysService.saveAccessions(institute, accessions);

src/test/java/org/genesys2/server/test/TaxonomyEnsureTest.java

@@ -27,6 +27,7 @@ import org.genesys2.server.service.HtmlSanitizer;
 import org.genesys2.server.service.TaxonomyService;
 import org.genesys2.server.service.impl.CropServiceImpl;
 import org.genesys2.server.service.impl.OWASPSanitizer;
+import org.genesys2.server.service.impl.TaxonomyManager;
 import org.genesys2.server.service.impl.TaxonomyServiceImpl;
 import org.genesys2.spring.config.SpringCacheConfig;
 import org.junit.Test;
@@ -45,6 +46,11 @@ public class TaxonomyEnsureTest {
 
 	@Import({ JpaNoCacheDataConfig.class, SpringCacheConfig.class })
 	public static class Config {
+		
+		@Bean
+		public TaxonomyManager taxonomyManager() {
+			return new TaxonomyManager();
+		}
 
 		@Bean
 		public TaxonomyService taxonomyService() {
@@ -66,6 +72,9 @@ public class TaxonomyEnsureTest {
 	// use it without @Scheduled
 	@Autowired
 	private TaxonomyService taxonomyService;
+	
+	@Autowired
+	private TaxonomyManager taxonomyManager;
 
 	@Test
 	public void testSubstr() {
@@ -86,7 +95,7 @@ public class TaxonomyEnsureTest {
 	@Test
 	public void testEnsure1() {
 		try {
-			final Taxonomy2 tax = taxonomyService.ensureTaxonomy2("Aaaaa", "bbbb", null, "alfalfa", null);
+			final Taxonomy2 tax = taxonomyManager.ensureTaxonomy2("Aaaaa", "bbbb", null, "alfalfa", null);
 			assertTrue("id missing", tax.getId() != null);
 			assertTrue("taxGenus should not be null", tax.getTaxGenus() != null);
 			assertTrue("taxSpecies should not be null", tax.getTaxSpecies() != null);
@@ -108,7 +117,7 @@ public class TaxonomyEnsureTest {
 	@Test
 	public void testNullSpecies() {
 		try {
-			final Taxonomy2 tax = taxonomyService.ensureTaxonomy2("Aaaaa", null, null, null, null);
+			final Taxonomy2 tax = taxonomyManager.ensureTaxonomy2("Aaaaa", null, null, null, null);
 			assertTrue("id missing", tax.getId() != null);
 			assertTrue("taxGenus should not be null", tax.getTaxGenus() != null);
 			assertTrue("taxSpecies should not be null", tax.getTaxSpecies() != null);
@@ -123,7 +132,7 @@ public class TaxonomyEnsureTest {
 	@Test
 	public void testSpSpecies() {
 		try {
-			final Taxonomy2 tax = taxonomyService.ensureTaxonomy2("Aaaaa", "sp.", null, null, null);
+			final Taxonomy2 tax = taxonomyManager.ensureTaxonomy2("Aaaaa", "sp.", null, null, null);
 			assertTrue("id missing", tax.getId() != null);
 			assertTrue("species must be sp.", tax.getSpecies().equals("sp."));
 			assertTrue("taxGenus should not be null", tax.getTaxGenus() != null);
@@ -139,7 +148,7 @@ public class TaxonomyEnsureTest {
 	@Test
 	public void testSpecies() {
 		try {
-			final Taxonomy2 tax = taxonomyService.ensureTaxonomy2("Aaaaa", "species", null, null, null);
+			final Taxonomy2 tax = taxonomyManager.ensureTaxonomy2("Aaaaa", "species", null, null, null);
 			assertTrue("id missing", tax.getId() != null);
 			assertTrue("species must be species", tax.getSpecies().equals("species"));
 			assertTrue("taxGenus should not be null", tax.getTaxGenus() != null);
@@ -155,7 +164,7 @@ public class TaxonomyEnsureTest {
 	@Test
 	public void testFullTaxonomy() {
 		try {
-			final Taxonomy2 tax = taxonomyService.ensureTaxonomy2("Aaaaa", "species", "spauthor", "subtaxa", "subtauthor");
+			final Taxonomy2 tax = taxonomyManager.ensureTaxonomy2("Aaaaa", "species", "spauthor", "subtaxa", "subtauthor");
 			System.out.println(tax);
 			assertTrue("id missing", tax.getId() != null);
 			assertTrue("species must be sp.", tax.getSpecies().equals("species"));