Commit 03fb6b4c authored by Matija Obreza's avatar Matija Obreza

Fix: VocabularyTerms remain in the database, terms disappear from ControllledVocabularies

- Unit tests added
- liquibase to remove unreferenced Terms
parent 88ed31ba
......@@ -46,7 +46,7 @@ import io.swagger.annotations.ApiOperation;
public class GeoController {
/** The Constant API_BASE. */
protected static final String CONTROLLER_URL = ApiBaseController.APIv0_BASE + "/geo";
public static final String CONTROLLER_URL = ApiBaseController.APIv0_BASE + "/geo";
/** The Constant ISO3166_2ALPHA. */
public static final UUID ISO3166_2ALPHA = ISO3166VocabularyUpdater.ISO3166_2ALPHA;
......
......@@ -51,7 +51,7 @@ import io.swagger.annotations.Api;
public class LanguagesController {
/** The Constant API_BASE. */
protected static final String CONTROLLER_URL = ApiBaseController.APIv0_BASE + "/lang";
public static final String CONTROLLER_URL = ApiBaseController.APIv0_BASE + "/lang";
/** The Constant ISO639_3. */
public static final UUID ISO639_3 = ISO639VocabularyUpdater.ISO639_3;
......
......@@ -171,7 +171,7 @@ public class Descriptor extends UuidModel implements SelfCleaning, Publishable,
/**
* Vocabulary terms specific to this descriptor (99% of the cases).
*/
@OneToMany(fetch = FetchType.LAZY, cascade = { CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REFRESH }, orphanRemoval = true)
@OneToMany(fetch = FetchType.LAZY, cascade = { CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REMOVE, CascadeType.REFRESH }, orphanRemoval = true)
@JoinTable(name = "descriptor_term", joinColumns = @JoinColumn(name = "descriptorId"),
// other side
inverseJoinColumns = @JoinColumn(name = "termId", unique = true),
......
......@@ -82,7 +82,7 @@ public class ControlledVocabulary extends UuidModel implements Publishable, Self
private String termUrlPrefix;
/** The terms. */
@OneToMany(fetch = FetchType.LAZY, cascade = { CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REFRESH }, orphanRemoval = true)
@OneToMany(fetch = FetchType.LAZY, cascade = { CascadeType.REFRESH })
@JoinTable(name = "vocabulary_term", joinColumns = @JoinColumn(name = "vocabularyId"),
// other side
inverseJoinColumns = @JoinColumn(name = "termId", unique = true),
......@@ -112,6 +112,8 @@ public class ControlledVocabulary extends UuidModel implements Publishable, Self
@PreUpdate
private void preupdate() {
trimStringsToNull();
this.termCount = this.terms == null ? 0 : this.terms.size();
}
/**
......@@ -291,7 +293,7 @@ public class ControlledVocabulary extends UuidModel implements Publishable, Self
*
* @param termCount the new term count
*/
public void setTermCount(final int termCount) {
protected void setTermCount(final int termCount) {
this.termCount = termCount;
}
}
......@@ -18,6 +18,7 @@ package org.genesys.catalog.service.impl;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import java.util.stream.Collectors;
import javax.persistence.EntityManager;
......@@ -79,12 +80,10 @@ public class VocabularyServiceImpl implements VocabularyService {
controlledVocabulary.setOwner(input.getOwner());
if (input.getTerms() != null && !input.getTerms().isEmpty()) {
updateTerms(input);
controlledVocabulary.setTerms(input.getTerms());
controlledVocabulary.setTerms(persistTerms(input.getTerms()));
} else {
controlledVocabulary.setTerms(new ArrayList<>());
}
controlledVocabulary.setTermCount(controlledVocabulary.getTerms().size());
return vocabRepository.save(controlledVocabulary);
}
......@@ -93,14 +92,10 @@ public class VocabularyServiceImpl implements VocabularyService {
* List
*
* @param vocabulary the vocabulary
* @return
*/
protected void updateTerms(final ControlledVocabulary vocabulary) {
final List<VocabularyTerm> terms = vocabulary.getTerms();
if (terms != null && !terms.isEmpty()) {
final List<VocabularyTerm> r = termRepository.save(terms);
terms.clear();
terms.addAll(r);
}
protected List<VocabularyTerm> persistTerms(final List<VocabularyTerm> terms) {
return termRepository.save(terms);
}
@Override
......@@ -163,15 +158,7 @@ public class VocabularyServiceImpl implements VocabularyService {
vocabulary.setVersionTag(input.getVersionTag());
if (input.getTerms() != null) {
final List<VocabularyTerm> terms = vocabulary.getTerms();
if (terms == null) {
vocabulary.setTerms(new ArrayList<>());
}
terms.clear();
terms.addAll(input.getTerms());
updateTerms(vocabulary);
vocabulary.setTermCount(terms.size());
autoUpdateTerms(vocabulary, input.getTerms());
}
return vocabRepository.save(vocabulary);
......@@ -187,16 +174,7 @@ public class VocabularyServiceImpl implements VocabularyService {
LOG.info("Updating {} vocabulary with {} terms", oldVocabulary.getTitle(), input.getTerms().size());
if (input.getTerms() != null) {
final List<VocabularyTerm> terms = oldVocabulary.getTerms();
updateTerms(terms, input);
if (terms == null) {
oldVocabulary.setTerms(input.getTerms());
} else {
terms.clear();
terms.addAll(input.getTerms());
}
oldVocabulary.setTermCount(oldVocabulary.getTerms().size());
autoUpdateTerms(oldVocabulary, input.getTerms());
}
return vocabRepository.save(oldVocabulary);
......@@ -215,19 +193,24 @@ public class VocabularyServiceImpl implements VocabularyService {
* @param existing
* @param input
*/
private void updateTerms(final List<VocabularyTerm> existing, final ControlledVocabulary input) {
if (existing == null || existing.isEmpty()) {
// Nothing to match against, persist all if them
updateTerms(input);
} else if (input != null && input.getTerms() != null) {
private void autoUpdateTerms(ControlledVocabulary vocabulary, final List<VocabularyTerm> newTerms) {
if (vocabulary.getTerms() == null) {
vocabulary.setTerms(newTerms);
persistTerms(vocabulary.getTerms());
} else if (newTerms != null) {
List<VocabularyTerm> existing = vocabulary.getTerms();
LOG.info("Matching against {} existing terms", existing.size());
// Remove missing ones
List<VocabularyTerm> removedTerms = existing.stream().filter(old -> newTerms.stream().filter(term -> old.getCode().equals(term.getCode())).findFirst().orElse(null) == null).collect(Collectors.toList());
termRepository.delete(removedTerms);
// match existing codes
input.getTerms().forEach(inputTerm -> {
newTerms.forEach(inputTerm -> {
// only when there's a code
if (inputTerm.getCode() != null) {
inputTerm.setId(existing.stream()
// the ones with codes only
.filter(existingTerm -> existingTerm != null && existingTerm.getCode() != null)
......@@ -243,6 +226,8 @@ public class VocabularyServiceImpl implements VocabularyService {
}
}
});
vocabulary.setTerms(persistTerms(newTerms));
}
}
......
......@@ -4386,6 +4386,13 @@ databaseChangeLog:
defaultValue: false
tableName: crop
- changeSet:
id: 1539975792-1
author: mobreza
comment: delete unreferenced terms
changes:
- sql:
- sql: delete t from term t left outer join vocabulary_term vt on vt.termId=t.id left outer join descriptor_term dt on dt.termId=t.id where dt.descriptorId is null and vt.vocabularyId is null;
# ENABLE AFTER SOME TIME
# - changeSet:
......
/*
* Copyright 2017 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.genesys.test.catalog.api.v0;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.assertThat;
import static org.springframework.restdocs.mockmvc.MockMvcRestDocumentation.documentationConfiguration;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import org.apache.commons.lang3.RandomUtils;
import org.genesys.catalog.api.v0.GeoController;
import org.genesys.catalog.api.v0.LanguagesController;
import org.genesys.catalog.api.v0.WiewsController;
import org.genesys.catalog.model.vocab.QVocabularyTerm;
import org.genesys.catalog.model.vocab.VocabularyTerm;
import org.genesys.catalog.persistence.vocab.ControlledVocabularyRepository;
import org.genesys.catalog.persistence.vocab.VocabularyTermRepository;
import org.genesys.test.base.AbstractApiTest;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.web.context.WebApplicationContext;
/**
* Debugging disappearing terms.
*
* @author Matija Obreza
*/
@Ignore
public class VocabularyControllerTest extends AbstractApiTest {
@Autowired
private WebApplicationContext webApplicationContext;
@Autowired
private VocabularyTermRepository termRepository;
@Autowired
private ControlledVocabularyRepository vocabularyRepository;
private MockMvc mockMvc;
@Before
public void beforeTest() throws Exception {
super.beforeTest();
mockMvc = MockMvcBuilders.webAppContextSetup(webApplicationContext).apply(documentationConfiguration(restDocumentation).uris().withScheme("https").withHost(
"api.catalog.genesys-pgr.org").withPort(443)).build();
}
@After
@Override
public void cleanup() throws Exception {
termRepository.deleteAll();
vocabularyRepository.deleteAll();
assertThat(termRepository.count(), is(0l));
assertThat(vocabularyRepository.count(), is(0l));
super.cleanup();
}
@Test
public void autoUpdateLang() throws Exception {
/*@formatter:off*/
this.mockMvc
.perform(RestDocumentationRequestBuilders.post(LanguagesController.CONTROLLER_URL.concat("/update")))
// .andDo(MockMvcResultHandlers.print())
.andExpect(status().isOk());
/*@formatter:on*/
assertThat(termRepository.count(), greaterThan(0l));
}
@Test
public void multiUpdateLang() throws Exception {
assertThat(termRepository.count(), is(0l));
autoUpdateLang();
long totalTerms = termRepository.count();
autoUpdateLang();
assertThat(termRepository.count(), is(totalTerms));
assertThat(termRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
autoUpdateLang();
autoUpdateLang();
autoUpdateLang();
assertThat(termRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(termRepository.count(), is(totalTerms));
autoUpdateLang();
assertThat(termRepository.count(), is(totalTerms));
}
@Test
public void autoUpdateGeo() throws Exception {
/*@formatter:off*/
this.mockMvc
.perform(RestDocumentationRequestBuilders.post(GeoController.CONTROLLER_URL.concat("/update")))
// .andDo(MockMvcResultHandlers.print())
.andExpect(status().isOk());
/*@formatter:on*/
assertThat(termRepository.count(), greaterThan(0l));
}
@Test
public void multiUpdateGeo() throws Exception {
assertThat(termRepository.count(), is(0l));
autoUpdateGeo();
long totalTerms = termRepository.count();
autoUpdateGeo();
assertThat(termRepository.count(), is(totalTerms));
autoUpdateGeo();
assertThat(termRepository.count(), is(totalTerms));
autoUpdateGeo();
assertThat(termRepository.count(), is(totalTerms));
autoUpdateGeo();
assertThat(termRepository.count(), is(totalTerms));
autoUpdateGeo();
assertThat(termRepository.count(), is(totalTerms));
}
@Test
public void autoUpdateWiews() throws Exception {
/*@formatter:off*/
this.mockMvc
.perform(RestDocumentationRequestBuilders.post(WiewsController.CONTROLLER_URL.concat("/update")))
// .andDo(MockMvcResultHandlers.print())
.andExpect(status().isOk());
/*@formatter:on*/
assertThat(termRepository.count(), greaterThan(0l));
}
@Test
public void autoUpdateAll() throws Exception {
long totalTerms = 0;
autoUpdateWiews();
totalTerms = termRepository.count();
autoUpdateGeo();
totalTerms = termRepository.count();
autoUpdateLang();
totalTerms = termRepository.count();
assertThat(termRepository.count(), greaterThan(0l));
PageRequest page = new PageRequest(0, 100);
vocabularyRepository.findAll().forEach(vocabulary -> {
Page<VocabularyTerm> p = vocabularyRepository.listVocabularyTerms(vocabulary, page);
assertThat(p.getSize(), greaterThan(0));
assertThat(p.getTotalElements(), greaterThan((long) p.getSize()));
});
for (int i = 0; i < 20; i++) {
int x = RandomUtils.nextInt(0, 3);
switch (x) {
case 0:
// autoUpdateWiews();
break;
case 1:
autoUpdateLang();
break;
case 2:
autoUpdateGeo();
break;
}
assertThat(totalTerms, is(termRepository.count()));
assertThat(termRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
}
}
}
......@@ -24,6 +24,7 @@ import java.util.List;
import java.util.UUID;
import org.genesys.catalog.model.vocab.ControlledVocabulary;
import org.genesys.catalog.model.vocab.QVocabularyTerm;
import org.genesys.catalog.model.vocab.VocabularyTerm;
import org.genesys.catalog.persistence.vocab.ControlledVocabularyRepository;
import org.junit.Test;
......@@ -50,12 +51,15 @@ public class ControlledVocabularyTest extends CatalogServiceTest {
private static final String TERM_TITLE_1 = "Term title 1";
private static final String TERM_TITLE_2 = "Term title 2";
private static final UUID VOCABULARY_UUID = UUID.randomUUID();
@Autowired
private ControlledVocabularyRepository vocabRepository;
@Override
public void cleanup() throws Exception {
vocabRepository.deleteAll();
vocabularyTermRepository.deleteAll();
super.cleanup();
}
......@@ -242,6 +246,7 @@ public class ControlledVocabularyTest extends CatalogServiceTest {
ControlledVocabulary result = vocabularyService.createVocabulary(vocab1);
assertThat(result.getUuid(), is(uuid));
assertThat(result.getTerms(), hasSize(0));
assertThat(vocabularyTermRepository.count(), is(0l));
// add 2 terms
ArrayList<VocabularyTerm> terms = Lists.newArrayList(VocabularyTerm.fromData(TERM_CODE_1, TERM_TITLE_1), VocabularyTerm.fromData(TERM_CODE_2, TERM_TITLE_2));
......@@ -249,6 +254,8 @@ public class ControlledVocabularyTest extends CatalogServiceTest {
result.setTerms(terms);
result = vocabularyService.updateVocabulary(result);
assertThat(result.getTerms(), hasSize(2));
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(2l));
// add 2 other terms
terms = new ArrayList<>(result.getTerms());
......@@ -258,10 +265,14 @@ public class ControlledVocabularyTest extends CatalogServiceTest {
result.setTerms(terms);
result = vocabularyService.updateVocabulary(result);
assertThat(result.getTerms(), hasSize(4));
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(4l));
// remove listed terms
result.setTerms(new ArrayList<>());
result = vocabularyService.updateVocabulary(result);
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(0l));
result = vocabularyService.getVocabulary(uuid);
assertThat(result.getTerms(), hasSize(0));
......@@ -303,6 +314,8 @@ public class ControlledVocabularyTest extends CatalogServiceTest {
result.setTerms(terms);
result = vocabularyService.updateVocabulary(result);
assertThat(result.getTerms(), hasSize(3));
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(3l));
}
/**
......@@ -373,6 +386,8 @@ public class ControlledVocabularyTest extends CatalogServiceTest {
terms.add(VocabularyTerm.fromData("a" + i, "a" + i));
}
vocabularyService.createVocabulary(vocab1);
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(100l));
ControlledVocabulary result = vocabularyService.getVocabulary(uuid);
assertThat(result, not(nullValue()));
......@@ -386,6 +401,8 @@ public class ControlledVocabularyTest extends CatalogServiceTest {
}
result.setTerms(terms);
vocabularyService.updateVocabulary(result);
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(200l));
result = vocabularyService.getVocabulary(uuid);
assertThat(result, not(nullValue()));
......@@ -406,6 +423,8 @@ public class ControlledVocabularyTest extends CatalogServiceTest {
terms.add(VocabularyTerm.fromData("a" + i, "a" + i));
}
vocabularyService.createVocabulary(vocab1);
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(200l));
final ControlledVocabulary result = vocabularyService.getVocabulary(uuid);
Page<VocabularyTerm> paged = vocabularyService.listTerms(result, new PageRequest(0, 100));
......@@ -420,4 +439,50 @@ public class ControlledVocabularyTest extends CatalogServiceTest {
assertThat(paged.getNumber(), is(1));
assertThat(paged.getSize(), is(100));
}
@Test
public void autoUpdateTest() {
ControlledVocabulary updated = new ControlledVocabulary();
updated.setTitle(VOCABULARY_TITLE_1);
updated.setOwner(partner);
updated.setTerms(new ArrayList<>());
makeTerms(updated.getTerms(), 200);
ControlledVocabulary result = vocabularyService.autoUpdateOrCreateVocabulary(VOCABULARY_UUID, updated);
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(200l));
assertThat(result.getTitle(), is(updated.getTitle()));
assertThat(result.getUuid(), is(VOCABULARY_UUID));
updated.setTitle(VOCABULARY_TITLE_2);
vocabularyService.autoUpdateOrCreateVocabulary(VOCABULARY_UUID, updated);
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(200l));
assertThat(result.getTitle(), not(updated.getTitle())); // title will not be modified
// Clear some terms
makeTerms(updated.getTerms(), 190);
vocabularyService.autoUpdateOrCreateVocabulary(VOCABULARY_UUID, updated);
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(190l));
// More terms
makeTerms(updated.getTerms(), 200);
vocabularyService.autoUpdateOrCreateVocabulary(VOCABULARY_UUID, updated);
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(200l));
// No terms
updated.setTerms(new ArrayList<>());
vocabularyService.autoUpdateOrCreateVocabulary(VOCABULARY_UUID, updated);
assertThat(vocabularyTermRepository.count(QVocabularyTerm.vocabularyTerm.vocabulary.isNull()), is(0l));
assertThat(vocabularyTermRepository.count(), is(0l));
}
private void makeTerms(List<VocabularyTerm> terms, int count) {
terms.clear();
for (int i = 0; i < count; i++) {
terms.add(VocabularyTerm.fromData("a" + i, "a" + i));
}
}
}
......@@ -60,6 +60,7 @@ public class DescriptorServiceTest extends CatalogServiceTest {
descriptorListRepository.deleteAll();
descriptorRepository.deleteAll();
partnerRepository.deleteAll();
super.cleanup();
}
......@@ -115,13 +116,24 @@ public class DescriptorServiceTest extends CatalogServiceTest {
@Test(expected = NotFoundElement.class)
public void testRemoveDescriptor() {
final UUID uuid = UUID.randomUUID();
final Descriptor input = setupDescriptor(uuid, DESCRIPTOR_TITLE_1, Category.PASSPORT, DataType.NUMERIC, VERSION_1_0, PublishState.DRAFT);
Descriptor input = setupDescriptor(uuid, DESCRIPTOR_TITLE_1, Category.PASSPORT, DataType.CODED, VERSION_1_0, PublishState.DRAFT);
input.setTerms(Lists.newArrayList(VocabularyTerm.fromData(TERM_CODE_1, TERM_TITLE_1), VocabularyTerm.fromData(TERM_CODE_2, TERM_TITLE_2)));
input.setMinValue(10D);
input.setMaxValue(10D);
Descriptor result = descriptorService.createDescriptor(input);
// Another one
input = setupDescriptor(null, DESCRIPTOR_TITLE_2, Category.PASSPORT, DataType.CODED, VERSION_1_0, PublishState.DRAFT);
input.setTerms(Lists.newArrayList(VocabularyTerm.fromData(TERM_CODE_1, TERM_TITLE_1), VocabularyTerm.fromData(TERM_CODE_2, TERM_TITLE_2)));
input.setMinValue(10D);
input.setMaxValue(10D);
descriptorService.createDescriptor(input);
assertThat(vocabularyTermRepository.count(), is(4l));
result = descriptorService.removeDescriptor(result);
descriptorService.getDescriptor(result.getUuid(), result.getVersion());
assertThat(vocabularyTermRepository.count(), is(2l));
}
......
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