Commit dca03d2c authored by Matija Obreza's avatar Matija Obreza

Fix: `newAcceNumb` should not be ignored

parent 9de7d3e2
......@@ -186,8 +186,8 @@ public class AccessionUploader implements InitializingBean {
throw new InvalidApiUsageException("dataProviderId must begin with '" + dataProviderIdPrefix + "'");
}
}
accessions.add(accession);
responses.add(new AccessionOpResponse(accession));
accessions.add(accession);
responses.add(new AccessionOpResponse(accession));
} catch (IOException e) {
LOG.error("Could not parse input: {}", e.getMessage(), e);
......@@ -424,8 +424,8 @@ public class AccessionUploader implements InitializingBean {
private void updateAccessionNumber(JsonNode nodeAcceNumb, JsonNode nodeNewNumb, JsonNode nodeDataProviderId, Accession accession) {
String accessionNumber = nodeAcceNumb.textValue();
String newAcceNumb = nodeNewNumb == null ? null : nodeNewNumb.textValue();
String dataProviderId = nodeDataProviderId == null ? null : nodeDataProviderId.textValue();
String newAcceNumb = nodeNewNumb == null ? null : nodeNewNumb.textValue().trim();
String dataProviderId = nodeDataProviderId == null ? null : nodeDataProviderId.textValue().trim();
if (accession.isNew()) {
if (newAcceNumb != null) {
......@@ -435,14 +435,18 @@ public class AccessionUploader implements InitializingBean {
accession.setAccessionNumber(accessionNumber);
} else if (accession.getDoi() != null) {
// When accession has a DOI, override accession number
accession.setAccessionNumber(accessionNumber);
// 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());
}
} else if (dataProviderId != null && accession.getDataProviderId().equals(dataProviderId)) {
// When accession has dataProviderId, update accession number
accession.setAccessionNumber(accessionNumber);
} else if (newAcceNumb != null) {
}
if (StringUtils.isNotBlank(newAcceNumb)) {
if (! StringUtils.equals(newAcceNumb, accession.getAccessionNumber())) {
LOG.warn("Renaming accession {}: {} to {}", accession.getInstitute().getCode(), accession.getAccessionNumber(), newAcceNumb);
accession.setAccessionNumber(newAcceNumb);
......
......@@ -73,7 +73,7 @@ public class AccessionUploaderTest extends AbstractServicesTest {
private InstituteService instituteService;
@Autowired
private PartnerService partnerService;
@Autowired
private FaoInstituteRepository instituteRepository;
@Autowired
......@@ -100,7 +100,7 @@ public class AccessionUploaderTest extends AbstractServicesTest {
@Override
public void beforeTest() throws Exception {
super.beforeTest();
this.partner1 = partnerService.createPartner(new Partner("PARTNER1", "Data provider 1", null, null, null, null, null));
this.partner2 = partnerService.createPartner(new Partner("PARTNER2", "Data provider 2", null, null, null, null, null));
......@@ -224,7 +224,7 @@ public class AccessionUploaderTest extends AbstractServicesTest {
List<AccessionOpResponse> op = accessionUploader.upsertAccessions(institute, upsertAccessions(institute2.getCode(), ACCENUMB_1, GENUS_1, null));
assertThat(op.get(1).getError(), is("Accession does not belong to institute " + institute.getCode()));
}
/**
* Inserting new accession with a DOI must work.
......@@ -338,13 +338,31 @@ public class AccessionUploaderTest extends AbstractServicesTest {
assertThat(a1.getDoi(), is(DOI_1));
List<AccessionOpResponse> op = accessionUploader.upsertAccessions(institute, upsertAccessions(institute.getCode(), ACCENUMB_1, GENUS_1, null));
assertThat(accessionRepository.count(), is(3l));
assertThat(op.size(), is(3));
assertThat(op.get(1).getError(), notNullValue());
assertThat(op.get(1).getError(), is("DOI not included for accession with registered DOI=10.1000/Test"));
assertThat(accessionRepository.count(), is(3l));
assertThat(op.size(), is(3));
assertThat(op.get(1).getError(), notNullValue());
assertThat(op.get(1).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(1)).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.
*/
......@@ -353,14 +371,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(5l));
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(1).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(1).getResult().getAction(), is(UpsertResult.Type.ERROR));
assertThat(res.get(1).getError(), is("ACCENUMB for doi=" + DOI_1 + " does not match current " + ACCENUMB_1));
}
/**
......@@ -441,7 +482,7 @@ public class AccessionUploaderTest extends AbstractServicesTest {
assertNull(a2);
}
@Test
@Test
public void testDeleteAccessionsWithDoiShouldFail() {
accessionUploader.upsertAccessions(institute, upsertAccessions(institute.getCode(), ACCENUMB_1, GENUS_1, DOI_1));
Accession a1 = accessionRepository.findByInstituteCodeAndAccessionNumber(institute.getCode(), ACCENUMB_1);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment