Fix: `newAcceNumb` should not be ignored

src/main/java/org/genesys2/server/service/worker/AccessionUploader.java

@@ -382,7 +382,7 @@ public class AccessionUploader implements InitializingBean {
 
 	private void updateAccessionNumber(JsonNode nodeAcceNumb, JsonNode nodeNewNumb, Accession accession) {
 		String accessionNumber = nodeAcceNumb.textValue();
-		String newAcceNumb = nodeNewNumb == null ? null : nodeNewNumb.textValue();
+		String newAcceNumb = nodeNewNumb == null ? null : nodeNewNumb.textValue().trim();
 
 		if (! accession.isPersisted()) {
 			if (newAcceNumb != null) {
@@ -392,10 +392,13 @@ public class AccessionUploader implements InitializingBean {
 			accession.setAccessionNumber(accessionNumber);
 
 		} else if (accession.getDoi() != null) {
-			// When accession has a DOI, override accession number
-			accession.setAccessionNumber(accessionNumber);
-			
-		} else if (newAcceNumb != null) {
+			// DO NOT IMPLICITLY UPDATE ACCESSION NUMBERS
+			if (! accession.getAccessionNumber().equalsIgnoreCase(accessionNumber)) {
+				throw new InvalidApiUsageException("ACCENUMB for doi=" + accession.getDoi() + " does not match current " + accession.getAccessionNumber());
+			}
+		}
+
+		if (StringUtils.isNotBlank(newAcceNumb)) {
 			if (! StringUtils.equals(newAcceNumb, accession.getAccessionNumber())) {
 				LOG.warn("Renaming accession {}: {} to {}", accession.getInstitute().getCode(), accession.getAccessionNumber(), newAcceNumb);
 				accession.setAccessionNumber(newAcceNumb);

src/test/java/org/genesys/test/server/services/AccessionUploaderTest.java

@@ -17,11 +17,7 @@ package org.genesys.test.server.services;
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
 
 import java.util.List;
 
@@ -37,8 +33,8 @@ import org.genesys2.server.persistence.FaoInstituteRepository;
 import org.genesys2.server.persistence.Taxonomy2Repository;
 import org.genesys2.server.service.InstituteService;
 import org.genesys2.server.service.worker.AccessionOpResponse;
-import org.genesys2.server.service.worker.AccessionUploader;
 import org.genesys2.server.service.worker.AccessionOpResponse.UpsertResult;
+import org.genesys2.server.service.worker.AccessionUploader;
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -312,10 +308,28 @@ public class AccessionUploaderTest extends AbstractServicesTest {
 		assertThat(accessionRepository.count(), is(1l));
 		assertThat(op.size(), is(1));
 		assertThat(op.get(0).getError(), notNullValue());
-		assertThat(op.get(0).getError(), is("DOI not included for accession with registered DOI=10.1000/Test"));
+		assertThat(op.get(0).getError(), is("DOI not included for accession with registered DOI=" + DOI_1));
 	}
 	
 
+	/**
+	 * Updates to accession with existing DOI must always include the DOI
+	 */
+	@Test
+	public void testRenameAccession() {
+		accessionUploader.upsertAccessions(institute, upsertAccessions(institute.getCode(), ACCENUMB_1, GENUS_1, DOI_1));
+		Accession a1 = accessionRepository.findByDoi(DOI_1);
+		assertThat(a1.getDoi(), is(DOI_1));
+		assertThat(a1.getAccessionNumber(), is(ACCENUMB_1));
+		
+		ArrayNode withNewAcceNumb = upsertAccessions(institute.getCode(), ACCENUMB_1, GENUS_2, DOI_1);
+		((ObjectNode)withNewAcceNumb.get(0)).put("newAcceNumb", ACCENUMB_2);
+		accessionUploader.upsertAccessions(institute, withNewAcceNumb);
+		Accession a2 = accessionRepository.findByDoi(DOI_1);
+		assertThat(a2.getAccessionNumber(), is(ACCENUMB_2));
+		assertThat(a2.getGenus(), is(GENUS_1));
+	}
+	
 	/**
 	 * Updating accession number and taxonomy must work with DOI only.
 	 */
@@ -324,14 +338,37 @@ public class AccessionUploaderTest extends AbstractServicesTest {
 		accessionUploader.upsertAccessions(institute, upsertAccessions(institute.getCode(), ACCENUMB_1, GENUS_1, DOI_1));
 		Accession a1 = accessionRepository.findOne(institute, null, ACCENUMB_1, GENUS_1);
 
-		accessionUploader.upsertAccessions(institute, upsertAccessions(institute.getCode(), ACCENUMB_2, GENUS_2, DOI_1));
+		accessionUploader.upsertAccessions(institute, upsertAccessions(institute.getCode(), ACCENUMB_1, GENUS_2, DOI_1));
 		assertThat(accessionRepository.count(), is(1l));
 		Accession a2 = accessionRepository.findByDoi(DOI_1);
 		assertThat(a2.getId(), equalTo(a1.getId()));
 		assertThat(a2.getUuid(), equalTo(a1.getUuid()));
 		// Genus not updated, missing species
 		assertThat(a2.getGenus(), is(GENUS_1));
-		assertThat(a2.getAccessionNumber(), is(ACCENUMB_2));
+		assertThat(a2.getAccessionNumber(), is(ACCENUMB_1));
+		
+		ArrayNode withSpecies = upsertAccessions(institute.getCode(), ACCENUMB_1, GENUS_2, DOI_1);
+		ObjectNode taxa = (ObjectNode) withSpecies.get(0).get("taxonomy");
+		taxa.put("species", "hello");
+		accessionUploader.upsertAccessions(institute, withSpecies);
+		a2 = accessionRepository.findByDoi(DOI_1);
+		// Genus updated!
+		assertThat(a2.getGenus(), is(GENUS_2));
+		assertThat(a2.getTaxonomy().getSpecies(), is("hello"));
+	}
+
+	/**
+	 * Updating accession number must NOT work with DOI only.
+	 */
+	@Test
+	public void testComplainWrongAccenumbWithDOI() {
+		accessionUploader.upsertAccessions(institute, upsertAccessions(institute.getCode(), ACCENUMB_1, GENUS_1, DOI_1));
+		Accession a1 = accessionRepository.findOne(institute, null, ACCENUMB_1, GENUS_1);
+		assertThat(a1, notNullValue());
+
+		List<AccessionOpResponse> res = accessionUploader.upsertAccessions(institute, upsertAccessions(institute.getCode(), ACCENUMB_2, GENUS_2, DOI_1));
+		assertThat(res.get(0).getResult().getAction(), is(UpsertResult.Type.ERROR));
+		assertThat(res.get(0).getError(), is("ACCENUMB for doi=" + DOI_1 + " does not match current " + ACCENUMB_1));
 	}
 
 	/**