Commit e5e0c608 authored by Matija Obreza's avatar Matija Obreza

Fix: Dataset locations affect Dataset#start-/endDate

- Also reduced log clutter
parent 07edb134
......@@ -19,6 +19,7 @@ import java.io.EOFException;
import javax.servlet.http.HttpServletRequest;
import org.genesys.blocks.security.NotUniqueUserException;
import org.genesys.catalog.exceptions.InvalidApiUsageException;
import org.genesys.catalog.exceptions.NotFoundElement;
import org.genesys.filerepository.FileRepositoryException;
......@@ -131,7 +132,7 @@ public class ApiExceptionHandler {
* @return the api error
*/
@ResponseStatus(code = HttpStatus.BAD_REQUEST)
@ExceptionHandler({ InvalidApiUsageException.class, ConcurrencyFailureException.class, FileRepositoryException.class })
@ExceptionHandler({ InvalidApiUsageException.class, ConcurrencyFailureException.class, FileRepositoryException.class, NotUniqueUserException.class })
@ResponseBody
public ApiError<Exception> handleInvalidApiUsage(final Exception e, final HttpServletRequest request) {
LOG.warn("{} for {} {}", e.getMessage(), request.getMethod(), request.getRequestURL()); //, e);
......
......@@ -86,7 +86,7 @@ public class DatasetCreator extends UuidModel implements PublishValidationInterf
private DatasetCreatorRole role;
/** The dataset. */
@ManyToOne(cascade = { CascadeType.REFRESH, CascadeType.MERGE, CascadeType.PERSIST, CascadeType.DETACH }, optional = false, fetch = FetchType.EAGER)
@ManyToOne(cascade = { CascadeType.REFRESH, CascadeType.MERGE, CascadeType.PERSIST, CascadeType.DETACH }, optional = false, fetch = FetchType.LAZY)
@JoinColumn(name = "datasetId")
@JsonIgnore
private Dataset dataset;
......
......@@ -39,7 +39,7 @@ public class DatasetLocation extends UuidModel implements PublishValidationInter
private static final long serialVersionUID = -3107167591979299613L;
/** The dataset. */
@ManyToOne(cascade = { CascadeType.REFRESH, CascadeType.MERGE, CascadeType.PERSIST, CascadeType.DETACH }, optional = false, fetch = FetchType.EAGER)
@ManyToOne(cascade = { CascadeType.REFRESH, CascadeType.MERGE, CascadeType.PERSIST, CascadeType.DETACH }, optional = false, fetch = FetchType.LAZY)
@JoinColumn(name = "datasetId")
@JsonIgnore
private Dataset dataset;
......
......@@ -295,16 +295,6 @@ public interface DatasetService {
*/
DatasetCreator removeDatasetCreator(Dataset dataset, DatasetCreator input) throws NotFoundElement;
/**
* Removes the dataset creator.
*
* @param dataset the dataset
* @param datasetCreatorUuid the dataset creator uuid
* @return the dataset creator
* @throws NotFoundElement the not found element
*/
DatasetCreator removeDatasetCreator(Dataset dataset, UUID datasetCreatorUuid) throws NotFoundElement;
/**
* Load list DatasetCreators by Dataset UUID.
*
......
......@@ -24,6 +24,7 @@ import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
......@@ -31,7 +32,6 @@ import java.util.UUID;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import org.apache.commons.lang.StringUtils;
import org.genesys.blocks.security.service.CustomAclService;
import org.genesys.catalog.exceptions.InvalidApiUsageException;
......@@ -84,7 +84,6 @@ import com.google.common.collect.Lists;
import com.querydsl.core.BooleanBuilder;
import com.querydsl.core.types.Predicate;
/**
* The Class DatasetServiceImpl.
*/
......@@ -126,7 +125,7 @@ public class DatasetServiceImpl implements DatasetService {
/** The download service. */
@Autowired
private DownloadService downloadService;
/** The file repository path. */
@Value("${file.repository.datasets.folder}")
public String datasetRepositoryPath;
......@@ -606,7 +605,6 @@ public class DatasetServiceImpl implements DatasetService {
return datasetRepository.lastPublished();
}
/**
* {@inheritDoc}
*/
......@@ -626,27 +624,12 @@ public class DatasetServiceImpl implements DatasetService {
@Override
@PreAuthorize("hasRole('ADMINISTRATOR') or hasPermission(#dataset, 'write')")
public DatasetCreator removeDatasetCreator(Dataset dataset, final DatasetCreator input) throws NotFoundElement {
dataset = loadDataset(dataset);
final DatasetCreator datasetCreator = loadDatasetCreator(input);
if (!datasetCreator.getDataset().getUuid().equals(dataset.getUuid())) {
throw new InvalidApiUsageException("Creator does not belong to dataset");
}
dataset.getCreators().remove(datasetCreator);
return datasetCreator;
}
/**
* {@inheritDoc}
*/
@Transactional
@Override
@PreAuthorize("hasRole('ADMINISTRATOR') or hasPermission(#dataset, 'write')")
public DatasetCreator removeDatasetCreator(Dataset dataset, final UUID datasetCreatorUuid) throws NotFoundElement {
dataset = loadDataset(dataset);
final DatasetCreator datasetCreator = loadDatasetCreator(datasetCreatorUuid);
if (!datasetCreator.getDataset().getUuid().equals(dataset.getUuid())) {
throw new InvalidApiUsageException("Creator does not belong to dataset");
}
dataset.getCreators().remove(datasetCreator);
datasetCreatorRepository.delete(datasetCreator);
return datasetCreator;
}
......@@ -718,7 +701,8 @@ public class DatasetServiceImpl implements DatasetService {
@Override
public List<DatasetCreator> autocompleteCreators(final String text) {
final HashSet<Long> ids = new HashSet<>(securityUtils.listObjectIdentityIdsForCurrentUser(Dataset.class, BasePermission.WRITE));
final Predicate predicate = datasetCreator.dataset.id.in(ids).and(datasetCreator.fullName.startsWithIgnoreCase(text).or(datasetCreator.institutionalAffiliation.startsWithIgnoreCase(text)));
final Predicate predicate = datasetCreator.dataset.id.in(ids).and(datasetCreator.fullName.startsWithIgnoreCase(text).or(datasetCreator.institutionalAffiliation
.startsWithIgnoreCase(text)));
return datasetCreatorRepository.findAll(predicate, new PageRequest(0, 20, new Sort("fullName"))).getContent();
}
......@@ -793,19 +777,16 @@ public class DatasetServiceImpl implements DatasetService {
private Set<AccessionRef> lookupMatchingAccessions(final Set<AccessionRef> accessionRefs) {
List<AccessionRef> list = new ArrayList<AccessionRef>(accessionRefs);
List<Accession> foundAccessions = accessionRepository.findById(list);
LOG.info("Found {} matches for {} identifiers", foundAccessions.size(), accessionRefs.size());
accessionRefs.forEach(ref -> {
Accession foundAccession = foundAccessions.stream().filter(a -> {
return StringUtils.equalsIgnoreCase(a.getInstCode(), ref.getInstCode())
&& (
// when DOI provided
(ref.getDoi() != null && a.getDoi() != null && StringUtils.equals(a.getDoi(), ref.getDoi()))
// OR when without DOI
||
(StringUtils.equalsIgnoreCase(a.getAccessionNumber(), ref.getAcceNumb()) && StringUtils.equalsIgnoreCase(a.getGenus(), ref.getGenus()))
);
return StringUtils.equalsIgnoreCase(a.getInstCode(), ref.getInstCode()) && (
// when DOI provided
(ref.getDoi() != null && a.getDoi() != null && StringUtils.equals(a.getDoi(), ref.getDoi()))
// OR when without DOI
|| (StringUtils.equalsIgnoreCase(a.getAccessionNumber(), ref.getAcceNumb()) && StringUtils.equalsIgnoreCase(a.getGenus(), ref.getGenus())));
}).findFirst().orElse(null);
ref.setAccession(foundAccession);
if (foundAccession == null) {
......@@ -816,7 +797,6 @@ public class DatasetServiceImpl implements DatasetService {
return accessionRefs;
}
@Override
@Transactional(readOnly = false)
@PreAuthorize("hasRole('ADMINISTRATOR')")
......@@ -825,7 +805,7 @@ public class DatasetServiceImpl implements DatasetService {
rematchDatasetAccessions(dataset);
});
}
@Override
@Transactional(readOnly = false)
@PreAuthorize("hasRole('ADMINISTRATOR') or hasPermission(#dataset, 'write')")
......@@ -833,7 +813,7 @@ public class DatasetServiceImpl implements DatasetService {
dataset = datasetRepository.findByUuid(dataset.getUuid());
LOG.warn("Linking {} accessions with dataset {}", dataset.getAccessionCount(), dataset.getId());
dataset.setAccessionRefs(lookupMatchingAccessions(dataset.getAccessionRefs()));
return datasetRepository.save(dataset);
return datasetRepository.save(dataset);
}
@Override
......@@ -864,17 +844,16 @@ public class DatasetServiceImpl implements DatasetService {
@Transactional
@PreAuthorize("hasRole('ADMINISTRATOR') or hasPermission(#dataset, 'write')")
public DatasetLocation createLocation(Dataset dataset, final DatasetLocation input) throws NotFoundElement {
dataset = loadDataset(dataset);
dataset = datasetRepository.findByUuidAndVersion(dataset.getUuid(), dataset.getVersion());
LOG.info("Create DatasetLocation {} for dataset {}", input, dataset.getUuid());
input.setDataset(dataset);
DatasetLocation savedLocation = locationRepository.save(input);
DatasetLocation saved = locationRepository.save(input);
dataset.setStartDate(saved.getStartDate());
dataset.setEndDate(saved.getEndDate());
dataset.getLocations().add(saved);
updateDataset(dataset);
dataset = datasetRepository.findByUuidAndVersion(dataset.getUuid(), dataset.getVersion());
dataset.setLastModifiedDate(new Date());
datasetRepository.save(dataset);
return saved;
return savedLocation;
}
/**
......@@ -885,15 +864,18 @@ public class DatasetServiceImpl implements DatasetService {
@PreAuthorize("hasRole('ADMINISTRATOR') or hasPermission(#dataset, 'write')")
public DatasetLocation removeLocation(Dataset dataset, final DatasetLocation input) throws NotFoundElement {
LOG.info("Remove DatasetLocation {} of dataset {}", input, dataset.getUuid());
dataset = loadDataset(dataset);
final DatasetLocation datasetLocation = loadLocation(input);
if (!datasetLocation.getDataset().getUuid().equals(dataset.getUuid())) {
throw new InvalidApiUsageException("Location does not belong to dataset");
}
locationRepository.delete(datasetLocation);
/// Force update of Dataset#startDate, Dataset#endDate
dataset = datasetRepository.findByUuidAndVersion(dataset.getUuid(), dataset.getVersion());
dataset.getLocations().remove(datasetLocation);
dataset.setStartDate("--------");
dataset.setEndDate("--------");
updateDataset(dataset);
dataset.setLastModifiedDate(new Date());
datasetRepository.save(dataset);
return datasetLocation;
}
......@@ -922,7 +904,7 @@ public class DatasetServiceImpl implements DatasetService {
*/
@Override
public DatasetLocation loadLocation(final DatasetLocation input) throws NotFoundElement {
LOG.info("Load list DatasetLocation {}", input);
LOG.info("Load DatasetLocation {}", input);
final DatasetLocation datasetLocation = locationRepository.findByUuid(input.getUuid());
......
......@@ -94,14 +94,14 @@ public class UserRegistrationController {
emailVerificationService.sendVerificationEmail(newUser);
return newUser;
} catch (final PasswordPolicyException e) {
LOG.error(e.getMessage(), e);
LOG.error(e.getMessage());
throw new PasswordPolicyException(e.getMessage());
} catch (final DataIntegrityViolationException e) {
LOG.error(e.getMessage(), e);
LOG.error(e.getMessage());
throw new NotUniqueUserException("E-mail already taken: " + email);
} catch (final Exception e) {
LOG.error(e.getMessage(), e);
throw new Exception(e.getMessage());
LOG.error(e.getMessage());
throw e;
}
}
......
......@@ -108,7 +108,7 @@ public class EMailServiceImpl implements EMailService {
try {
mailSender.send(preparator);
} catch (final Exception e) {
LOG.error(e.getMessage(), e);
LOG.error(e.getMessage());
}
}
});
......
......@@ -33,6 +33,8 @@ import org.genesys.blocks.model.filters.StringFilter;
import org.genesys.catalog.exceptions.InvalidApiUsageException;
import org.genesys.catalog.exceptions.NotFoundElement;
import org.genesys.catalog.model.dataset.Dataset;
import org.genesys.catalog.model.dataset.DatasetCreator;
import org.genesys.catalog.model.dataset.DatasetCreator.DatasetCreatorRole;
import org.genesys.catalog.model.dataset.DatasetLocation;
import org.genesys.catalog.model.filters.AccessionRefFilter;
import org.genesys.catalog.model.filters.DatasetFilter;
......@@ -511,7 +513,6 @@ public class DatasetServiceTest extends AbstractDatasetServiceTest {
assertThat(dataset.getRepositoryFiles().get(0).getVersion(), is(1));
}
@Test
public void testRemoveFilesOnDatasetDelete() throws IOException, InvalidRepositoryFileDataException, NotFoundElement, InvalidRepositoryPathException {
......@@ -528,12 +529,11 @@ public class DatasetServiceTest extends AbstractDatasetServiceTest {
assertThat(dataset.getRepositoryFiles(), is(notNullValue()));
assertThat(dataset.getRepositoryFiles().size(), is(2));
assertThat(dataset.getRepositoryFiles().get(0).getVersion(), is(1));
datasetService.removeDataset(dataset);
assertThat(repositoryFilePersistence.count(), is(0l));
}
@Test
public void testUpdateFile() throws IOException, InvalidRepositoryFileDataException, NotFoundElement, InvalidRepositoryPathException, NoSuchRepositoryFileException {
final File file = new File(getClass().getResource("/mcpd20177.csv").getPath());
......@@ -668,16 +668,28 @@ public class DatasetServiceTest extends AbstractDatasetServiceTest {
}
@Test
public void testAddLocationToDataset() {
DatasetLocation savedLocation = createDatasetLocation("testCountry", "testMapCountry", "testStateProvince", "testVerbatimLocality", 10.0, 20.0, "20000101", "20010101");
Dataset savedDataset = buildAndSaveDataset(DATASET_TITLE_1, DATASET_DESCRIPTION_1, partner, PublishState.DRAFT);
public void testAddLocationsToDataset() {
DatasetLocation location1 = createDatasetLocation("testCountry", "testMapCountry", "testStateProvince", "testVerbatimLocality", 10.0, 20.0, "20000101", "20010101");
Dataset dataset = buildAndSaveDataset(DATASET_TITLE_1, DATASET_DESCRIPTION_1, partner, PublishState.DRAFT);
savedLocation = datasetService.createLocation(savedDataset, savedLocation);
location1 = datasetService.createLocation(dataset, location1);
Dataset loadedDataset = datasetService.loadDataset(savedDataset.getUuid());
assertThat(loadedDataset.getLocations(), hasSize(1));
assertEquals(savedLocation.getStartDate(), loadedDataset.getStartDate());
assertEquals(savedLocation.getEndDate(), loadedDataset.getEndDate());
// System.out.println("\n\n\nLoading");
dataset = datasetService.loadDataset(dataset.getUuid());
assertThat(dataset.getLocations(), hasSize(1));
assertEquals(location1.getStartDate(), dataset.getStartDate());
assertEquals(location1.getEndDate(), dataset.getEndDate());
DatasetLocation location2 = createDatasetLocation("testCountry2", "testMapCountry2", "testStateProvince2", "testVerbatimLocality2", -10.0, -20.0, "1999----", "199902--");
location2 = datasetService.createLocation(dataset, location2);
// System.out.println("\n\n\nLoading2");
dataset = datasetService.loadDataset(dataset.getUuid());
assertThat(dataset.getLocations(), hasSize(2));
assertEquals(location2.getStartDate(), dataset.getStartDate());
assertEquals(location1.getEndDate(), dataset.getEndDate());
}
@Test
......@@ -740,29 +752,54 @@ public class DatasetServiceTest extends AbstractDatasetServiceTest {
@Test
public void testRemoveLocationFromDataset() {
DatasetLocation savedLocation1 = createDatasetLocation("testCountry1", "testMapCountry1", "testStateProvince1", "testVerbatimLocality1", 10.0, 20.0, "20010101",
"20110101");
DatasetLocation savedLocation2 = createDatasetLocation("testCountry2", "testMapCountry2", "testStateProvince2", "testVerbatimLocality2", 11.0, 21.0, "20020202",
"20120202");
DatasetLocation location1 = createDatasetLocation("testCountry1", "testMapCountry1", "testStateProvince1", "testVerbatimLocality1", 10.0, 20.0, "20010101", "20010301");
DatasetLocation location2 = createDatasetLocation("testCountry2", "testMapCountry2", "testStateProvince2", "testVerbatimLocality2", 11.0, 21.0, "20010201", "20010401");
Dataset dataset = buildAndSaveDataset(DATASET_TITLE_1, DATASET_DESCRIPTION_1, partner, PublishState.DRAFT);
location1 = datasetService.createLocation(dataset, location1);
dataset = datasetService.loadDataset(dataset.getUuid());
assertEquals(location1.getStartDate(), dataset.getStartDate());
assertEquals(location1.getEndDate(), dataset.getEndDate());
location2 = datasetService.createLocation(dataset, location2);
dataset = datasetService.loadDataset(dataset.getUuid());
assertThat(dataset.getLocations(), hasSize(2));
assertEquals(location1.getStartDate(), dataset.getStartDate());
assertEquals(location2.getEndDate(), dataset.getEndDate());
location1 = datasetService.removeLocation(dataset, location1);
// System.out.println("\n\n\nLoading");
dataset = datasetService.loadDataset(dataset.getUuid());
assertThat(dataset.getLocations(), hasSize(1));
assertThat(dataset.getLocations(), contains(location2));
assertEquals(location2.getStartDate(), dataset.getStartDate());
assertEquals(location2.getEndDate(), dataset.getEndDate());
}
@Test
public void testRemoveCreatorFromDataset() {
DatasetCreator creator1 = createDatasetCreator("Creator 1", DatasetCreator.DatasetCreatorRole.MANAGER);
DatasetCreator creator2 = createDatasetCreator("Creator 2", DatasetCreator.DatasetCreatorRole.COLLECTOR);
Dataset savedDataset = buildAndSaveDataset(DATASET_TITLE_1, DATASET_DESCRIPTION_1, partner, PublishState.DRAFT);
savedLocation1 = datasetService.createLocation(savedDataset, savedLocation1);
creator1 = datasetService.createDatasetCreator(savedDataset, creator1);
savedDataset = datasetService.loadDataset(savedDataset.getUuid());
savedLocation2 = datasetService.createLocation(savedDataset, savedLocation2);
Dataset loadedDataset = datasetService.loadDataset(savedDataset.getUuid());
assertThat(loadedDataset.getLocations(), hasSize(2));
assertEquals(savedLocation1.getStartDate(), loadedDataset.getStartDate());
assertEquals(savedLocation2.getEndDate(), loadedDataset.getEndDate());
creator2 = datasetService.createDatasetCreator(savedDataset, creator2);
Dataset dataset = datasetService.loadDataset(savedDataset.getUuid());
savedLocation1 = datasetService.removeLocation(loadedDataset, savedLocation1);
assertThat(dataset.getCreators(), hasSize(2));
assertEquals(creator1.getFullName(), dataset.getCreators().get(0).getFullName());
assertEquals(creator2.getFullName(), dataset.getCreators().get(1).getFullName());
loadedDataset = datasetService.loadDataset(savedDataset.getUuid());
assertThat(loadedDataset.getLocations(), hasSize(1));
assertThat(loadedDataset.getLocations(), contains(savedLocation2));
creator1 = datasetService.removeDatasetCreator(dataset, creator1);
assertEquals(savedLocation2.getStartDate(), loadedDataset.getStartDate());
assertEquals(savedLocation2.getEndDate(), loadedDataset.getEndDate());
// System.out.println("\n\n\nLoading");
dataset = datasetService.loadDataset(savedDataset.getUuid());
assertThat(dataset.getCreators(), hasSize(1));
assertThat(dataset.getCreators(), contains(creator2));
}
@Test
......@@ -848,4 +885,11 @@ public class DatasetServiceTest extends AbstractDatasetServiceTest {
dataset1.getAccessionRefs().forEach(acceRef -> assertThat(acceRef.getAccession(), nullValue()));
}
private DatasetCreator createDatasetCreator(String fullName, DatasetCreatorRole role) {
DatasetCreator creator = new DatasetCreator();
creator.setFullName(fullName);
creator.setRole(role);
return creator;
}
}
......@@ -16,23 +16,14 @@
package org.genesys.test.server.api.v1;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.*;
import static org.springframework.restdocs.mockmvc.MockMvcRestDocumentation.documentationConfiguration;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
import java.util.HashSet;
import java.util.Locale;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.genesys.test.base.AbstractApiTest;
import org.genesys2.server.api.v1.CMSController;
import org.genesys2.server.model.impl.Article;
......@@ -50,11 +41,14 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.MediaType;
import org.springframework.restdocs.JUnitRestDocumentation;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.result.MockMvcResultHandlers;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.context.WebApplicationContext;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
/**
* @author Maxym Borodenko
*/
......@@ -134,7 +128,7 @@ public class CMSControllerTest extends AbstractApiTest {
/*@formatter:off*/
mockMvc.perform(get(CMSController.API_BASE.concat("/all-news"))
.contentType(MediaType.APPLICATION_JSON))
.andDo(MockMvcResultHandlers.print())
// .andDo(MockMvcResultHandlers.print())
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8))
.andExpect(jsonPath("$.totalElements", is(2)));
......
......@@ -113,7 +113,7 @@ public class UserRegistrationControllerTest extends AbstractApiTest {
.param("fullName", FULL_NAME)
.param("g-recaptcha-response", RECAPTCHA_RESPONSE))
// .andDo(MockMvcResultHandlers.print())
.andExpect(status().isInternalServerError())
.andExpect(status().is4xxClientError())
.andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8));
/*@formatter:on*/
}
......
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