Commit d5fd3b46 authored by Matija Obreza's avatar Matija Obreza

Merge branch '416-update-of-accession-aliases'

* 416-update-of-accession-aliases:
  FaoInstitute#uniqueAcceNumbs: assume false when no records exist in Genesys
  Accession Uploader updates for WIEWS code validation
  Update of Accession Aliases
parents e532e16b 13d20ec6
......@@ -23,6 +23,7 @@ import java.util.Set;
import javax.persistence.PersistenceException;
import javax.persistence.RollbackException;
import javax.validation.ConstraintViolationException;
import org.apache.commons.lang3.RandomUtils;
import org.apache.commons.lang3.StringUtils;
......@@ -254,7 +255,10 @@ public class AccessionController extends ApiBaseController {
response.addAll(uploader.upsertAccessions(institute, single));
lastException = null;
break;
} catch (TransactionException | RollbackException | CannotAcquireLockException | DataIntegrityViolationException | PleaseRetryException e) {
} catch (DataIntegrityViolationException | ConstraintViolationException e) {
lastException = e;
break; // don't retry
} catch (TransactionException | RollbackException | CannotAcquireLockException | PleaseRetryException e) {
lastException = e;
if (tryCount > 1) {
LOG.info("Retry {} of {} tries due to: {}", tryCount, UPLOAD_RETRIES, e.getMessage());
......@@ -266,7 +270,10 @@ public class AccessionController extends ApiBaseController {
}
}
if (lastException != null) {
LOG.warn("Upsert failed due to: {} data={}", lastException.getMessage(), update, lastException);
LOG.warn("Upsert failed due to: {} data={}", lastException.getMessage(), update);
if (LOG.isDebugEnabled()) {
LOG.debug("Caused by {}", lastException.getMessage(), lastException);
}
// record error
response.add(new AccessionOpResponse(update.get("instituteCode").asText(), update.get("accessionNumber").asText(), update.get("taxonomy").get("genus").asText())
.setResult(new UpsertResult(UpsertResult.Type.ERROR)).setError(lastException.getMessage()));
......
......@@ -48,7 +48,8 @@ public class AccessionAlias extends BasicModel implements AccessionRelated, Self
private static final Pattern HAS_WORD_CHARACTER = Pattern.compile("\\w", Pattern.MULTILINE);
private static Pattern USED_BY_PATTERN = Pattern.compile("^((\\p{Alnum}+):(.+))?$");
// INSTCODE:alias
private static Pattern USED_BY_PATTERN = Pattern.compile("^(([A-Z]{3}\\d{3,4}):(.+))?$");
public static enum AliasType {
ACCENAME(0), DONORNUMB(1), BREDNUMB(2), DATABASEID(3), LOCALNAME(4), OTHERNUMB(5), COLLNUMB(6);
......@@ -97,8 +98,8 @@ public class AccessionAlias extends BasicModel implements AccessionRelated, Self
private String lang;
@javax.validation.constraints.Pattern(regexp = "[A-Z]{3}\\d{3,4}")
@Size(max = 64)
@Column(length = 64)
@Size(max = 7)
@Column(length = 7)
private String usedBy;
public AccessionAlias() {
......
......@@ -33,6 +33,7 @@ import javax.persistence.OneToOne;
import javax.persistence.PrePersist;
import javax.persistence.PreUpdate;
import javax.persistence.Transient;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Pattern;
import javax.validation.constraints.Size;
......@@ -64,6 +65,7 @@ public abstract class AccessionData extends AuditedVersionedModel implements IdU
private static final long serialVersionUID = -3428918862058441943L;
@Valid
@MapsId
@OneToOne(cascade = { CascadeType.PERSIST, CascadeType.MERGE }, fetch = FetchType.EAGER, optional = false, orphanRemoval = false)
@JoinColumn(name = "id")
......@@ -176,9 +178,9 @@ public abstract class AccessionData extends AuditedVersionedModel implements IdU
@Type(type = "org.hibernate.type.TextType")
private String otherIds;
@Size(max = 9)
@Size(max = 7)
@Pattern(regexp = "[A-Z]{3}\\d{3,4}")
@Column(name = "donorCode", length = 9)
@Column(name = "donorCode", length = 7)
private String donorCode;
@Size(max = 300)
......
......@@ -94,6 +94,11 @@ public class AccessionGeo extends BasicModel implements GeoReferencedEntity, Acc
this.longitude = null;
this.latitude = null;
}
if (this.longitude == null && this.latitude == null) {
this.datum = null;
this.uncertainty = null;
this.method = null;
}
tileIndex = WorldClimUtil.getWorldclim25Tile(this.longitude, this.latitude);
}
......@@ -189,13 +194,7 @@ public class AccessionGeo extends BasicModel implements GeoReferencedEntity, Acc
@JsonIgnore
public boolean isEmpty() {
if (StringUtils.isNotBlank(datum)) {
return false;
}
if (StringUtils.isNotBlank(method)) {
return false;
}
if (this.latitude != null || this.longitude != null || this.elevation != null || this.uncertainty != null) {
if (this.latitude != null || this.longitude != null || this.elevation != null) {
return false;
}
return true;
......
......@@ -61,6 +61,9 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonView;
import com.fasterxml.jackson.annotation.ObjectIdGenerators;
import cz.jirutka.validator.collection.constraints.EachPattern;
import cz.jirutka.validator.collection.constraints.EachSize;
/**
* Entity holds the assigned accession identifiers regardless of active or
* historic records.
......@@ -107,7 +110,9 @@ public class AccessionId extends AuditedVersionedModel implements IdUUID {
@Field(type = FieldType.Auto, index = FieldIndex.not_analyzed)
private Set<Integer> storage;
@Column(name = "duplSite", nullable = false, length = 9)
@EachSize(max = 7)
@EachPattern(regexp = "[A-Z]{3}\\d{3,4}")
@Column(name = "duplSite", nullable = false, length = 7)
@ElementCollection(fetch = FetchType.LAZY)
@CollectionTable(name = "accession_duplsite", joinColumns = @JoinColumn(name = "accessionId", referencedColumnName = "id"))
@OrderBy("duplSite")
......@@ -129,14 +134,18 @@ public class AccessionId extends AuditedVersionedModel implements IdUUID {
@OneToMany(mappedBy = "accession", cascade = { CascadeType.ALL }, fetch = FetchType.LAZY, orphanRemoval = true)
@JsonIgnoreProperties({ "accession" })
@Field(type = FieldType.Nested)
@OrderBy("id")
private List<AccessionAlias> aliases;
@OneToMany(mappedBy = "accession", cascade = { CascadeType.ALL }, fetch = FetchType.LAZY, orphanRemoval = true)
@JsonIgnoreProperties({ "accession" })
@Field(type = FieldType.Nested)
@OrderBy("id")
private List<AccessionRemark> remarks;
@Column(name = "breederCode", nullable = false, length = 9)
@EachSize(max = 7)
@EachPattern(regexp = "[A-Z]{3}\\d{3,4}")
@Column(name = "breederCode", nullable = false, length = 7)
@ElementCollection(fetch = FetchType.LAZY)
@CollectionTable(name = "accession_breedercode", joinColumns = @JoinColumn(name = "accessionId", referencedColumnName = "id"))
private Set<String> breederCode;
......
......@@ -163,6 +163,9 @@ public interface AccessionRepository extends JpaRepository<Accession, Long>, Acc
@Modifying
void removeDOIs();
@Query("select count(distinct a.accessionNumber) from Accession a where a.institute.id = ?1")
public long countUniqueAccessionNumbers(long instituteId);
@Query("select count(a.id) - count(distinct a.accessionNumber) from Accession a where a.institute.id = ?1")
public long countNonuniqueAccessionNumbers(long instituteId);
}
......@@ -184,7 +184,14 @@ public class InstituteServiceImpl implements InstituteService {
public List<FaoInstitute> update(final @Valid Collection<FaoInstitute> institutes) {
institutes.forEach(institute -> {
if (institute.getId() != null) {
institute.setUniqueAcceNumbs(0 == accessionRepository.countNonuniqueAccessionNumbers(institute.getId()));
long uniqueAcceNumbs = accessionRepository.countUniqueAccessionNumbers(institute.getId());
if (uniqueAcceNumbs > 0) {
long nonUniqueAcceNumbs = accessionRepository.countNonuniqueAccessionNumbers(institute.getId());
institute.setUniqueAcceNumbs(0 == nonUniqueAcceNumbs);
} else {
// Assume first insert into Genesys may contain duplicates
institute.setUniqueAcceNumbs(false);
}
}
});
return instituteRepository.save(institutes);
......
......@@ -27,6 +27,10 @@ import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import javax.validation.ConstraintViolation;
import javax.validation.ConstraintViolationException;
import javax.validation.Validator;
import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.time.StopWatch;
......@@ -73,8 +77,6 @@ import com.fasterxml.jackson.databind.node.ArrayNode;
import com.google.common.collect.Iterators;
import javassist.util.proxy.Proxy;
import javax.validation.ConstraintViolation;
import javax.validation.Validator;
/**
* The Class AccessionUploader.
......@@ -107,7 +109,7 @@ public class AccessionUploader implements InitializingBean {
@Autowired
private Validator validator;
private static final String[] ACCESSIONID_FIELDS = { "breederCode", "storage", "duplSite" };
private static final String[] ACCESSIONID_FIELDS = { "breederCode", "breederName", "storage", "duplSite" };
@Override
public void afterPropertiesSet() throws Exception {
......@@ -131,9 +133,9 @@ public class AccessionUploader implements InitializingBean {
* @param updates the updates
* @throws IOException
*/
@Transactional(timeout = 250, isolation = Isolation.READ_UNCOMMITTED, rollbackFor = Throwable.class)
@Transactional(timeout = 250, isolation = Isolation.READ_UNCOMMITTED, rollbackFor = Exception.class)
@PreAuthorize("hasRole('ADMINISTRATOR') or hasPermission(#institute, 'WRITE')")
public List<AccessionOpResponse> upsertAccessions(FaoInstitute institute, ArrayNode updates) {
public List<AccessionOpResponse> upsertAccessions(FaoInstitute institute, ArrayNode updates) throws InvalidApiUsageException, ConstraintViolationException {
assert (updates.isArray());
StopWatch stopWatch = StopWatch.createStarted();
ObjectReader reader = objectMapper.readerFor(Accession.class);
......@@ -321,6 +323,9 @@ public class AccessionUploader implements InitializingBean {
}
}
});
// Fixes multiple donor alias entries
updateDonorAlias(accession);
return accession;
}
......@@ -369,6 +374,37 @@ public class AccessionUploader implements InitializingBean {
}
}
/**
* Keep only one DONORNUMB type alias
*
* @param accession the accession
*/
private void updateDonorAlias(Accession accession) {
AccessionId accessionId = accession.getAccessionId();
final List<AccessionAlias> allAliases = accessionId.getAliases() != null ? accessionId.getAliases() : new ArrayList<>();
List<AccessionAlias> donorAliases = allAliases.stream().filter(al -> AliasType.DONORNUMB == al.getAliasType()).collect(Collectors.toList());
AccessionAlias donorAlias = donorAliases.size() > 0 ? donorAliases.get(0) : null;
if (donorAlias == null) {
donorAlias = new AccessionAlias();
donorAlias.setAliasType(AliasType.DONORNUMB);
donorAlias.setAccession(accessionId);
allAliases.add(donorAlias);
donorAliases.add(donorAlias);
}
donorAlias.setUsedBy(accession.getDonorCode());
donorAlias.setName(accession.getDonorNumb());
for (int i = 0; i < donorAliases.size(); i++) {
AccessionAlias alias = donorAliases.get(i);
if (alias.isEmpty() || i > 0) {
allAliases.remove(alias);
}
}
accessionId.setAliases(allAliases);
}
private void updateAliases(String fieldName, JsonNode jsonNode, AccessionId accession) {
if (jsonNode == null) {
return;
......
......@@ -292,7 +292,7 @@ public class AccessionControllerTest extends AbstractApiTest {
upsert(accessionJson);
assertThat(accessionRepository.count(), is(1L));
accessionJson.put("breederCode", "BREEDER1");
accessionJson.put("breederCode", "INS003;INS004");
/*@formatter:off*/
upsert(accessionJson)
.andExpect(jsonPath("$[0].result.action", equalTo("UPDATE")));
......@@ -301,7 +301,7 @@ public class AccessionControllerTest extends AbstractApiTest {
assertThat(accessionRepository.count(), is(1L));
UUID uuid = accessionRepository.findAll().get(0).getUuid();
Accession result = fetch(uuid);
assertThat(result.getAccessionId().getBreederCode(), containsInAnyOrder("BREEDER1"));
assertThat(result.getAccessionId().getBreederCode(), containsInAnyOrder("INS003", "INS004"));
accessionJson.set("breederCode", null);
upsert(accessionJson);
......@@ -310,7 +310,7 @@ public class AccessionControllerTest extends AbstractApiTest {
}
@Test
public void testAliases() throws Exception {
public void testAcceNameAlias() throws Exception {
ObjectNode accessionJson = setUpAccession();
upsert(accessionJson);
......@@ -325,6 +325,7 @@ public class AccessionControllerTest extends AbstractApiTest {
assertThat(accessionRepository.count(), is(1L));
UUID uuid = accessionRepository.findAll().get(0).getUuid();
Accession result = fetch(uuid);
result.getAccessionId().getAliases().forEach(alias -> System.err.println(alias));
assertThat(result.getAccessionId().getAliases().size(), is(1));
assertThat(result.getAccessionId().getAliases().get(0).getName(), is("ACCENAME1"));
assertThat(result.getAccessionId().getAliases().get(0).getAliasType(), is(AliasType.ACCENAME));
......@@ -339,14 +340,14 @@ public class AccessionControllerTest extends AbstractApiTest {
* Expecting an error - alias is invalid
*/
@Test
public void createAccessionWithInvalidAlias() throws Exception {
public void updateAccessionWithInvalidBreederCode() throws Exception {
ObjectNode accessionJson = setUpAccession();
upsert(accessionJson);
assertThat(accessionRepository.count(), is(1L));
// TEST012 is invalid value
accessionJson.put("acceName", "INVALID001:IDC0057");
accessionJson.put("breederCode", "INVALID001");
upsertWithErrors(accessionJson);
}
......@@ -361,14 +362,14 @@ public class AccessionControllerTest extends AbstractApiTest {
upsert(accessionJson);
assertThat(accessionRepository.count(), is(1L));
ArrayNode codes = objectMapper.createArrayNode();
ArrayNode collCodes = objectMapper.createArrayNode();
// add one valid code
codes.add("TET012");
collCodes.add("TET012");
// add one invalid code
codes.add("INVALID002");
collCodes.add("INVALID002");
ObjectNode coll = accessionJson.putObject("coll");
coll.putPOJO("collCode", codes);
coll.putPOJO("collCode", collCodes);
upsertWithErrors(accessionJson);
}
......@@ -386,6 +387,81 @@ public class AccessionControllerTest extends AbstractApiTest {
upsertWithErrors(accessionJson);
}
@Test
public void testBreederCodeWithArrayValue() throws Exception {
ObjectNode accessionJson = setUpAccession();
upsert(accessionJson);
assertThat(accessionRepository.count(), is(1L));
ArrayNode bredCodeValues = objectMapper.createArrayNode();
bredCodeValues.add("INS001");
bredCodeValues.add("INS002");
accessionJson.set("breederCode", bredCodeValues);
/*@formatter:off*/
upsert(accessionJson)
.andExpect(jsonPath("$[0].result.action", equalTo("UPDATE")));
/*@formatter:on*/
assertThat(accessionRepository.count(), is(1L));
UUID uuid = accessionRepository.findAll().get(0).getUuid();
Accession result = fetch(uuid);
// result.getAccessionId().getBreederCode().forEach(alias -> System.err.println(alias));
assertThat(result.getAccessionId().getBreederCode().size(), is(2));
assertThat(result.getAccessionId().getBreederCode(), containsInAnyOrder("INS001", "INS002"));
accessionJson.set("breederCode", null);
upsert(accessionJson);
result = fetch(uuid);
assertThat("breederNumb must be an empty array", result.getAccessionId().getBreederCode().size(), is(0));
}
@Test
public void testDonorNumbAliases() throws Exception {
ObjectNode accessionJson = setUpAccession();
upsert(accessionJson);
assertThat(accessionRepository.count(), is(1L));
accessionJson.put("donorCode", "ALB004");
accessionJson.put("donorNumb", "IPV0018");
/*@formatter:off*/
upsert(accessionJson).andExpect(jsonPath("$[0].result.action", equalTo("UPDATE")));
/*@formatter:on*/
assertThat(accessionRepository.count(), is(1L));
UUID uuid = accessionRepository.findAll().get(0).getUuid();
Accession result = fetch(uuid);
assertThat(result.getAccessionId().getAliases().size(), is(1));
assertThat(result.getAccessionId().getAliases().get(0).getName(), is("IPV0018"));
assertThat(result.getAccessionId().getAliases().get(0).getUsedBy(), is("ALB004"));
assertThat(result.getAccessionId().getAliases().get(0).getAliasType(), is(AliasType.DONORNUMB));
accessionJson.put("donorNumb", "IPV0021");
accessionJson.put("donorCode", "ALB005");
/*@formatter:off*/
upsert(accessionJson)
.andExpect(jsonPath("$[0].result.action", equalTo("UPDATE")));
/*@formatter:on*/
assertThat(accessionRepository.count(), is(1L));
uuid = accessionRepository.findAll().get(0).getUuid();
result = fetch(uuid);
assertThat(result.getAccessionId().getAliases().size(), is(1));
assertThat(result.getAccessionId().getAliases().get(0).getName(), is("IPV0021"));
assertThat(result.getAccessionId().getAliases().get(0).getUsedBy(), is("ALB005"));
assertThat(result.getAccessionId().getAliases().get(0).getAliasType(), is(AliasType.DONORNUMB));
accessionJson.set("donorNumb", null);
upsert(accessionJson);
result = fetch(uuid);
assertThat("donorNumb must be an empty array", result.getAccessionId().getAliases().size(), is(0));
}
@Test
public void testCollNumb() throws Exception {
ObjectNode accessionJson = setUpAccession();
......@@ -394,7 +470,7 @@ public class AccessionControllerTest extends AbstractApiTest {
assertThat(accessionRepository.count(), is(1L));
ObjectNode coll = accessionJson.putObject("coll");
coll.put("collNumb", "COLLNAME1");
coll.put("collNumb", "COLLNUMB1");
/*@formatter:off*/
upsert(accessionJson)
.andExpect(jsonPath("$[0].result.action", equalTo("UPDATE")));
......@@ -403,9 +479,10 @@ public class AccessionControllerTest extends AbstractApiTest {
assertThat(accessionRepository.count(), is(1L));
UUID uuid = accessionRepository.findAll().get(0).getUuid();
Accession result = fetch(uuid);
assertThat(result.getAccessionId().getColl().getCollNumb(), is("COLLNAME1"));
assertThat(result.getAccessionId().getColl().getCollNumb(), is("COLLNUMB1"));
result.getAccessionId().getAliases().forEach(alias -> System.err.println(alias));
assertThat(result.getAccessionId().getAliases().size(), is(1));
assertThat(result.getAccessionId().getAliases().get(0).getName(), is("COLLNAME1"));
assertThat(result.getAccessionId().getAliases().get(0).getName(), is("COLLNUMB1"));
assertThat(result.getAccessionId().getAliases().get(0).getAliasType(), is(AliasType.COLLNUMB));
coll.set("collNumb", null);
......
......@@ -298,8 +298,8 @@ public class AccessionControllerTest extends AbstractApiTest {
assertThat(accessionRepository.count(), is(1L));
ArrayNode arr = accessionJson.putArray("breederCode");
arr.add("BREEDER1");
arr.add("BREEDER2");
arr.add("MEX008");
arr.add("SVN009");
/*@formatter:off*/
upsert(accessionJson)
.andExpect(jsonPath("$[0].result.action", equalTo("UPDATE")));
......@@ -308,7 +308,7 @@ public class AccessionControllerTest extends AbstractApiTest {
assertThat(accessionRepository.count(), is(1L));
UUID uuid = accessionRepository.findAll().get(0).getUuid();
Accession result = fetch(uuid);
assertThat(result.getAccessionId().getBreederCode(), containsInAnyOrder("BREEDER1", "BREEDER2"));
assertThat(result.getAccessionId().getBreederCode(), containsInAnyOrder("MEX008", "SVN009"));
accessionJson.set("breederCode", null);
upsert(accessionJson);
......@@ -317,6 +317,21 @@ public class AccessionControllerTest extends AbstractApiTest {
}
@Test
public void testInvalidBreederCode() throws Exception {
ObjectNode accessionJson = setUpAccession();
upsert(accessionJson);
assertThat(accessionRepository.count(), is(1L));
ArrayNode arr = accessionJson.putArray("breederCode");
arr.add("BREEDER1");
arr.add("BREEDER2");
/*@formatter:off*/
upsertWithErrors(accessionJson);
}
@Test
public void testGeoJson() throws Exception {
final int accAmount = 5;
......@@ -433,6 +448,19 @@ public class AccessionControllerTest extends AbstractApiTest {
.andExpect(jsonPath("$[0].error", nullValue()));
/*@formatter:on*/
}
private ResultActions upsertWithErrors(ObjectNode accessionJson) throws Exception, JsonProcessingException {
LOG.debug("Upsering {}", verboseMapper.writerWithDefaultPrettyPrinter().writeValueAsString(accessionJson));
/*@formatter:off*/
return mockMvc.perform(post(AccessionUploadController.CONTROLLER_URL + "/" + institute.getCode() + "/upsert")
.contentType(MediaType.APPLICATION_JSON)
.content("[" + verboseMapper.writeValueAsString(accessionJson) + "]"))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8))
.andExpect(jsonPath("$", not(nullValue())))
.andExpect(jsonPath("$[0].error", not(nullValue())));
/*@formatter:on*/
}
private Accession fetch(UUID uuid) throws Exception, JsonProcessingException {
/*@formatter:off*/
......
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