From 4972b5ee3aa4628e73e88e2426426cb84454b87e Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Tue, 24 Sep 2024 11:32:44 +0200 Subject: [PATCH 01/29] refactor: WIP move resources and human_readable from request to negotiation --- .../resource/ResourceRepository.java | 30 ++++---- .../resource/ResourceServiceImpl.java | 6 +- .../negotiator/negotiation/Negotiation.java | 29 +++++--- .../negotiation/NegotiationRepository.java | 12 ++-- .../negotiation/NegotiationService.java | 9 --- .../negotiation/NegotiationServiceImpl.java | 54 ++------------ .../negotiation/NegotiationSpecification.java | 5 +- .../mappers/NegotiationModelMapper.java | 34 ++++----- .../request/QueryV2Controller.java | 31 +------- ...rces_and_human_readable_to_negotiation.sql | 7 ++ .../db/test/migration/R__Initial_data.sql | 25 ++++--- .../AttachmentRepositoriesTest.java | 22 ++---- .../repository/NegotiationRepositoryTest.java | 72 ++++++------------- .../NotificationRepositoryTest.java | 3 +- .../repository/ResourceRepositoryTest.java | 49 +++++-------- .../NegotiationLifecycleServiceImplTest.java | 2 +- .../service/UserNotificationServiceTest.java | 42 +++++++---- .../unit/mappers/NegotiationMapperTest.java | 27 +++---- .../unit/model/NegotiationTest.java | 13 +--- .../unit/service/NegotiationServiceTest.java | 34 ++++----- .../unit/service/PostServiceTest.java | 9 ++- 21 files changed, 196 insertions(+), 319 deletions(-) create mode 100644 src/main/resources/db/migration/V15.0__move_resources_and_human_readable_to_negotiation.sql diff --git a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java index 9e97608d9..e3c483ecd 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java @@ -16,22 +16,22 @@ public interface ResourceRepository extends JpaRepository, JpaSpecificationExecutor { @Query( - value = - """ -select rs.id as id, - r.negotiation_id as negotiationId, - rs.name as name, - rs.source_id as sourceId, - rspn.current_state as currentState, - o.name as organizationName, - o.external_id as organizationExternalId, - o.id as organizationId + value = """ +select + rs.id as id, + nrl.negotiation_id as negotiationId, + rs.name as name, + rs.source_id as sourceId, + rspn.current_state as currentState, + o.name as organizationName, + o.external_id as organizationExternalId, + o.id as organizationId from resource rs - join public.request_resources_link rrl on rs.id = rrl.resource_id - join public.organization o on o.id = rs.organization_id - join public.request r on r.id = rrl.request_id - left join public.resource_state_per_negotiation rspn on rs.source_id = rspn.resource_id and r.negotiation_id = rspn.negotiation_id - where r.negotiation_id = :negotiationId; + join public.negotiation_resources_link nrl on rs.id = nrl.resource_id + join public.organization o on o.id = rs.organization_id + left join public.resource_state_per_negotiation rspn on rs.source_id = rspn.resource_id and nrl.negotiation_id = rspn.negotiation_id +where + nrl.negotiation_id = :negotiationId; """, nativeQuery = true) List findByNegotiation(String negotiationId); diff --git a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java index 1460c0a32..e7d2e8ceb 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java @@ -131,11 +131,9 @@ private void updateResources(String negotiationId, UpdateResourcesDTO updateReso } private void persistChanges(Negotiation negotiation, Set resources) { - Request request = negotiation.getRequests().iterator().next(); - negotiationRepository.saveAndFlush(negotiation); resources.addAll(negotiation.getResources()); - request.setResources(resources); - requestRepository.saveAndFlush(request); + negotiation.setResources(resources); + negotiationRepository.saveAndFlush(negotiation); if (negotiation.getCurrentState().equals(NegotiationState.IN_PROGRESS)) { applicationEventPublisher.publishEvent(new NewResourcesAddedEvent(this, negotiation.getId())); } diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index 616c27f48..de88f14c6 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -20,6 +20,8 @@ import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; import jakarta.persistence.JoinColumn; +import jakarta.persistence.JoinTable; +import jakarta.persistence.ManyToMany; import jakarta.persistence.MapKeyColumn; import jakarta.persistence.NamedAttributeNode; import jakarta.persistence.NamedEntityGraph; @@ -32,6 +34,8 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; + +import jakarta.validation.constraints.NotNull; import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.Builder; @@ -81,6 +85,20 @@ public class Negotiation extends AuditEntity { @Exclude private Set requests; + @NotNull + @Column(columnDefinition = "TEXT") + private String humanReadable; + + @ManyToMany + @JoinTable( + name = "negotiation_resources_link", + joinColumns = @JoinColumn(name = "negotiation_id"), + inverseJoinColumns = @JoinColumn(name = "resource_id")) + @Exclude + @NotNull + private Set resources; + + @Formula(value = "JSONB_EXTRACT_PATH_TEXT(payload, 'project', 'title')") private String title; @@ -160,17 +178,6 @@ public int hashCode() { return Objects.hash(getId()); } - /** - * Returns all resources involved in the negotiation. - * - * @return an UnmodifiableSet of resources - */ - public Set getResources() { - return requests.stream() - .flatMap(request -> request.getResources().stream()) - .collect(Collectors.toUnmodifiableSet()); - } - private Resource lookupResource(Set resources, String resourceId) { return resources.stream() .filter(r -> r.getSourceId().equals(resourceId)) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java index 2f49c0afe..4e6d472fb 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java @@ -41,8 +41,7 @@ List findByModifiedDateBeforeAndCurrentState( "SELECT EXISTS (" + "SELECT distinct(n.id) " + "FROM negotiation n " - + " JOIN request rq ON n.id = rq.negotiation_id " - + " JOIN request_resources_link rrl ON rrl.request_id = rq.id " + + " JOIN negotiation_resources_link rrl ON rrl.negotiation_id = m.id " + " JOIN resource rs ON rrl.resource_id = rs.id " + " JOIN organization o ON rs.organization_id = o.id " + "WHERE n.id = :negotiationId and o.external_id = :organizationExternalId)", @@ -53,8 +52,7 @@ List findByModifiedDateBeforeAndCurrentState( value = "SELECT distinct (n) " + "FROM Negotiation n " - + "JOIN n.requests rq " - + "JOIN rq.resources rs " + + "JOIN n.resources rs " + "JOIN rs.networks net " + "WHERE net.id = :networkId") Page findAllForNetwork(Long networkId, Pageable pageable); @@ -63,8 +61,7 @@ List findByModifiedDateBeforeAndCurrentState( value = "select count (distinct n.id) " + "FROM Negotiation n " - + "JOIN n.requests rq " - + "JOIN rq.resources rs " + + "JOIN n.resources rs " + "JOIN rs.networks net " + "WHERE net.id = :networkId") Integer countAllForNetwork(Long networkId); @@ -73,8 +70,7 @@ List findByModifiedDateBeforeAndCurrentState( value = "select n.currentState, COUNT ( distinct n.id)" + "FROM Negotiation n " - + "JOIN n.requests rq " - + "JOIN rq.resources rs " + + "JOIN n.resources rs " + "JOIN rs.networks net " + "WHERE net.id = :networkId group by n.currentState") List countStatusDistribution(Long networkId); diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationService.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationService.java index 0deec57fa..cd1f458fc 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationService.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationService.java @@ -44,15 +44,6 @@ NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorId) NegotiationDTO update(String negotiationId, NegotiationCreateDTO negotiationBody) throws EntityNotFoundException; - /** - * Adds the request with :requestId to the negotiation with identified by :negotiationId - * - * @param negotiationId the id of the negotiation - * @param requestId the id of the request to add to the negotiation - * @return a NegotiationID with the data of the negotiation - */ - NegotiationDTO addRequestToNegotiation(String negotiationId, String requestId); - /** * Returns a list of all negotiation in the negotiator * diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java index cd5c59913..9df1edbb4 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java @@ -126,31 +126,19 @@ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorI // Gets the Entities for the requests log.debug("Getting request entities"); List requests = findRequests(negotiationBody.getRequests()); - - // Check if any negotiationBody is already associated to a negotiation - if (requests.stream().anyMatch(request -> request.getNegotiation() != null)) { - log.error("One or more negotiationBody object is already assigned to another negotiation"); - throw new WrongRequestException( - "One or more negotiationBody object is already assigned to another negotiation"); - } - - // Updates the bidirectional relationship between negotiation and requests - negotiationEntity.setRequests(new HashSet<>(requests)); - requests.forEach( - request -> { - request.setNegotiation(negotiationEntity); - }); + negotiationEntity.setResources(requests.stream().findFirst().get().getResources()); + negotiationEntity.setHumanReadable(requests.stream().findFirst().get().getHumanReadable()); Negotiation savedNegotiation; try { // Finally, save the negotiation. NB: it also cascades operations for other Requests savedNegotiation = negotiationRepository.save(negotiationEntity); - } catch (DataException | DataIntegrityViolationException ex) { log.error("Error while saving the Negotiation into db. Some db constraint violated"); log.error(ex); throw new EntityNotStorableException(); } + if (negotiationBody.getAttachments() != null) { List attachments = findAttachments(negotiationBody.getAttachments()); negotiationEntity.setAttachments(new HashSet<>(attachments)); @@ -164,24 +152,8 @@ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorI return modelMapper.map(savedNegotiation, NegotiationDTO.class); } - private NegotiationDTO update(Negotiation negotiationEntity, NegotiationCreateDTO request) { - List requests = findRequests(request.getRequests()); - - if (requests.stream() - .anyMatch( - query -> - query.getNegotiation() != null && query.getNegotiation() != negotiationEntity)) { - throw new WrongRequestException( - "One or more request object is already assigned to another negotiation"); - } - - requests.forEach( - query -> { - query.setNegotiation(negotiationEntity); - }); - - negotiationEntity.setRequests(new HashSet<>(requests)); - + private NegotiationDTO update( + Negotiation negotiationEntity, NegotiationCreateDTO negotiationCreateDTO) { try { Negotiation negotiation = negotiationRepository.save(negotiationEntity); return modelMapper.map(negotiation, NegotiationDTO.class); @@ -202,22 +174,6 @@ public NegotiationDTO update(String negotiationId, NegotiationCreateDTO negotiat return update(negotiationEntity, negotiationBody); } - public NegotiationDTO addRequestToNegotiation(String negotiationId, String requestId) { - Negotiation negotiationEntity = findEntityById(negotiationId, false); - Request requestEntity = - requestRepository - .findById(requestId) - .orElseThrow(() -> new EntityNotFoundException(requestId)); - negotiationEntity.getRequests().add(requestEntity); - requestEntity.setNegotiation(negotiationEntity); - try { - negotiationEntity = negotiationRepository.save(negotiationEntity); - return modelMapper.map(negotiationEntity, NegotiationDTO.class); - } catch (DataIntegrityViolationException ex) { - throw new EntityNotStorableException(); - } - } - /** * Returns all negotiation in the repository * diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java index 5ee8d1eeb..b961b26f6 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java @@ -105,8 +105,7 @@ public Predicate toPredicate( query.distinct(true); Predicate authorPredicate = criteriaBuilder.equal(root.get("createdBy"), person); if (!person.getResources().isEmpty()) { - Predicate involvedInResources = - root.joinSet("requests").joinSet("resources").in(person.getResources()); + Predicate involvedInResources = root.joinSet("resources").in(person.getResources()); return criteriaBuilder.or(authorPredicate, involvedInResources); } return authorPredicate; @@ -148,7 +147,7 @@ public Predicate toPredicate( @Nonnull CriteriaQuery query, @Nonnull CriteriaBuilder criteriaBuilder) { query.distinct(true); - return root.joinSet("requests").joinSet("resources").in(resources); + return root.joinSet("resources").in(resources); } }; } diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java index c8fea8129..59f50af29 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java @@ -41,8 +41,8 @@ public void addMappings() { Converter negotiationStatusConverter = status -> negotiationStatusConverter(status.getSource()); - Converter> resourcesConverter = - negotiation -> resourceConverter(negotiation.getSource()); + // Converter> resourcesConverter = + // negotiation -> resourceConverter(negotiation.getSource()); Converter payloadConverter = p -> { @@ -69,21 +69,21 @@ public void addMappings() { .map(Negotiation::getCurrentState, NegotiationDTO::setStatus)); } - private Set resourceConverter(Negotiation negotiation) { - Set requests = negotiation.getRequests(); - final Map statePerResource = - negotiation.getCurrentStatePerResource(); - - return requests.stream() - .flatMap( - request -> - request.getResources().stream() - .map( - resource -> - buildResourceWithStatus( - resource, statePerResource, negotiation.getId()))) - .collect(Collectors.toSet()); - } +// private Set resourceConverter(Negotiation negotiation) { +// Set requests = negotiation.getRequests(); +// final Map statePerResource = +// negotiation.getCurrentStatePerResource(); +// +// return requests.stream() +// .flatMap( +// request -> +// request.getResources().stream() +// .map( +// resource -> +// buildResourceWithStatus( +// resource, statePerResource, negotiation.getId()))) +// .collect(Collectors.toSet()); +// } private ResourceWithStatusDTO buildResourceWithStatus( Resource resource, diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/QueryV2Controller.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/QueryV2Controller.java index a436c664f..6b9063c25 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/QueryV2Controller.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/QueryV2Controller.java @@ -1,7 +1,6 @@ package eu.bbmri_eric.negotiator.negotiation.request; import eu.bbmri_eric.negotiator.negotiation.NegotiationService; -import eu.bbmri_eric.negotiator.negotiation.dto.NegotiationDTO; import eu.bbmri_eric.negotiator.negotiation.dto.QueryCreateV2DTO; import eu.bbmri_eric.negotiator.negotiation.dto.QueryV2DTO; import eu.bbmri_eric.negotiator.negotiation.dto.RequestCreateDTO; @@ -37,34 +36,8 @@ public class QueryV2Controller { ResponseEntity add(@Valid @RequestBody QueryCreateV2DTO queryRequest) { RequestCreateDTO v3Request = modelMapper.map(queryRequest, RequestCreateDTO.class); RequestDTO requestResponse; - boolean created; - if (queryRequest.getToken() != null && !queryRequest.getToken().isEmpty()) { - // Update an old request or add a new one to a negotiation - String[] tokens = queryRequest.getToken().split("__search__"); - // If the negotiation was not found in V2, a new request was created - if (negotiationService.exists(tokens[0])) { - created = false; - if (tokens.length == 1) { - requestResponse = requestService.create(v3Request); - NegotiationDTO negotiationDTO = - negotiationService.addRequestToNegotiation(tokens[0], requestResponse.getId()); - requestResponse.setNegotiationId(negotiationDTO.getId()); - } else { // Updating an old request: the requestToken can be ignored - requestResponse = requestService.update(tokens[1], v3Request); - } - } else { - requestResponse = requestService.create(v3Request); - created = true; - } - } else { - requestResponse = requestService.create(v3Request); - created = true; - } + requestResponse = requestService.create(v3Request); QueryV2DTO response = modelMapper.map(requestResponse, QueryV2DTO.class); - if (created) { - return ResponseEntity.created(URI.create(response.getRedirectUri())).body(response); - } else { - return ResponseEntity.accepted().header("Location", response.getRedirectUri()).body(response); - } + return ResponseEntity.created(URI.create(response.getRedirectUri())).body(response); } } diff --git a/src/main/resources/db/migration/V15.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V15.0__move_resources_and_human_readable_to_negotiation.sql new file mode 100644 index 000000000..214523cf3 --- /dev/null +++ b/src/main/resources/db/migration/V15.0__move_resources_and_human_readable_to_negotiation.sql @@ -0,0 +1,7 @@ +ALTER TABLE negotiation + ADD COLUMN human_readable TEXT NOT NULL; + +create TABLE negotiation_resources_link ( + negotiation_id character varying(255) NOT NULL, + resource_id bigint NOT NULL +); diff --git a/src/main/resources/db/test/migration/R__Initial_data.sql b/src/main/resources/db/test/migration/R__Initial_data.sql index 400427bc5..65afab133 100644 --- a/src/main/resources/db/test/migration/R__Initial_data.sql +++ b/src/main/resources/db/test/migration/R__Initial_data.sql @@ -58,23 +58,23 @@ values (4, 103), (8, 105), (9, 105); -insert into negotiation (id, creation_date, current_state, modified_date, created_by, modified_by, payload, private_posts_enabled, public_posts_enabled) -values ('negotiation-1', '2024-10-12', 'IN_PROGRESS', '2024-10-12', 108, 108, +insert into negotiation (id, creation_date, current_state, modified_date, created_by, modified_by, human_readable, payload, private_posts_enabled, public_posts_enabled) +values ('negotiation-1', '2024-10-12', 'IN_PROGRESS', '2024-10-12', 108, 108, '#1 Material Type: DNA', '{"project":{"title":"Biobanking project","description":"desc"},"samples":{"sample-type":"DNA","num-of-subjects": 10,"num-of-sample": "100","volume":3},"ethics-vote":{"ethics-vote":"My ethics"}}', true, true), - ('negotiation-2', '2024-03-12', 'SUBMITTED', '2024-04-02', 108, 108, + ('negotiation-2', '2024-03-12', 'SUBMITTED', '2024-04-02', 108, 108, '#1 Material Type: DNA; #2 Diagnosis: C18.2', '{"project":{"title":"Interesting project","description":"desc"},"samples":{"sample-type":"Plasma","num-of-subjects": 10,"num-of-sample": "100","volume":3},"ethics-vote":{"ethics-vote":"My ethics"}}', false, true), - ('negotiation-v2', '2023-04-12', 'ABANDONED', '2024-10-12', 108, 108, + ('negotiation-v2', '2023-04-12', 'ABANDONED', '2024-10-12', 108, 108, '#1 Diagnosis: C18.2', '{"project":{"title":"A Project 3","description":"Project 3 desc"},"samples":{"sample-type":"Blood","num-of-subjects": 5,"num-of-sample": "10","volume":4},"ethics-vote":{"ethics-vote":"My ethics"}}', false, false), - ('negotiation-3', '2024-02-24', 'IN_PROGRESS', '2024-02-24', 105, 105, + ('negotiation-3', '2024-02-24', 'IN_PROGRESS', '2024-02-24', 105, 105, '#1 Type: RD', '{"project":{"title":"Project 3","description":"Project 3 desc"},"samples":{"sample-type":"Blood","num-of-subjects": 5,"num-of-sample": "10","volume":4},"ethics-vote":{"ethics-vote":"My ethics"}}', true, true), - ('negotiation-4', '2024-01-10', 'ABANDONED', '2024-01-10', 105, 105, + ('negotiation-4', '2024-01-10', 'ABANDONED', '2024-01-10', 105, 105, '#1 Type: Cohort', '{"project":{"title":"Project 3","description":"Project 3 desc"},"samples":{"sample-type":"Blood","num-of-subjects": 5,"num-of-sample": "10","volume":4},"ethics-vote":{"ethics-vote":"My ethics"}}', false, false), - ('negotiation-5', '2024-03-11', 'SUBMITTED', '2024-04-12', 108, 108, + ('negotiation-5', '2024-03-11', 'SUBMITTED', '2024-04-12', 108, 108, '#1 Quality: ISO', '{"project":{"title":"Yet another important project","description":"desc"},"samples":{"sample-type":"Plasma","num-of-subjects": 10,"num-of-sample": "100","volume":3},"ethics-vote":{"ethics-vote":"My ethics"}}', false, true); @@ -102,7 +102,16 @@ values ('request-1', 4), ('request-4', 5), ('request-4', 7), ('request-5', 5), - ('request-5', 7), + ('request-5', 7); + +insert into negotiation_resources_link (negotiation_id, resource_id) +values ('negotiation-1', 4), + ('negotiation-v2', 7), + ('negotiation-3', 5), + ('negotiation-4', 5), + ('negotiation-4', 7), + ('negotiation-5', 5), + ('negotiation-5', 7), ('request-unassigned', 7); insert into resource_state_per_negotiation (negotiation_id, resource_id, current_state) diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java index c9bfcab84..914dcb184 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java @@ -68,10 +68,8 @@ void setUp() { this.organization2 = createOrganization(ORG_2); this.resource1 = createResource(this.organization1, RESOURCE_1); this.resource2 = createResource(this.organization2, RESOURCE_2); - this.request1 = createRequest(resource1, REQUEST_1); - this.request2 = createRequest(resource2, REQUEST_2); - this.negotiation1 = createNegotiation(NEGOTIATION_1_ID, request1); - this.negotiation2 = createNegotiation(NEGOTIATION_2_ID, request2); + this.negotiation1 = createNegotiation(NEGOTIATION_1_ID, resource1); + this.negotiation2 = createNegotiation(NEGOTIATION_2_ID, resource2); } private Organization createOrganization(String organizationID) { @@ -93,22 +91,12 @@ private Resource createResource(Organization organization, String resourceId) { .build()); } - private Request createRequest(Resource resource, String requestId) { - return requestRepository.save( - Request.builder() - .id(requestId) - .url("http://test") - .resources(Set.of(resource)) - .discoveryService(discoveryService) - .humanReadable("everything") - .build()); - } - - private Negotiation createNegotiation(String negotiationId, Request request) { + private Negotiation createNegotiation(String negotiationId, Resource resource) { return negotiationRepository.save( Negotiation.builder() .id(negotiationId) - .requests(Set.of(request)) + .resources(Set.of(resource)) + .humanReadable("#1 Material Type: DNA") .payload("{\"project\":{\"title\":\"negtitle\"} }") .build()); } diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java index 4755c2454..7f81bbd20 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java @@ -46,22 +46,22 @@ public class NegotiationRepositoryTest { String payload = """ - { - "project": { - "title": "Title", - "description": "Description" - }, - "samples": { - "sample-type": "DNA", - "num-of-subjects": 10, - "num-of-samples": 20, - "volume-per-sample": 5 - }, - "ethics-vote": { - "ethics-vote": "My ethic vote" - } - } - """; + { + "project": { + "title": "Title", + "description": "Description" + }, + "samples": { + "sample-type": "DNA", + "num-of-subjects": 10, + "num-of-samples": 20, + "volume-per-sample": 5 + }, + "ethics-vote": { + "ethics-vote": "My ethic vote" + } + } + """; @BeforeEach void setUp() { @@ -160,21 +160,12 @@ void save_10000differentResources_ok() { assertEquals(20, resource.getRepresentatives().size()); } - // Create and save the request and negotiation - Request request = - Request.builder() - .url("http://test") - .resources(new HashSet<>(resources)) - .discoveryService(discoveryService) - .humanReadable("everything") - .build(); - request = requestRepository.save(request); - Negotiation negotiation = Negotiation.builder() .currentState(NegotiationState.SUBMITTED) - .requests(Set.of(request)) + .resources(new HashSet<>(resources)) .publicPostsEnabled(false) + .humanReadable("#1 Material Type: DNA") .payload(payload) .build(); negotiation = negotiationRepository.save(negotiation); @@ -326,28 +317,18 @@ void findAllRelated_10authored30rep_ok() { } private void saveNegotiation() { - Set requests = new HashSet<>(); Set resources = new HashSet<>(); resources.add(resource); - Request request = - Request.builder() - .url("http://test") - .resources(resources) - .discoveryService(discoveryService) - .humanReadable("everything") - .build(); - request = requestRepository.save(request); - requests.add(request); Negotiation negotiation = Negotiation.builder() .currentState(NegotiationState.SUBMITTED) - .requests(requests) + .resources(resources) + .humanReadable("#1 Material Type: DNA") .publicPostsEnabled(false) .payload(payload) .build(); negotiation.setCreationDate(LocalDateTime.now()); negotiation.setCreatedBy(person); - request.setNegotiation(negotiation); negotiationRepository.save(negotiation); } @@ -355,24 +336,15 @@ private void saveNegotiation(Person author) { Set requests = new HashSet<>(); Set resources = new HashSet<>(); resources.add(resource); - Request request = - Request.builder() - .url("http://test") - .resources(resources) - .discoveryService(discoveryService) - .humanReadable("everything") - .build(); - request = requestRepository.save(request); - requests.add(request); Negotiation negotiation = Negotiation.builder() .currentState(NegotiationState.SUBMITTED) - .requests(requests) + .resources(resources) + .humanReadable("everything") .publicPostsEnabled(false) .payload(payload) .build(); negotiation.setCreatedBy(author); - request.setNegotiation(negotiation); negotiationRepository.save(negotiation); } diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java index 000d73456..ccb4259e1 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java @@ -100,7 +100,8 @@ private Negotiation saveNegotiation(Person author) { Negotiation negotiation = Negotiation.builder() .currentState(NegotiationState.SUBMITTED) - .requests(requests) + .resources(resources) + .humanReadable("#1 Material Type: DNA") .publicPostsEnabled(false) .payload(payload) .build(); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java index 492a766e8..c5b85e9ef 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java @@ -12,11 +12,9 @@ import eu.bbmri_eric.negotiator.governance.resource.ResourceViewDTO; import eu.bbmri_eric.negotiator.negotiation.Negotiation; import eu.bbmri_eric.negotiator.negotiation.NegotiationRepository; -import eu.bbmri_eric.negotiator.negotiation.request.Request; import eu.bbmri_eric.negotiator.negotiation.request.RequestRepository; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; -import eu.bbmri_eric.negotiator.user.PersonRepository; import eu.bbmri_eric.negotiator.util.RepositoryTest; import java.util.List; import java.util.Set; @@ -27,8 +25,6 @@ @RepositoryTest public class ResourceRepositoryTest { - @Autowired PersonRepository personRepository; - @Autowired RequestRepository requestRepository; @Autowired NegotiationRepository negotiationRepository; @@ -40,22 +36,22 @@ public class ResourceRepositoryTest { String payload = """ - { - "project": { - "title": "Title", - "description": "Description" - }, - "samples": { - "sample-type": "DNA", - "num-of-subjects": 10, - "num-of-samples": 20, - "volume-per-sample": 5 - }, - "ethics-vote": { - "ethics-vote": "My ethic vote" - } - } - """; + { + "project": { + "title": "Title", + "description": "Description" + }, + "samples": { + "sample-type": "DNA", + "num-of-subjects": 10, + "num-of-samples": 20, + "volume-per-sample": 5 + }, + "ethics-vote": { + "ethics-vote": "My ethic vote" + } + } + """; private DiscoveryService discoveryService; @BeforeEach @@ -118,23 +114,14 @@ void findAllByNegotiationId() { .name("test") .build()); - Request request = - Request.builder() - .url("http://test") - .resources(Set.of(res1, res2)) - .discoveryService(discoveryService) - .humanReadable("everything") - .build(); - request = requestRepository.save(request); - Negotiation negotiation = Negotiation.builder() .currentState(NegotiationState.SUBMITTED) - .requests(Set.of(request)) + .resources(Set.of(res1, res2)) + .humanReadable("#1 MaterialType: DNA") .publicPostsEnabled(false) .payload(payload) .build(); - request.setNegotiation(negotiation); negotiation.setStateForResource(res1.getSourceId(), NegotiationResourceState.SUBMITTED); negotiation.setStateForResource( res2.getSourceId(), NegotiationResourceState.REPRESENTATIVE_CONTACTED); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java index 770767fd1..16fb82579 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java @@ -139,7 +139,7 @@ private NegotiationDTO saveNegotiation() throws IOException { Request request = requestRepository.findById("request-2").get(); Negotiation negotiation = Negotiation.builder() - .requests(Set.of(request)) + .resources(request.getResources()) .payload(negotiationCreateDTO.getPayload().toString()) .build(); negotiation.setCreatedBy(Person.builder().id(101L).name("TheBuilder").build()); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java index 2370d08ae..23d576f6a 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java @@ -12,7 +12,6 @@ import eu.bbmri_eric.negotiator.negotiation.NegotiationService; import eu.bbmri_eric.negotiator.negotiation.dto.NegotiationCreateDTO; import eu.bbmri_eric.negotiator.negotiation.dto.NegotiationDTO; -import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationLifecycleService; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceEvent; import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; @@ -34,6 +33,7 @@ import java.time.LocalDateTime; import java.util.*; import org.junit.jupiter.api.Test; +import org.opentest4j.TestAbortedException; import org.springframework.beans.factory.annotation.Autowired; @IntegrationTest(loadTestData = true) @@ -45,7 +45,6 @@ public class UserNotificationServiceTest { @Autowired NotificationRepository notificationRepository; @Autowired NegotiationRepository negotiationRepository; @Autowired ResourceRepository resourceRepository; - @Autowired NegotiationLifecycleService negotiationLifecycleService; @Autowired ResourceLifecycleService resourceLifecycleService; @Autowired NotificationEmailRepository notificationEmailRepository; @Autowired PostService postService; @@ -119,18 +118,28 @@ void notifyRepresentativesAboutNewNegotiation_atLeastOne_ok() { @WithMockNegotiatorUser( id = 109L, authorities = {"ROLE_ADMIN"}) - void notifyRepresentatives_sameRepFor2Resources_oneNotification() throws InterruptedException { - Negotiation negotiation = negotiationRepository.findAll().get(0); + void notifyRepresentatives_sameRepFor2Resources_oneNotification() { + Negotiation negotiation = + negotiationRepository.findById("negotiation-1").orElseThrow(TestAbortedException::new); + Resource resource1 = - negotiation.getRequests().iterator().next().getResources().iterator().next(); - Person representative = resource1.getRepresentatives().iterator().next(); - Resource resource2 = resourceRepository.findBySourceId("biobank:1:collection:2").get(); + resourceRepository + .findBySourceId("biobank:1:collection:1") + .orElseThrow(TestAbortedException::new); + + Person representative = + resource1.getRepresentatives().stream().findFirst().orElseThrow(TestAbortedException::new); + + Resource resource2 = + resourceRepository + .findBySourceId("biobank:1:collection:2") + .orElseThrow(TestAbortedException::new); resource2.setRepresentatives(Set.of(representative)); - Set resources = negotiation.getRequests().iterator().next().getResources(); - resources.add(resource2); - negotiation.getRequests().iterator().next().setResources(resources); + + negotiation.setResources(Set.of(resource1, resource2)); negotiation.setStateForResource(resource2.getSourceId(), NegotiationResourceState.SUBMITTED); negotiation = negotiationRepository.save(negotiation); + assertEquals(2, negotiation.getResources().size()); assertTrue(resource2.getRepresentatives().contains(representative)); assertTrue(notificationRepository.findByRecipientId(representative.getId()).isEmpty()); @@ -213,9 +222,12 @@ void notifyUsersForNewPost_publicPost_authorIsNotNotified() { @Test @WithMockNegotiatorUser(id = 109L) void notifyUsersForNewPost_publicPost_repsAreNotified() { - Negotiation negotiation = negotiationRepository.findAll().get(0); + Negotiation negotiation = + negotiationRepository.findById("negotiation-1").orElseThrow(TestAbortedException::new); + Resource resource1 = - negotiation.getRequests().iterator().next().getResources().iterator().next(); + negotiation.getResources().stream().findFirst().orElseThrow(TestAbortedException::new); + Person representative = resource1.getRepresentatives().stream() .filter(person -> !person.getId().equals(109L)) @@ -230,9 +242,9 @@ void notifyUsersForNewPost_publicPost_repsAreNotified() { @Test @WithMockNegotiatorUser(id = 109L) void notifyUsersForNewPost_privatePost_onlyRepsOfOrgAreNotified() { - Negotiation negotiation = negotiationRepository.findAll().get(0); - Resource resource1 = - negotiation.getRequests().iterator().next().getResources().iterator().next(); + Negotiation negotiation = + negotiationRepository.findById("negotiation-1").orElseThrow(TestAbortedException::new); + Resource resource1 = negotiation.getResources().iterator().next(); Person representative = resource1.getRepresentatives().stream().findFirst().get(); assertTrue(Objects.nonNull(representative.getId())); assertTrue(notificationRepository.findByRecipientId(representative.getId()).isEmpty()); diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/mappers/NegotiationMapperTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/mappers/NegotiationMapperTest.java index e13d035a0..bc95863e4 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/mappers/NegotiationMapperTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/mappers/NegotiationMapperTest.java @@ -9,7 +9,6 @@ import eu.bbmri_eric.negotiator.negotiation.Negotiation; import eu.bbmri_eric.negotiator.negotiation.dto.NegotiationDTO; import eu.bbmri_eric.negotiator.negotiation.mappers.NegotiationModelMapper; -import eu.bbmri_eric.negotiator.negotiation.request.Request; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; import java.time.LocalDateTime; @@ -29,24 +28,18 @@ public class NegotiationMapperTest { public NegotiationModelMapper negotiationModelMapper = new NegotiationModelMapper(mapper); private static Negotiation buildNegotiation() { - Request request = - Request.builder() - .resources( - Set.of( - Resource.builder() - .sourceId("collection:1") - .organization( - Organization.builder() - .name("Test Biobank") - .externalId("biobank:1") - .build()) - .discoveryService(DiscoveryService.builder().build()) - .build())) - .build(); - + Set resources = + Set.of( + Resource.builder() + .sourceId("collection:1") + .organization( + Organization.builder().name("Test Biobank").externalId("biobank:1").build()) + .discoveryService(DiscoveryService.builder().build()) + .build()); Negotiation negotiation = Negotiation.builder() - .requests(Set.of(request)) + .resources(resources) + .humanReadable("#1 Material Type: DNA") .currentState(NegotiationState.SUBMITTED) .build(); negotiation.setCreationDate(LocalDateTime.of(2023, Month.SEPTEMBER, 19, 00, 00)); diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java index f0d44de23..f4ac89553 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java @@ -24,23 +24,12 @@ void createNegotiation_ConstructorAndBuilder_Ok() { assertInstanceOf(Negotiation.class, negotiationFromBuilder); } - @Test - void getNegotiationRequests_Ok() { - Negotiation negotiation = new Negotiation(); - Request request = new Request(); - negotiation.setRequests(new HashSet<>(List.of(request))); - assertEquals(1, negotiation.getRequests().size()); - assertEquals(request, negotiation.getRequests().iterator().next()); - } - @Test void getNegotiationResources_Ok() { Negotiation negotiation = new Negotiation(); - Request request = new Request(); Resource resource = new Resource(); resource.setSourceId("fancyId"); - request.setResources(new HashSet<>(List.of(resource))); - negotiation.setRequests(new HashSet<>(List.of(request))); + negotiation.setResources(new HashSet<>(List.of(resource))); assertEquals(Set.of(resource), negotiation.getResources()); } diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java index ed2ab4751..015e63aca 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java @@ -62,23 +62,19 @@ public class NegotiationServiceTest { private AutoCloseable closeable; private static Negotiation buildNegotiation() { - Request request = - Request.builder() - .resources( - Set.of( - Resource.builder() - .sourceId("collection:1") - .discoveryService(new DiscoveryService()) - .organization( - Organization.builder() - .externalId("biobank:1") - .name("TestBiobank") - .build()) - .build())) - .build(); + Set resources = + Set.of( + Resource.builder() + .sourceId("collection:1") + .discoveryService(new DiscoveryService()) + .organization( + Organization.builder().externalId("biobank:1").name("TestBiobank").build()) + .build()); + return Negotiation.builder() - .requests(Set.of(request)) + .resources(resources) .currentState(NegotiationState.SUBMITTED) + .humanReadable("#1 Material Type: DNA") .build(); } @@ -134,7 +130,7 @@ void testCreateNegotiation_ok() throws IOException { Negotiation negotiation = Negotiation.builder().build(); Request request = new Request(); request.setResources(Set.of(new Resource())); - negotiation.setRequests(Set.of(request)); + negotiation.setResources(request.getResources()); when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(request)); when(modelMapper.map(negotiationCreateDTO, Negotiation.class)).thenReturn(negotiation); NegotiationDTO savedDTO = new NegotiationDTO(); @@ -156,7 +152,7 @@ void testCreateNegotiation_ok_with_attachments() throws IOException { Negotiation negotiation = Negotiation.builder().build(); Request request = new Request(); request.setResources(Set.of(new Resource())); - negotiation.setRequests(Set.of(request)); + negotiation.setResources(request.getResources()); Attachment attachment = Attachment.builder().id("attachment-1").name("Attachment-1").build(); when(attachmentRepository.findAllById(List.of("attachment-1"))).thenReturn(List.of(attachment)); @@ -178,7 +174,7 @@ void testCreateNegotiation_fails_when_DataException() throws IOException { Negotiation negotiation = new Negotiation(); Request request = new Request(); request.setResources(Set.of(new Resource())); - negotiation.setRequests(Set.of(request)); + negotiation.setResources(request.getResources()); when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(new Request())); when(modelMapper.map(negotiationCreateDTO, Negotiation.class)).thenReturn(negotiation); when(negotiationRepository.save(any())).thenThrow(DataException.class); @@ -194,7 +190,7 @@ void testCreateNegotiation_fails_when_DataIntegrityViolationException() throws I Negotiation negotiation = new Negotiation(); Request request = new Request(); request.setResources(Set.of(new Resource())); - negotiation.setRequests(Set.of(request)); + negotiation.setResources(request.getResources()); when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(request)); when(modelMapper.map(negotiationCreateDTO, Negotiation.class)).thenReturn(negotiation); when(negotiationRepository.save(negotiation)).thenThrow(DataIntegrityViolationException.class); diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java index 3815a87b4..634ed6943 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java @@ -152,9 +152,12 @@ void before() { organization1.setResources(Set.of(resource1)); organization2.setResources(Set.of(resource2)); - Request request = Request.builder().resources(Set.of(resource1, resource2)).build(); - - negotiation = Negotiation.builder().id(NEG_1).requests(Set.of(request)).build(); + negotiation = + Negotiation.builder() + .id(NEG_1) + .resources(Set.of(resource1, resource2)) + .humanReadable("#1 Material Type: DNA") + .build(); negotiation.setCreatedBy(researcher); publicPost1 = From 8ae8a314ac7fbcdda65afa55a892f033f5abe2ba Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Tue, 24 Sep 2024 17:30:54 +0200 Subject: [PATCH 02/29] refactor: WIP almost all test passing --- .../resource/ResourceRepository.java | 6 ++-- .../negotiator/negotiation/Negotiation.java | 28 +++------------ .../negotiation/NegotiationRepository.java | 2 +- .../negotiation/NegotiationServiceImpl.java | 13 ++++--- .../negotiation/dto/NegotiationDTO.java | 5 ++- .../mappers/NegotiationModelMapper.java | 29 ++++++--------- .../UserNotificationServiceImpl.java | 3 +- .../negotiator/user/PersonRepository.java | 16 ++++----- .../db/test/migration/R__Initial_data.sql | 3 +- .../api/v2/QueryV2ControllerTests.java | 8 ++--- .../api/v3/NegotiationControllerTests.java | 18 +++++----- .../api/v3/RequestControllerTests.java | 23 ++++++------ .../AttachmentRepositoriesTest.java | 35 ++++++++----------- .../repository/NegotiationRepositoryTest.java | 2 -- .../NegotiationLifecycleServiceImplTest.java | 4 ++- .../service/UserNotificationServiceTest.java | 17 ++++----- .../unit/model/NegotiationTest.java | 1 - .../unit/service/NegotiationServiceTest.java | 5 +-- .../unit/service/PostServiceTest.java | 1 - 19 files changed, 99 insertions(+), 120 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java index e3c483ecd..744569a9d 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java @@ -16,7 +16,8 @@ public interface ResourceRepository extends JpaRepository, JpaSpecificationExecutor { @Query( - value = """ + value = + """ select rs.id as id, nrl.negotiation_id as negotiationId, @@ -31,7 +32,8 @@ public interface ResourceRepository join public.organization o on o.id = rs.organization_id left join public.resource_state_per_negotiation rspn on rs.source_id = rspn.resource_id and nrl.negotiation_id = rspn.negotiation_id where - nrl.negotiation_id = :negotiationId; + nrl.negotiation_id = :negotiationId +order by rs.source_id; """, nativeQuery = true) List findByNegotiation(String negotiationId); diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index de88f14c6..1631f34dc 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -4,7 +4,6 @@ import eu.bbmri_eric.negotiator.attachment.Attachment; import eu.bbmri_eric.negotiator.common.AuditEntity; import eu.bbmri_eric.negotiator.governance.resource.Resource; -import eu.bbmri_eric.negotiator.negotiation.request.Request; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationLifecycleRecord; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceLifecycleRecord; @@ -28,14 +27,12 @@ import jakarta.persistence.NamedSubgraph; import jakarta.persistence.OneToMany; import jakarta.persistence.Table; +import jakarta.validation.constraints.NotNull; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; - -import jakarta.validation.constraints.NotNull; import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.Builder; @@ -56,18 +53,6 @@ @Builder @Table(name = "negotiation") @Convert(converter = JsonType.class, attributeName = "json") -@NamedEntityGraph( - name = "negotiation-with-detailed-children", - attributeNodes = { - @NamedAttributeNode(value = "requests", subgraph = "requests-detailed"), - @NamedAttributeNode(value = "attachments"), - @NamedAttributeNode(value = "currentStatePerResource") - }, - subgraphs = { - @NamedSubgraph( - name = "requests-detailed", - attributeNodes = {@NamedAttributeNode(value = "resources")}) - }) public class Negotiation extends AuditEntity { @Id @@ -81,24 +66,19 @@ public class Negotiation extends AuditEntity { cascade = {CascadeType.MERGE}) private Set attachments; - @OneToMany(mappedBy = "negotiation", cascade = CascadeType.MERGE) - @Exclude - private Set requests; - @NotNull @Column(columnDefinition = "TEXT") private String humanReadable; @ManyToMany @JoinTable( - name = "negotiation_resources_link", - joinColumns = @JoinColumn(name = "negotiation_id"), - inverseJoinColumns = @JoinColumn(name = "resource_id")) + name = "negotiation_resources_link", + joinColumns = @JoinColumn(name = "negotiation_id"), + inverseJoinColumns = @JoinColumn(name = "resource_id")) @Exclude @NotNull private Set resources; - @Formula(value = "JSONB_EXTRACT_PATH_TEXT(payload, 'project', 'title')") private String title; diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java index 4e6d472fb..11f78f78c 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java @@ -41,7 +41,7 @@ List findByModifiedDateBeforeAndCurrentState( "SELECT EXISTS (" + "SELECT distinct(n.id) " + "FROM negotiation n " - + " JOIN negotiation_resources_link rrl ON rrl.negotiation_id = m.id " + + " JOIN negotiation_resources_link rrl ON rrl.negotiation_id = n.id " + " JOIN resource rs ON rrl.resource_id = rs.id " + " JOIN organization o ON rs.organization_id = o.id " + "WHERE n.id = :negotiationId and o.external_id = :organizationExternalId)", diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java index 9df1edbb4..76143e1fd 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java @@ -126,7 +126,8 @@ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorI // Gets the Entities for the requests log.debug("Getting request entities"); List requests = findRequests(negotiationBody.getRequests()); - negotiationEntity.setResources(requests.stream().findFirst().get().getResources()); + + negotiationEntity.setResources(new HashSet<>(requests.stream().findFirst().get().getResources())); negotiationEntity.setHumanReadable(requests.stream().findFirst().get().getHumanReadable()); Negotiation savedNegotiation; @@ -143,11 +144,15 @@ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorI List attachments = findAttachments(negotiationBody.getAttachments()); negotiationEntity.setAttachments(new HashSet<>(attachments)); attachments.forEach( - attachment -> { - attachment.setNegotiation(negotiationEntity); - }); + attachment -> attachment.setNegotiation(negotiationEntity)); } eventPublisher.publishEvent(new NewNegotiationEvent(this, negotiationEntity.getId())); + +// for (Request request : requests) { +// requestRepository.delete(request); +// } + + // TODO: Add call to send email. userNotificationService.notifyAdmins(negotiationEntity); return modelMapper.map(savedNegotiation, NegotiationDTO.class); } diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java index 42cd0fff4..3fec2fa72 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java @@ -3,7 +3,9 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.JsonNode; +import eu.bbmri_eric.negotiator.governance.resource.dto.ResourceDTO; import eu.bbmri_eric.negotiator.user.UserResponseModel; +import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.NotNull; import java.time.LocalDateTime; import java.util.Set; @@ -25,13 +27,14 @@ public class NegotiationDTO { @NotNull private String id; private UserResponseModel author; - private Set requests; @NotNull private JsonNode payload; @NotNull private String status; + @NotNull private String humanReadable; @NotNull private boolean publicPostsEnabled; @NotNull private boolean privatePostsEnabled; @NotNull private LocalDateTime creationDate; @NotNull private LocalDateTime modifiedDate; + @NotNull private Set resources; @JsonIgnore public String getStatusForResource(String resourceId) { diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java index 59f50af29..2fee2ecbe 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java @@ -8,7 +8,6 @@ import eu.bbmri_eric.negotiator.governance.resource.dto.ResourceWithStatusDTO; import eu.bbmri_eric.negotiator.negotiation.Negotiation; import eu.bbmri_eric.negotiator.negotiation.dto.NegotiationDTO; -import eu.bbmri_eric.negotiator.negotiation.request.Request; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; import jakarta.annotation.PostConstruct; @@ -16,6 +15,7 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; + import lombok.extern.apachecommons.CommonsLog; import org.modelmapper.Converter; import org.modelmapper.ModelMapper; @@ -41,8 +41,8 @@ public void addMappings() { Converter negotiationStatusConverter = status -> negotiationStatusConverter(status.getSource()); - // Converter> resourcesConverter = - // negotiation -> resourceConverter(negotiation.getSource()); + Converter> resourcesConverter = + negotiation -> resourceConverter(negotiation.getSource()); Converter payloadConverter = p -> { @@ -69,21 +69,14 @@ public void addMappings() { .map(Negotiation::getCurrentState, NegotiationDTO::setStatus)); } -// private Set resourceConverter(Negotiation negotiation) { -// Set requests = negotiation.getRequests(); -// final Map statePerResource = -// negotiation.getCurrentStatePerResource(); -// -// return requests.stream() -// .flatMap( -// request -> -// request.getResources().stream() -// .map( -// resource -> -// buildResourceWithStatus( -// resource, statePerResource, negotiation.getId()))) -// .collect(Collectors.toSet()); -// } + private Set resourceConverter(Negotiation negotiation) { + final Map statePerResource = + negotiation.getCurrentStatePerResource(); + + return negotiation.getResources().stream() + .map(resource -> buildResourceWithStatus(resource, statePerResource, negotiation.getId())) + .collect(Collectors.toSet()); + } private ResourceWithStatusDTO buildResourceWithStatus( Resource resource, diff --git a/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java index 52bb7aebc..eb13784ec 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java @@ -300,7 +300,8 @@ private List createNotificationsForRepresentatives( private List createNotificationsForAdmins( Negotiation negotiation, NotificationEmailStatus status, String notificationMessage) { List newNotifications = new ArrayList<>(); - for (Person admin : personRepository.findAllByAdminIsTrue()) { + List admins = personRepository.findAllByAdminIsTrue(); + for (Person admin : admins) { Notification newNotification = buildNewNotification(negotiation, status, admin, notificationMessage); newNotifications.add(newNotification); diff --git a/src/main/java/eu/bbmri_eric/negotiator/user/PersonRepository.java b/src/main/java/eu/bbmri_eric/negotiator/user/PersonRepository.java index c7099e3ea..b265994de 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/user/PersonRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/user/PersonRepository.java @@ -40,14 +40,14 @@ public interface PersonRepository @Query( value = - "SELECT EXISTS (SELECT rs.id " - + "FROM request rq JOIN request_resources_link rrl on rq.id = rrl.request_id " - + " JOIN resource rs on rs.id = rrl.resource_id " - + "WHERE rq.negotiation_id = :negotiationId AND " - + " rs.id in (" - + " select rrl.resource_id " - + " from person p join resource_representative_link rrl ON p.id = rrl.person_id " - + " where p.id = :personId" + "select exists ( " + + "select nrl.resource_id " + + "from negotiation_resources_link nrl " + + "where nrl.negotiation_id = :negotiationId and " + + " nrl.resource_id in ( " + + " select rrl.resource_id " + + " from person p join resource_representative_link rrl ON p.id = rrl.person_id " + + " where p.id = :personId" + "))", nativeQuery = true) boolean isRepresentativeOfAnyResourceOfNegotiation(Long personId, String negotiationId); diff --git a/src/main/resources/db/test/migration/R__Initial_data.sql b/src/main/resources/db/test/migration/R__Initial_data.sql index 65afab133..3325781b1 100644 --- a/src/main/resources/db/test/migration/R__Initial_data.sql +++ b/src/main/resources/db/test/migration/R__Initial_data.sql @@ -111,8 +111,7 @@ values ('negotiation-1', 4), ('negotiation-4', 5), ('negotiation-4', 7), ('negotiation-5', 5), - ('negotiation-5', 7), - ('request-unassigned', 7); + ('negotiation-5', 7); insert into resource_state_per_negotiation (negotiation_id, resource_id, current_state) values ('negotiation-1', 'biobank:1:collection:1', 'SUBMITTED'), diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java index 22cb64eba..b73525833 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java @@ -19,6 +19,7 @@ import java.util.Optional; import java.util.Set; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.modelmapper.ModelMapper; import org.springframework.beans.factory.annotation.Autowired; @@ -39,10 +40,6 @@ public class QueryV2ControllerTests { private static final String ENDPOINT = "/directory/create_query"; @Autowired public RequestRepository requestRepository; @Autowired private WebApplicationContext context; - @Autowired private QueryV2Controller controller; - @Autowired private RequestServiceImpl requestService; - @Autowired private NegotiationRepository negotiationRepository; - @Autowired private ModelMapper modelMapper; private MockMvc mockMvc; @@ -149,6 +146,7 @@ public void testCreate_Ok() throws Exception { assertEquals(requestRepository.count(), previousCount + 1); } + @Deprecated @Test public void testUpdate_CreateWhenRequestIsNotFound() throws Exception { QueryCreateV2DTO updateRequest = TestUtils.createQueryV2Request(); @@ -166,6 +164,7 @@ public void testUpdate_CreateWhenRequestIsNotFound() throws Exception { .andExpect(jsonPath("$.redirect_uri", containsString("http://localhost/request"))); } + @Disabled @Test @Transactional public void testUpdate_Ok_whenChangeQuery() throws Exception { @@ -195,6 +194,7 @@ public void testUpdate_Ok_whenChangeQuery() throws Exception { assertEquals(requestRepository.count(), previousCount); } + @Disabled @Test public void testUpdate_Ok_whenAddQueryToARequest() throws Exception { QueryCreateV2DTO updateRequest = TestUtils.createQueryV2Request(); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java index f26a25fd9..1d37fa945 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java @@ -25,7 +25,6 @@ import eu.bbmri_eric.negotiator.negotiation.NegotiationRepository; import eu.bbmri_eric.negotiator.negotiation.dto.NegotiationCreateDTO; import eu.bbmri_eric.negotiator.negotiation.dto.UpdateResourcesDTO; -import eu.bbmri_eric.negotiator.negotiation.request.Request; import eu.bbmri_eric.negotiator.negotiation.request.RequestRepository; import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; import eu.bbmri_eric.negotiator.user.PersonRepository; @@ -39,7 +38,9 @@ import java.util.Optional; import java.util.Set; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.opentest4j.TestAbortedException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; @@ -959,6 +960,7 @@ public void testCreate_BadRequest_whenSomeRequests_IsNotFound() throws Exception .andExpect(status().isBadRequest()); } + @Disabled @Test @WithUserDetails("researcher") public void testCreate_BadRequest_whenRequest_IsAlreadyAssignedToAnotherRequest() @@ -1031,6 +1033,7 @@ public void testUpdate_Unauthorized_whenWrongAuth() throws Exception { "%s/1".formatted(NEGOTIATIONS_URL)); } + @Disabled @Test @WithUserDetails("TheResearcher") public void testUpdate_BadRequest_whenRequestIsAlreadyAssignedToAnotherNegotiation() @@ -1120,17 +1123,16 @@ void getNegotiation_2000resources_ok() throws Exception { .build()); resources.add(resource); } - Request request = requestRepository.findAll().get(0); - request.setResources(resources); + Negotiation negotiation = + negotiationRepository.findById("negotiation-1").orElseThrow(TestAbortedException::new); + negotiation.setResources(resources); for (Resource resource : resources) { - request - .getNegotiation() - .setStateForResource(resource.getSourceId(), NegotiationResourceState.SUBMITTED); + negotiation.setStateForResource(resource.getSourceId(), NegotiationResourceState.SUBMITTED); } - requestRepository.save(request); + negotiationRepository.save(negotiation); mockMvc .perform( - MockMvcRequestBuilders.get(NEGOTIATIONS_URL + "/" + request.getNegotiation().getId())) + MockMvcRequestBuilders.get(NEGOTIATIONS_URL + "/" + negotiation.getId())) .andExpect(status().isOk()); } diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java index f447868c4..9cd1c54c7 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java @@ -15,6 +15,7 @@ import com.github.tomakehurst.wiremock.junit5.WireMockTest; import eu.bbmri_eric.negotiator.governance.resource.dto.ResourceDTO; import eu.bbmri_eric.negotiator.negotiation.NegotiationService; +import eu.bbmri_eric.negotiator.negotiation.dto.NegotiationCreateDTO; import eu.bbmri_eric.negotiator.negotiation.dto.NegotiationDTO; import eu.bbmri_eric.negotiator.negotiation.dto.RequestCreateDTO; import eu.bbmri_eric.negotiator.negotiation.dto.RequestDTO; @@ -23,10 +24,10 @@ import eu.bbmri_eric.negotiator.negotiation.request.RequestServiceImpl; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationLifecycleService; import eu.bbmri_eric.negotiator.util.IntegrationTest; -import java.util.Collections; import java.util.Optional; import java.util.Set; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.modelmapper.ModelMapper; import org.springframework.beans.factory.annotation.Autowired; @@ -48,14 +49,11 @@ public class RequestControllerTests { private static final String NEGOTIATION_1 = "negotiation-1"; private static final String UNASSIGNED_REQUEST_ID = "request-unassigned"; - @Autowired public RequestServiceImpl service; - @Autowired public RequestRepository repository; - @Autowired private WebApplicationContext context; - @Autowired private RequestController controller; - @Autowired private RequestServiceImpl requestService; - @Autowired private NegotiationService negotiationService; - @Autowired private NegotiationLifecycleService negotiationLifecycleService; - @Autowired private ModelMapper modelMapper; + @Autowired RequestServiceImpl service; + @Autowired RequestRepository repository; + @Autowired WebApplicationContext context; + @Autowired RequestServiceImpl requestService; + @Autowired NegotiationService negotiationService; private MockMvc mockMvc; @@ -246,13 +244,14 @@ public void testGetById_Ok_whenNoNegotiationIsAssigned() throws Exception { assertEquals(repository.count(), previousCount); } + // This is no longer possible since when the negotiation is assigned the request is removed + @Disabled @Test @WithUserDetails("researcher") public void testGetById_Ok_whenNegotiationIsAssigned() throws Exception { RequestDTO r = requestService.create(TestUtils.createRequest(false)); - NegotiationDTO n = - negotiationService.create( - TestUtils.createNegotiation(Collections.singleton(r.getId())), CREATOR_ID); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of(r.getId())); + NegotiationDTO n = negotiationService.create(negotiationCreateDTO, CREATOR_ID); long previousCount = repository.count(); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java index 914dcb184..c8acb391c 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java @@ -16,8 +16,7 @@ import eu.bbmri_eric.negotiator.governance.resource.ResourceRepository; import eu.bbmri_eric.negotiator.negotiation.Negotiation; import eu.bbmri_eric.negotiator.negotiation.NegotiationRepository; -import eu.bbmri_eric.negotiator.negotiation.request.Request; -import eu.bbmri_eric.negotiator.negotiation.request.RequestRepository; +import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.user.Person; import eu.bbmri_eric.negotiator.user.PersonRepository; import eu.bbmri_eric.negotiator.util.RepositoryTest; @@ -36,24 +35,17 @@ public class AttachmentRepositoriesTest { private static final String NEGOTIATION_2_ID = "negotiation_2"; private static final String RESOURCE_1 = "resource_1"; private static final String RESOURCE_2 = "resource_2"; - private static final String REQUEST_1 = "request_1"; - private static final String REQUEST_2 = "request_2"; - @Autowired private PersonRepository personRepository; - @Autowired private ResourceRepository resourceRepository; - @Autowired private RequestRepository requestRepository; - @Autowired private DiscoveryServiceRepository discoveryServiceRepository; - @Autowired private OrganizationRepository organizationRepository; - @Autowired private NegotiationRepository negotiationRepository; - @Autowired private AttachmentRepository attachmentRepository; + @Autowired PersonRepository personRepository; + @Autowired ResourceRepository resourceRepository; + @Autowired DiscoveryServiceRepository discoveryServiceRepository; + @Autowired OrganizationRepository organizationRepository; + @Autowired NegotiationRepository negotiationRepository; + @Autowired AttachmentRepository attachmentRepository; private DiscoveryService discoveryService; private Person person; - private Resource resource1; - private Resource resource2; - private Request request1; - private Request request2; private Negotiation negotiation1; private Negotiation negotiation2; private Organization organization1; @@ -66,8 +58,8 @@ void setUp() { this.person = createPerson("person1"); this.organization1 = createOrganization(ORG_1); this.organization2 = createOrganization(ORG_2); - this.resource1 = createResource(this.organization1, RESOURCE_1); - this.resource2 = createResource(this.organization2, RESOURCE_2); + Resource resource1 = createResource(this.organization1, RESOURCE_1); + Resource resource2 = createResource(this.organization2, RESOURCE_2); this.negotiation1 = createNegotiation(NEGOTIATION_1_ID, resource1); this.negotiation2 = createNegotiation(NEGOTIATION_2_ID, resource2); } @@ -92,13 +84,16 @@ private Resource createResource(Organization organization, String resourceId) { } private Negotiation createNegotiation(String negotiationId, Resource resource) { - return negotiationRepository.save( + Negotiation negotiation = Negotiation.builder() .id(negotiationId) - .resources(Set.of(resource)) + .currentState(NegotiationState.SUBMITTED) + .publicPostsEnabled(false) .humanReadable("#1 Material Type: DNA") .payload("{\"project\":{\"title\":\"negtitle\"} }") - .build()); + .resources(Set.of(resource)) + .build(); + return negotiationRepository.save(negotiation); } private Person createPerson(String subjectId) { diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java index 7f81bbd20..cd50edaee 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java @@ -13,7 +13,6 @@ import eu.bbmri_eric.negotiator.negotiation.NegotiationRepository; import eu.bbmri_eric.negotiator.negotiation.NegotiationSpecification; import eu.bbmri_eric.negotiator.negotiation.request.Request; -import eu.bbmri_eric.negotiator.negotiation.request.RequestRepository; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.user.Person; import eu.bbmri_eric.negotiator.user.PersonRepository; @@ -35,7 +34,6 @@ public class NegotiationRepositoryTest { @Autowired PersonRepository personRepository; @Autowired ResourceRepository resourceRepository; - @Autowired RequestRepository requestRepository; @Autowired DiscoveryServiceRepository discoveryServiceRepository; @Autowired OrganizationRepository organizationRepository; @Autowired NegotiationRepository negotiationRepository; diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java index 16fb82579..7256a7874 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java @@ -38,6 +38,7 @@ import eu.bbmri_eric.negotiator.util.WithMockNegotiatorUser; import jakarta.transaction.Transactional; import java.io.IOException; +import java.util.HashSet; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -139,7 +140,8 @@ private NegotiationDTO saveNegotiation() throws IOException { Request request = requestRepository.findById("request-2").get(); Negotiation negotiation = Negotiation.builder() - .resources(request.getResources()) + .resources(new HashSet<>(request.getResources())) + .humanReadable("#1 MaterialType: DNA") .payload(negotiationCreateDTO.getPayload().toString()) .build(); negotiation.setCreatedBy(Person.builder().id(101L).name("TheBuilder").build()); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java index 23d576f6a..4f143b690 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java @@ -119,24 +119,25 @@ void notifyRepresentativesAboutNewNegotiation_atLeastOne_ok() { id = 109L, authorities = {"ROLE_ADMIN"}) void notifyRepresentatives_sameRepFor2Resources_oneNotification() { - Negotiation negotiation = - negotiationRepository.findById("negotiation-1").orElseThrow(TestAbortedException::new); - Resource resource1 = resourceRepository - .findBySourceId("biobank:1:collection:1") + .findBySourceId("biobank:1:collection:2") .orElseThrow(TestAbortedException::new); - Person representative = resource1.getRepresentatives().stream().findFirst().orElseThrow(TestAbortedException::new); - Resource resource2 = resourceRepository - .findBySourceId("biobank:1:collection:2") + .findBySourceId("biobank:2:collection:1") .orElseThrow(TestAbortedException::new); resource2.setRepresentatives(Set.of(representative)); - negotiation.setResources(Set.of(resource1, resource2)); + Negotiation negotiation = + Negotiation.builder() + .humanReadable("query") + .resources(Set.of(resource1, resource2)) + .payload( + "{\"project\":{\"title\":\"A Project 3\",\"description\":\"Project 3 desc\"}}") + .build(); negotiation.setStateForResource(resource2.getSourceId(), NegotiationResourceState.SUBMITTED); negotiation = negotiationRepository.save(negotiation); diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java index f4ac89553..2738115c1 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java @@ -5,7 +5,6 @@ import eu.bbmri_eric.negotiator.governance.resource.Resource; import eu.bbmri_eric.negotiator.negotiation.Negotiation; -import eu.bbmri_eric.negotiator.negotiation.request.Request; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; import java.util.HashSet; diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java index 015e63aca..6ec33f3bc 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java @@ -127,6 +127,7 @@ public void test_isNegotiatorCreator_IsTrue_WhenPersonRepositoryIsNegotiatiorCre @Test void testCreateNegotiation_ok() throws IOException { NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("requestID")); + Negotiation negotiation = Negotiation.builder().build(); Request request = new Request(); request.setResources(Set.of(new Resource())); @@ -156,7 +157,7 @@ void testCreateNegotiation_ok_with_attachments() throws IOException { Attachment attachment = Attachment.builder().id("attachment-1").name("Attachment-1").build(); when(attachmentRepository.findAllById(List.of("attachment-1"))).thenReturn(List.of(attachment)); - when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(new Request())); + when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(request)); when(modelMapper.map(negotiationCreateDTO, Negotiation.class)).thenReturn(negotiation); NegotiationDTO savedDTO = new NegotiationDTO(); @@ -175,7 +176,7 @@ void testCreateNegotiation_fails_when_DataException() throws IOException { Request request = new Request(); request.setResources(Set.of(new Resource())); negotiation.setResources(request.getResources()); - when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(new Request())); + when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(request)); when(modelMapper.map(negotiationCreateDTO, Negotiation.class)).thenReturn(negotiation); when(negotiationRepository.save(any())).thenThrow(DataException.class); assertThrows( diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java index 634ed6943..af55fc51b 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java @@ -16,7 +16,6 @@ import eu.bbmri_eric.negotiator.negotiation.Negotiation; import eu.bbmri_eric.negotiator.negotiation.NegotiationRepository; import eu.bbmri_eric.negotiator.negotiation.NegotiationService; -import eu.bbmri_eric.negotiator.negotiation.request.Request; import eu.bbmri_eric.negotiator.notification.UserNotificationServiceImpl; import eu.bbmri_eric.negotiator.post.Post; import eu.bbmri_eric.negotiator.post.PostCreateDTO; From 6d984d19a92434fc38e05e4239d3e3abc5a82ba7 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Tue, 24 Sep 2024 17:32:06 +0200 Subject: [PATCH 03/29] refactor: WIP code formatting --- .../negotiator/negotiation/Negotiation.java | 3 --- .../negotiation/NegotiationServiceImpl.java | 12 ++++++------ .../negotiator/negotiation/dto/NegotiationDTO.java | 1 - .../negotiation/mappers/NegotiationModelMapper.java | 1 - .../integration/api/v2/QueryV2ControllerTests.java | 4 ---- .../api/v3/NegotiationControllerTests.java | 3 +-- .../integration/api/v3/RequestControllerTests.java | 3 --- .../service/UserNotificationServiceTest.java | 3 +-- 8 files changed, 8 insertions(+), 22 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index 1631f34dc..e733ae622 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -22,9 +22,6 @@ import jakarta.persistence.JoinTable; import jakarta.persistence.ManyToMany; import jakarta.persistence.MapKeyColumn; -import jakarta.persistence.NamedAttributeNode; -import jakarta.persistence.NamedEntityGraph; -import jakarta.persistence.NamedSubgraph; import jakarta.persistence.OneToMany; import jakarta.persistence.Table; import jakarta.validation.constraints.NotNull; diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java index 76143e1fd..2ad51421f 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java @@ -127,7 +127,8 @@ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorI log.debug("Getting request entities"); List requests = findRequests(negotiationBody.getRequests()); - negotiationEntity.setResources(new HashSet<>(requests.stream().findFirst().get().getResources())); + negotiationEntity.setResources( + new HashSet<>(requests.stream().findFirst().get().getResources())); negotiationEntity.setHumanReadable(requests.stream().findFirst().get().getHumanReadable()); Negotiation savedNegotiation; @@ -143,14 +144,13 @@ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorI if (negotiationBody.getAttachments() != null) { List attachments = findAttachments(negotiationBody.getAttachments()); negotiationEntity.setAttachments(new HashSet<>(attachments)); - attachments.forEach( - attachment -> attachment.setNegotiation(negotiationEntity)); + attachments.forEach(attachment -> attachment.setNegotiation(negotiationEntity)); } eventPublisher.publishEvent(new NewNegotiationEvent(this, negotiationEntity.getId())); -// for (Request request : requests) { -// requestRepository.delete(request); -// } + // for (Request request : requests) { + // requestRepository.delete(request); + // } // TODO: Add call to send email. userNotificationService.notifyAdmins(negotiationEntity); diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java index 3fec2fa72..e7db688f7 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java @@ -5,7 +5,6 @@ import com.fasterxml.jackson.databind.JsonNode; import eu.bbmri_eric.negotiator.governance.resource.dto.ResourceDTO; import eu.bbmri_eric.negotiator.user.UserResponseModel; -import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.NotNull; import java.time.LocalDateTime; import java.util.Set; diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java index 2fee2ecbe..7d55074a5 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java @@ -15,7 +15,6 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; - import lombok.extern.apachecommons.CommonsLog; import org.modelmapper.Converter; import org.modelmapper.ModelMapper; diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java index b73525833..3450d129b 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java @@ -9,19 +9,15 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import eu.bbmri_eric.negotiator.integration.api.v3.TestUtils; -import eu.bbmri_eric.negotiator.negotiation.NegotiationRepository; import eu.bbmri_eric.negotiator.negotiation.dto.CollectionV2DTO; import eu.bbmri_eric.negotiator.negotiation.dto.QueryCreateV2DTO; -import eu.bbmri_eric.negotiator.negotiation.request.QueryV2Controller; import eu.bbmri_eric.negotiator.negotiation.request.RequestRepository; -import eu.bbmri_eric.negotiator.negotiation.request.RequestServiceImpl; import eu.bbmri_eric.negotiator.util.IntegrationTest; import java.util.Optional; import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.modelmapper.ModelMapper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java index 1d37fa945..b798d1155 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java @@ -1131,8 +1131,7 @@ void getNegotiation_2000resources_ok() throws Exception { } negotiationRepository.save(negotiation); mockMvc - .perform( - MockMvcRequestBuilders.get(NEGOTIATIONS_URL + "/" + negotiation.getId())) + .perform(MockMvcRequestBuilders.get(NEGOTIATIONS_URL + "/" + negotiation.getId())) .andExpect(status().isOk()); } diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java index 9cd1c54c7..314583f6e 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java @@ -19,17 +19,14 @@ import eu.bbmri_eric.negotiator.negotiation.dto.NegotiationDTO; import eu.bbmri_eric.negotiator.negotiation.dto.RequestCreateDTO; import eu.bbmri_eric.negotiator.negotiation.dto.RequestDTO; -import eu.bbmri_eric.negotiator.negotiation.request.RequestController; import eu.bbmri_eric.negotiator.negotiation.request.RequestRepository; import eu.bbmri_eric.negotiator.negotiation.request.RequestServiceImpl; -import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationLifecycleService; import eu.bbmri_eric.negotiator.util.IntegrationTest; import java.util.Optional; import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.modelmapper.ModelMapper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java index 4f143b690..a31be31c6 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java @@ -135,8 +135,7 @@ void notifyRepresentatives_sameRepFor2Resources_oneNotification() { Negotiation.builder() .humanReadable("query") .resources(Set.of(resource1, resource2)) - .payload( - "{\"project\":{\"title\":\"A Project 3\",\"description\":\"Project 3 desc\"}}") + .payload("{\"project\":{\"title\":\"A Project 3\",\"description\":\"Project 3 desc\"}}") .build(); negotiation.setStateForResource(resource2.getSourceId(), NegotiationResourceState.SUBMITTED); negotiation = negotiationRepository.save(negotiation); From e2942ba21b2593ac3f31d47b948016f06dfcd740 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 09:21:51 +0200 Subject: [PATCH 04/29] refactor: all tests passing --- .../bbmri_eric/negotiator/negotiation/Negotiation.java | 4 ++-- ...ove_resources_and_human_readable_to_negotiation.sql | 10 +++++----- .../integration/api/v3/NegotiationControllerTests.java | 2 ++ .../repository/AttachmentRepositoriesTest.java | 5 +---- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index e733ae622..bb8f9c3c2 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -65,7 +65,7 @@ public class Negotiation extends AuditEntity { @NotNull @Column(columnDefinition = "TEXT") - private String humanReadable; + private String humanReadable = ""; @ManyToMany @JoinTable( @@ -74,7 +74,7 @@ public class Negotiation extends AuditEntity { inverseJoinColumns = @JoinColumn(name = "resource_id")) @Exclude @NotNull - private Set resources; + private Set resources = new HashSet<>(); @Formula(value = "JSONB_EXTRACT_PATH_TEXT(payload, 'project', 'title')") private String title; diff --git a/src/main/resources/db/migration/V15.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V15.0__move_resources_and_human_readable_to_negotiation.sql index 214523cf3..179c520d0 100644 --- a/src/main/resources/db/migration/V15.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V15.0__move_resources_and_human_readable_to_negotiation.sql @@ -1,7 +1,7 @@ -ALTER TABLE negotiation - ADD COLUMN human_readable TEXT NOT NULL; +ALTER TABLE negotiation ADD COLUMN human_readable text NOT NULL; -create TABLE negotiation_resources_link ( - negotiation_id character varying(255) NOT NULL, - resource_id bigint NOT NULL +CREATE TABLE negotiation_resources_link ( + negotiation_id character varying(255) NOT NULL REFERENCES negotiation(id), + resource_id bigint NOT NULL REFERENCES resource(id), + PRIMARY KEY (negotiation_id, resource_id) ); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java index b798d1155..5b659e6e4 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java @@ -2,6 +2,7 @@ import static org.hamcrest.core.Is.is; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.anonymous; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic; @@ -1007,6 +1008,7 @@ public void testCreate_Ok() throws Exception { Optional negotiation = negotiationRepository.findById(negotiationId); assert negotiation.isPresent(); assertEquals(negotiation.get().getCreatedBy().getName(), "researcher"); + assertFalse(requestRepository.existsById(REQUEST_UNASSIGNED)); } @Test diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java index c8acb391c..bcef008a4 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java @@ -16,7 +16,6 @@ import eu.bbmri_eric.negotiator.governance.resource.ResourceRepository; import eu.bbmri_eric.negotiator.negotiation.Negotiation; import eu.bbmri_eric.negotiator.negotiation.NegotiationRepository; -import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.user.Person; import eu.bbmri_eric.negotiator.user.PersonRepository; import eu.bbmri_eric.negotiator.util.RepositoryTest; @@ -87,10 +86,8 @@ private Negotiation createNegotiation(String negotiationId, Resource resource) { Negotiation negotiation = Negotiation.builder() .id(negotiationId) - .currentState(NegotiationState.SUBMITTED) - .publicPostsEnabled(false) .humanReadable("#1 Material Type: DNA") - .payload("{\"project\":{\"title\":\"negtitle\"} }") + .payload("{\"project\":{\"title\":\"title\"} }") .resources(Set.of(resource)) .build(); return negotiationRepository.save(negotiation); From 5f75340288195a05bb6342459ed0389f95ceb460 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 09:51:15 +0200 Subject: [PATCH 05/29] refactor: adds removal of requests after a negotiation is created --- .../negotiator/negotiation/NegotiationServiceImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java index 2ad51421f..8e539247d 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java @@ -148,9 +148,9 @@ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorI } eventPublisher.publishEvent(new NewNegotiationEvent(this, negotiationEntity.getId())); - // for (Request request : requests) { - // requestRepository.delete(request); - // } + for (Request request : requests) { + requestRepository.delete(request); + } // TODO: Add call to send email. userNotificationService.notifyAdmins(negotiationEntity); From 0d007eda5fea4a2c6ac58e4ae88ce23b7cf2fde4 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 10:05:56 +0200 Subject: [PATCH 06/29] refactor: rename flyway migration script --- ...> V16.0__move_resources_and_human_readable_to_negotiation.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/main/resources/db/migration/{V15.0__move_resources_and_human_readable_to_negotiation.sql => V16.0__move_resources_and_human_readable_to_negotiation.sql} (100%) diff --git a/src/main/resources/db/migration/V15.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql similarity index 100% rename from src/main/resources/db/migration/V15.0__move_resources_and_human_readable_to_negotiation.sql rename to src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql From 3ec5e83efcab0fb17bba4aaf733545ddf5ab0641 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 10:30:07 +0200 Subject: [PATCH 07/29] refactor: adds data migration statement to flyway script --- ...rces_and_human_readable_to_negotiation.sql | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql index 179c520d0..189b5973b 100644 --- a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql @@ -1,7 +1,18 @@ -ALTER TABLE negotiation ADD COLUMN human_readable text NOT NULL; +alter table negotiation +add column human_readable text not null default = '' -CREATE TABLE negotiation_resources_link ( - negotiation_id character varying(255) NOT NULL REFERENCES negotiation(id), - resource_id bigint NOT NULL REFERENCES resource(id), - PRIMARY KEY (negotiation_id, resource_id) -); +create table negotiation_resources_link ( + negotiation_id character varying(255) not null references negotiation(id), + resource_id bigint not null references resource(id), + primary key (negotiation_id, resource_id) +) + +update negotiation +set human_readable = ( + select human_readable + from request + where negotiation.id = request.negotiation_id +) + +insert into negotiation_resources_link (negotiation_id, resource_id) +select r.negotiation_id, rrl.resource_id from request_resources_link rrl join request r on rrl.request_id = r.id From 6e68fb07ae0a9cd7c93cdc63ec2eac6e23bfd289 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 10:41:31 +0200 Subject: [PATCH 08/29] refactor(db): fix the flyway migration script --- ...__move_resources_and_human_readable_to_negotiation.sql | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql index 189b5973b..be550e45a 100644 --- a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql @@ -1,18 +1,18 @@ alter table negotiation -add column human_readable text not null default = '' +add column human_readable text not null; create table negotiation_resources_link ( negotiation_id character varying(255) not null references negotiation(id), resource_id bigint not null references resource(id), primary key (negotiation_id, resource_id) -) +); update negotiation set human_readable = ( select human_readable from request where negotiation.id = request.negotiation_id -) +); insert into negotiation_resources_link (negotiation_id, resource_id) -select r.negotiation_id, rrl.resource_id from request_resources_link rrl join request r on rrl.request_id = r.id +select r.negotiation_id, rrl.resource_id from request_resources_link rrl join request r on rrl.request_id = r.id; From a2480d59811a4b20d6832005512fe8dd823ef394 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 10:57:46 +0200 Subject: [PATCH 09/29] refactor: adds delete of requests in the migration scripts and some other minor changes --- .../negotiator/notification/UserNotificationServiceImpl.java | 3 +-- ...V16.0__move_resources_and_human_readable_to_negotiation.sql | 2 ++ .../negotiator/integration/api/v2/QueryV2ControllerTests.java | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java index eb13784ec..52bb7aebc 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java @@ -300,8 +300,7 @@ private List createNotificationsForRepresentatives( private List createNotificationsForAdmins( Negotiation negotiation, NotificationEmailStatus status, String notificationMessage) { List newNotifications = new ArrayList<>(); - List admins = personRepository.findAllByAdminIsTrue(); - for (Person admin : admins) { + for (Person admin : personRepository.findAllByAdminIsTrue()) { Notification newNotification = buildNewNotification(negotiation, status, admin, notificationMessage); newNotifications.add(newNotification); diff --git a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql index be550e45a..466ab904e 100644 --- a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql @@ -16,3 +16,5 @@ set human_readable = ( insert into negotiation_resources_link (negotiation_id, resource_id) select r.negotiation_id, rrl.resource_id from request_resources_link rrl join request r on rrl.request_id = r.id; + +delete from request where negotiation_id is null; \ No newline at end of file diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java index 3450d129b..e3aa8ec0d 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v2/QueryV2ControllerTests.java @@ -142,7 +142,7 @@ public void testCreate_Ok() throws Exception { assertEquals(requestRepository.count(), previousCount + 1); } - @Deprecated + @Disabled @Test public void testUpdate_CreateWhenRequestIsNotFound() throws Exception { QueryCreateV2DTO updateRequest = TestUtils.createQueryV2Request(); From 936f5c61f175081cf0f85b8482b271d8c045fbd8 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 11:33:11 +0200 Subject: [PATCH 10/29] refactor: changes NegotiationCreateDTO to have only one request --- .../negotiation/NegotiationServiceImpl.java | 35 ++++++++----------- .../negotiation/dto/NegotiationCreateDTO.java | 2 +- .../api/v3/NegotiationControllerTests.java | 29 +++++---------- .../api/v3/RequestControllerTests.java | 2 +- .../integration/api/v3/TestUtils.java | 4 +-- .../NegotiationLifecycleServiceImplTest.java | 5 +-- .../service/UserNotificationServiceTest.java | 8 ++--- .../unit/service/NegotiationServiceTest.java | 18 +++++----- 8 files changed, 45 insertions(+), 58 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java index 8e539247d..4c8966590 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java @@ -90,12 +90,11 @@ public boolean isOrganizationPartOfNegotiation( negotiationId, organizationExternalId); } - private List findRequests(Set requestsId) { - List entities = requestRepository.findAllById(requestsId); - if (entities.size() < requestsId.size()) { - throw new WrongRequestException("One or more of the specified requests do not exist"); - } - return entities; + private Request findRequests(String requestsId) { + return requestRepository + .findById(requestsId) + .orElseThrow( + () -> new WrongRequestException("One or more of the specified requests do not exist")); } private List findAttachments(Set attachmentDTOs) { @@ -122,19 +121,17 @@ public boolean exists(String negotiationId) { * @return the created Negotiation entity */ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorId) { - Negotiation negotiationEntity = modelMapper.map(negotiationBody, Negotiation.class); - // Gets the Entities for the requests log.debug("Getting request entities"); - List requests = findRequests(negotiationBody.getRequests()); + Request request = findRequests(negotiationBody.getRequest()); - negotiationEntity.setResources( - new HashSet<>(requests.stream().findFirst().get().getResources())); - negotiationEntity.setHumanReadable(requests.stream().findFirst().get().getHumanReadable()); + Negotiation negotiation = modelMapper.map(negotiationBody, Negotiation.class); + negotiation.setResources(new HashSet<>(request.getResources())); + negotiation.setHumanReadable(request.getHumanReadable()); Negotiation savedNegotiation; try { // Finally, save the negotiation. NB: it also cascades operations for other Requests - savedNegotiation = negotiationRepository.save(negotiationEntity); + savedNegotiation = negotiationRepository.save(negotiation); } catch (DataException | DataIntegrityViolationException ex) { log.error("Error while saving the Negotiation into db. Some db constraint violated"); log.error(ex); @@ -143,17 +140,15 @@ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorI if (negotiationBody.getAttachments() != null) { List attachments = findAttachments(negotiationBody.getAttachments()); - negotiationEntity.setAttachments(new HashSet<>(attachments)); - attachments.forEach(attachment -> attachment.setNegotiation(negotiationEntity)); + negotiation.setAttachments(new HashSet<>(attachments)); + attachments.forEach(attachment -> attachment.setNegotiation(negotiation)); } - eventPublisher.publishEvent(new NewNegotiationEvent(this, negotiationEntity.getId())); + eventPublisher.publishEvent(new NewNegotiationEvent(this, negotiation.getId())); - for (Request request : requests) { - requestRepository.delete(request); - } + requestRepository.delete(request); // TODO: Add call to send email. - userNotificationService.notifyAdmins(negotiationEntity); + userNotificationService.notifyAdmins(negotiation); return modelMapper.map(savedNegotiation, NegotiationDTO.class); } diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationCreateDTO.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationCreateDTO.java index 4f0ba3ae7..94fb8c948 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationCreateDTO.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationCreateDTO.java @@ -21,7 +21,7 @@ @JsonInclude(value = JsonInclude.Include.NON_NULL) public class NegotiationCreateDTO { - @Valid @NotEmpty private Set requests; + @Valid @NotEmpty private String request; @NotNull private JsonNode payload; diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java index 5b659e6e4..242f5d0df 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java @@ -927,8 +927,8 @@ public void testCreate_Unauthorized() throws Exception { @Test @WithMockUser public void testCreate_BadRequest_whenRequests_IsMissing() throws Exception { - NegotiationCreateDTO request = TestUtils.createNegotiation(Set.of(REQUEST_UNASSIGNED)); - request.setRequests(null); + NegotiationCreateDTO request = TestUtils.createNegotiation(REQUEST_UNASSIGNED); + request.setRequest(null); mockMvc .perform( MockMvcRequestBuilders.post(NEGOTIATIONS_URL) @@ -940,7 +940,7 @@ public void testCreate_BadRequest_whenRequests_IsMissing() throws Exception { @Test @WithMockUser public void testCreate_BadRequest_whenRequests_IsEmpty() throws Exception { - NegotiationCreateDTO request = TestUtils.createNegotiation(Collections.emptySet()); + NegotiationCreateDTO request = TestUtils.createNegotiation(null); mockMvc .perform( MockMvcRequestBuilders.post(NEGOTIATIONS_URL) @@ -952,7 +952,7 @@ public void testCreate_BadRequest_whenRequests_IsEmpty() throws Exception { @Test @WithUserDetails("researcher") public void testCreate_BadRequest_whenSomeRequests_IsNotFound() throws Exception { - NegotiationCreateDTO request = TestUtils.createNegotiation(Set.of("unknown")); + NegotiationCreateDTO request = TestUtils.createNegotiation("unknown"); mockMvc .perform( MockMvcRequestBuilders.post(NEGOTIATIONS_URL) @@ -967,7 +967,7 @@ public void testCreate_BadRequest_whenSomeRequests_IsNotFound() throws Exception public void testCreate_BadRequest_whenRequest_IsAlreadyAssignedToAnotherRequest() throws Exception { // It tries to create a request by assigning the already assigned REQUEST_1 - NegotiationCreateDTO negotiationBody = TestUtils.createNegotiation(Set.of(REQUEST_1_ID)); + NegotiationCreateDTO negotiationBody = TestUtils.createNegotiation(REQUEST_1_ID); String requestBody = TestUtils.jsonFromRequest(negotiationBody); long previousRequestCount = negotiationRepository.count(); mockMvc @@ -985,7 +985,7 @@ public void testCreate_BadRequest_whenRequest_IsAlreadyAssignedToAnotherRequest( @WithUserDetails("researcher") // researcher not @Transactional public void testCreate_Ok() throws Exception { - NegotiationCreateDTO request = TestUtils.createNegotiation(Set.of(REQUEST_UNASSIGNED)); + NegotiationCreateDTO request = TestUtils.createNegotiation(REQUEST_UNASSIGNED); String requestBody = TestUtils.jsonFromRequest(request); long previousRequestCount = negotiationRepository.count(); MvcResult result = @@ -1013,7 +1013,7 @@ public void testCreate_Ok() throws Exception { @Test public void testUpdate_Unauthorized_whenNoAuth() throws Exception { - NegotiationCreateDTO negotiationBody = TestUtils.createNegotiation(Set.of(REQUEST_2_ID)); + NegotiationCreateDTO negotiationBody = TestUtils.createNegotiation(REQUEST_2_ID); TestUtils.checkErrorResponse( mockMvc, HttpMethod.PUT, @@ -1025,7 +1025,7 @@ public void testUpdate_Unauthorized_whenNoAuth() throws Exception { @Test public void testUpdate_Unauthorized_whenWrongAuth() throws Exception { - NegotiationCreateDTO request = TestUtils.createNegotiation(Set.of(REQUEST_UNASSIGNED)); + NegotiationCreateDTO request = TestUtils.createNegotiation(REQUEST_UNASSIGNED); TestUtils.checkErrorResponse( mockMvc, HttpMethod.PUT, @@ -1040,17 +1040,6 @@ public void testUpdate_Unauthorized_whenWrongAuth() throws Exception { @WithUserDetails("TheResearcher") public void testUpdate_BadRequest_whenRequestIsAlreadyAssignedToAnotherNegotiation() throws Exception { - // It tries to update the known NEGOTIATION_2 by assigning the request of NEGOTIATION_1 - NegotiationCreateDTO updateRequest = - TestUtils.createNegotiation(Set.of("request-1", "request-2")); - String requestBody = TestUtils.jsonFromRequest(updateRequest); - mockMvc - .perform( - MockMvcRequestBuilders.put("%s/%s".formatted(NEGOTIATIONS_URL, NEGOTIATION_2_ID)) - .contentType(MediaType.APPLICATION_JSON) - .content(requestBody)) - .andExpect(status().isBadRequest()) - .andExpect(content().contentType(MediaType.APPLICATION_JSON)); } @Test @@ -1059,7 +1048,7 @@ public void testUpdate_BadRequest_whenRequestIsAlreadyAssignedToAnotherNegotiati public void testUpdate_Ok_whenChangePayload() throws Exception { // Tries to updated negotiation // Negotiation body with updated values - NegotiationCreateDTO request = TestUtils.createNegotiation(Set.of(REQUEST_1_ID)); + NegotiationCreateDTO request = TestUtils.createNegotiation(REQUEST_1_ID); String requestBody = TestUtils.jsonFromRequest(request); requestBody = requestBody.replace("Title", "New Title"); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java index 314583f6e..a81c2863f 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java @@ -247,7 +247,7 @@ public void testGetById_Ok_whenNoNegotiationIsAssigned() throws Exception { @WithUserDetails("researcher") public void testGetById_Ok_whenNegotiationIsAssigned() throws Exception { RequestDTO r = requestService.create(TestUtils.createRequest(false)); - NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of(r.getId())); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(r.getId()); NegotiationDTO n = negotiationService.create(negotiationCreateDTO, CREATOR_ID); long previousCount = repository.count(); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/TestUtils.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/TestUtils.java index 8cb8b9a28..c8a7411d7 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/TestUtils.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/TestUtils.java @@ -102,7 +102,7 @@ public static QueryCreateV2DTO createQueryV2Request() { .build(); } - public static NegotiationCreateDTO createNegotiation(Set requestsId) throws IOException { + public static NegotiationCreateDTO createNegotiation(String requestsId) throws IOException { String payload = " {\n" + "\"project\": {\n" @@ -122,7 +122,7 @@ public static NegotiationCreateDTO createNegotiation(Set requestsId) thr ObjectMapper mapper = new ObjectMapper(); JsonNode jsonPayload = mapper.readTree(payload); - return NegotiationCreateDTO.builder().payload(jsonPayload).requests(requestsId).build(); + return NegotiationCreateDTO.builder().payload(jsonPayload).request(requestsId).build(); } public static String jsonFromRequest(Object request) throws JsonProcessingException { diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java index 7256a7874..ccf0f12cf 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java @@ -44,6 +44,7 @@ import java.util.Set; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.opentest4j.TestAbortedException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.test.context.support.WithMockUser; import org.springframework.security.test.context.support.WithUserDetails; @@ -136,8 +137,8 @@ void sendEvent_abandonNegotiation_to_inProcess_Negotiation() throws IOException } private NegotiationDTO saveNegotiation() throws IOException { - NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("request-2")); - Request request = requestRepository.findById("request-2").get(); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("request-2"); + Request request = requestRepository.findById("request-2").orElseThrow(TestAbortedException::new); Negotiation negotiation = Negotiation.builder() .resources(new HashSet<>(request.getResources())) diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java index a31be31c6..4b16b0597 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java @@ -283,7 +283,7 @@ public void notifyPendingNegotiation() throws IOException { userNotificationService.createRemindersOldNegotiations(); long initialNotificationsCount = countNegotiationNotifications("is awaiting review."); - NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("request-2")); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("request-2"); negotiationService.create(negotiationCreateDTO, 101L); userNotificationService.createRemindersOldNegotiations(); assertEquals( @@ -295,7 +295,7 @@ public void doNotNotifyApprovedNegotiation() throws IOException { userNotificationService.createRemindersOldNegotiations(); long initialNotificationsCount = countNegotiationNotifications("is awaiting review."); - NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("request-2")); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("request-2"); NegotiationDTO negotiationDTO = negotiationService.create(negotiationCreateDTO, 101L); negotiationRepository .findById(negotiationDTO.getId()) @@ -313,7 +313,7 @@ public void notifyStaleNegotiation() throws IOException { countNegotiationNotifications("is stale and had no status change in a while."); long initialNegotiationsCount = countInProgressNegotiations(); - NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("request-2")); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("request-2"); NegotiationDTO negotiationDTO = negotiationService.create(negotiationCreateDTO, 101L); Negotiation negotiation = negotiationRepository.findById(negotiationDTO.getId()).get(); negotiation.setCurrentState(NegotiationState.IN_PROGRESS); @@ -337,7 +337,7 @@ public void doNotNotifyUnreachableNegotiation() throws IOException { countNegotiationNotifications("is stale and had no status change in a while."); long initialNegotiationsCount = countInProgressNegotiations(); - NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("request-2")); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("request-2"); NegotiationDTO negotiationDTO = negotiationService.create(negotiationCreateDTO, 101L); Negotiation negotiation = negotiationRepository.findById(negotiationDTO.getId()).get(); negotiation.setCurrentState(NegotiationState.IN_PROGRESS); diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java index 6ec33f3bc..0c8dcea0e 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java @@ -47,6 +47,8 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; +import javax.swing.text.html.Option; + @ExtendWith(SpringExtension.class) @ContextConfiguration public class NegotiationServiceTest { @@ -126,13 +128,13 @@ public void test_isNegotiatorCreator_IsTrue_WhenPersonRepositoryIsNegotiatiorCre @Test void testCreateNegotiation_ok() throws IOException { - NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("requestID")); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("requestID"); Negotiation negotiation = Negotiation.builder().build(); Request request = new Request(); request.setResources(Set.of(new Resource())); negotiation.setResources(request.getResources()); - when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(request)); + when(requestRepository.findById("requestID")).thenReturn(Optional.of(request)); when(modelMapper.map(negotiationCreateDTO, Negotiation.class)).thenReturn(negotiation); NegotiationDTO savedDTO = new NegotiationDTO(); savedDTO.setId("saved"); @@ -145,7 +147,7 @@ void testCreateNegotiation_ok() throws IOException { @Test void testCreateNegotiation_ok_with_attachments() throws IOException { - NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("requestID")); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("requestID"); AttachmentMetadataDTO attachmentMetadataDTO = AttachmentMetadataDTO.builder().id("attachment-1").build(); negotiationCreateDTO.setAttachments(Set.of(attachmentMetadataDTO)); @@ -157,7 +159,7 @@ void testCreateNegotiation_ok_with_attachments() throws IOException { Attachment attachment = Attachment.builder().id("attachment-1").name("Attachment-1").build(); when(attachmentRepository.findAllById(List.of("attachment-1"))).thenReturn(List.of(attachment)); - when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(request)); + when(requestRepository.findById("requestID")).thenReturn(Optional.of(request)); when(modelMapper.map(negotiationCreateDTO, Negotiation.class)).thenReturn(negotiation); NegotiationDTO savedDTO = new NegotiationDTO(); @@ -171,12 +173,12 @@ void testCreateNegotiation_ok_with_attachments() throws IOException { @Test void testCreateNegotiation_fails_when_DataException() throws IOException { - NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("requestID")); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("requestID"); Negotiation negotiation = new Negotiation(); Request request = new Request(); request.setResources(Set.of(new Resource())); negotiation.setResources(request.getResources()); - when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(request)); + when(requestRepository.findById("requestID")).thenReturn(Optional.of(request)); when(modelMapper.map(negotiationCreateDTO, Negotiation.class)).thenReturn(negotiation); when(negotiationRepository.save(any())).thenThrow(DataException.class); assertThrows( @@ -187,12 +189,12 @@ void testCreateNegotiation_fails_when_DataException() throws IOException { @Test void testCreateNegotiation_fails_when_DataIntegrityViolationException() throws IOException { - NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation(Set.of("requestID")); + NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("requestID"); Negotiation negotiation = new Negotiation(); Request request = new Request(); request.setResources(Set.of(new Resource())); negotiation.setResources(request.getResources()); - when(requestRepository.findAllById(Set.of("requestID"))).thenReturn(List.of(request)); + when(requestRepository.findById("requestID")).thenReturn(Optional.of(request)); when(modelMapper.map(negotiationCreateDTO, Negotiation.class)).thenReturn(negotiation); when(negotiationRepository.save(negotiation)).thenThrow(DataIntegrityViolationException.class); assertThrows( From 76c708c966220b6d27dfe888287ebb4a127c0e0b Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 11:35:15 +0200 Subject: [PATCH 11/29] style: reformatting --- .../integration/api/v3/NegotiationControllerTests.java | 4 +--- .../service/NegotiationLifecycleServiceImplTest.java | 3 ++- .../negotiator/unit/service/NegotiationServiceTest.java | 2 -- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java index 242f5d0df..fbb269f43 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java @@ -33,7 +33,6 @@ import eu.bbmri_eric.negotiator.util.WithMockNegotiatorUser; import jakarta.transaction.Transactional; import java.net.URI; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -1039,8 +1038,7 @@ public void testUpdate_Unauthorized_whenWrongAuth() throws Exception { @Test @WithUserDetails("TheResearcher") public void testUpdate_BadRequest_whenRequestIsAlreadyAssignedToAnotherNegotiation() - throws Exception { - } + throws Exception {} @Test @WithUserDetails("TheResearcher") diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java index ccf0f12cf..6440ecf03 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java @@ -138,7 +138,8 @@ void sendEvent_abandonNegotiation_to_inProcess_Negotiation() throws IOException private NegotiationDTO saveNegotiation() throws IOException { NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("request-2"); - Request request = requestRepository.findById("request-2").orElseThrow(TestAbortedException::new); + Request request = + requestRepository.findById("request-2").orElseThrow(TestAbortedException::new); Negotiation negotiation = Negotiation.builder() .resources(new HashSet<>(request.getResources())) diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java index 0c8dcea0e..0a997127d 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/service/NegotiationServiceTest.java @@ -47,8 +47,6 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; -import javax.swing.text.html.Option; - @ExtendWith(SpringExtension.class) @ContextConfiguration public class NegotiationServiceTest { From 2aec21691c689c1dbc36a1ff4fff1cd53e53c3fb Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 12:15:26 +0200 Subject: [PATCH 12/29] refactor: removes the negotiation column from the request table --- .../resource/ResourceServiceImpl.java | 17 +++-------------- .../mappers/RequestModelsMapper.java | 8 -------- .../negotiator/negotiation/request/Request.java | 6 ------ .../negotiation/request/RequestRepository.java | 2 -- ...ources_and_human_readable_to_negotiation.sql | 4 +++- .../db/test/migration/R__Initial_data.sql | 16 ++++++++-------- .../api/v3/RequestControllerTests.java | 5 +---- .../repository/NotificationRepositoryTest.java | 1 - 8 files changed, 15 insertions(+), 44 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java index e7d2e8ceb..6dc8febda 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java @@ -12,7 +12,6 @@ import eu.bbmri_eric.negotiator.negotiation.NegotiationRepository; import eu.bbmri_eric.negotiator.negotiation.NewResourcesAddedEvent; import eu.bbmri_eric.negotiator.negotiation.dto.UpdateResourcesDTO; -import eu.bbmri_eric.negotiator.negotiation.request.Request; import eu.bbmri_eric.negotiator.negotiation.request.RequestRepository; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; @@ -165,19 +164,9 @@ private static void initializeStateForNewResources( } private Negotiation getNegotiation(String negotiationId) { - Negotiation negotiation = - negotiationRepository - .findById(negotiationId) - .orElseThrow(() -> new EntityNotFoundException(negotiationId)); - return negotiation; - } - - private Request getRequest(String negotiationId) { - Request request = - requestRepository - .findByNegotiation_Id(negotiationId) - .orElseThrow(() -> new EntityNotFoundException(negotiationId)); - return request; + return negotiationRepository + .findById(negotiationId) + .orElseThrow(() -> new EntityNotFoundException(negotiationId)); } private boolean userIsntAuthorized(String negotiationId, Long userId) { diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/RequestModelsMapper.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/RequestModelsMapper.java index c7a9f9f8b..2f1962e54 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/RequestModelsMapper.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/RequestModelsMapper.java @@ -42,14 +42,6 @@ public void addMappings() { mapper -> mapper.using(requestToRedirectUrl).map(Request::getId, RequestDTO::setRedirectUrl)); - Converter negotiationToNegotiationId = - q -> convertNegotiationToNegotiationId(q.getSource()); - typeMap.addMappings( - mapper -> - mapper - .using(negotiationToNegotiationId) - .map(Request::getNegotiation, RequestDTO::setNegotiationId)); - /////////////////////////////////////////// // Mapper from v2 Request to V3 Request TypeMap v2ToV3Map = diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/Request.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/Request.java index cb876f7d6..8efc47f01 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/Request.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/Request.java @@ -3,7 +3,6 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import eu.bbmri_eric.negotiator.discovery.DiscoveryService; import eu.bbmri_eric.negotiator.governance.resource.Resource; -import eu.bbmri_eric.negotiator.negotiation.Negotiation; import jakarta.persistence.Column; import jakarta.persistence.Entity; import jakarta.persistence.FetchType; @@ -64,11 +63,6 @@ public class Request { @NotNull private Set resources; - @ManyToOne(fetch = FetchType.LAZY) - @JoinColumn(name = "negotiation_id") - @Exclude - private Negotiation negotiation; - @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "discovery_service_id") @JsonIgnore diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/RequestRepository.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/RequestRepository.java index ef622d588..269bb3fb0 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/RequestRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/request/RequestRepository.java @@ -19,6 +19,4 @@ public interface RequestRepository extends JpaRepository { @EntityGraph(value = "request-with-detailed-resources") Optional findDetailedById(String id); - - Optional findByNegotiation_Id(String negotiationId); } diff --git a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql index 466ab904e..1edb38f39 100644 --- a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql @@ -17,4 +17,6 @@ set human_readable = ( insert into negotiation_resources_link (negotiation_id, resource_id) select r.negotiation_id, rrl.resource_id from request_resources_link rrl join request r on rrl.request_id = r.id; -delete from request where negotiation_id is null; \ No newline at end of file +delete from request where negotiation_id is null; + +alter table request drop column negotiation_id; \ No newline at end of file diff --git a/src/main/resources/db/test/migration/R__Initial_data.sql b/src/main/resources/db/test/migration/R__Initial_data.sql index 3325781b1..641acb60f 100644 --- a/src/main/resources/db/test/migration/R__Initial_data.sql +++ b/src/main/resources/db/test/migration/R__Initial_data.sql @@ -84,14 +84,14 @@ values (101, '2024-03-11', 101, '2024-03-31', 'REPRESENTATIVE_CONTACTED', 'negot (101, '2024-03-11', 101, '2024-03-31', 'REPRESENTATIVE_CONTACTED', 'negotiation-3', 5), (101, '2024-03-11', 101, '2024-03-31', 'RESOURCE_AVAILABLE', 'negotiation-3', 5); -insert into request (id, url, human_readable, discovery_service_id, negotiation_id) -values ('request-1', 'http://discoveryservice.dev', '#1: No filters used', 1, 'negotiation-1'), - ('request-2', 'http://discoveryservice.dev', '#1: DNA Samples', 1, null), - ('request-5', 'http://discoveryservice.dev', '#1: DNA Samples', 1, 'negotiation-5'), - ('request-v2', 'http://discoveryservice.dev', '#1: Blood Samples', 1, 'negotiation-v2'), - ('request-3', 'http://discoveryservice.dev', '#1: Blood Samples', 1, 'negotiation-3'), - ('request-4', 'http://discoveryservice.dev', '#1: Blood Samples', 1, 'negotiation-4'), - ('request-unassigned', 'http://discoveryservice.dev', '#1: Blood Samples', 1, null); +insert into request (id, url, human_readable, discovery_service_id) +values ('request-1', 'http://discoveryservice.dev', '#1: No filters used', 1), + ('request-2', 'http://discoveryservice.dev', '#1: DNA Samples', 1), + ('request-5', 'http://discoveryservice.dev', '#1: DNA Samples', 1), + ('request-v2', 'http://discoveryservice.dev', '#1: Blood Samples', 1), + ('request-3', 'http://discoveryservice.dev', '#1: Blood Samples', 1), + ('request-4', 'http://discoveryservice.dev', '#1: Blood Samples', 1), + ('request-unassigned', 'http://discoveryservice.dev', '#1: Blood Samples', 1); insert into request_resources_link (request_id, resource_id) values ('request-1', 4), diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java index a81c2863f..0ae98d0ba 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/RequestControllerTests.java @@ -205,6 +205,7 @@ public void testGetAll_Ok_whenNoNegotiationIsAssigned() throws Exception { assertEquals(repository.count(), previousCount); } + @Disabled @Test @WithMockUser public void testGetAll_Ok_whenNegotiationIsAssigned() throws Exception { @@ -226,8 +227,6 @@ public void testGetAll_Ok_whenNegotiationIsAssigned() throws Exception { @WithUserDetails("researcher") public void testGetById_Ok_whenNoNegotiationIsAssigned() throws Exception { RequestDTO r = requestService.create(TestUtils.createRequest(false)); - long previousCount = repository.count(); - mockMvc .perform( MockMvcRequestBuilders.get("%s/%s".formatted(ENDPOINT, r.getId())) @@ -236,9 +235,7 @@ public void testGetById_Ok_whenNoNegotiationIsAssigned() throws Exception { .andExpect(jsonPath("$.id").isString()) .andExpect(jsonPath("$.url", is("http://discoveryservice.dev"))) .andExpect(jsonPath("$.redirectUrl", containsString("http://localhost/request"))) - .andExpect(jsonPath("$.negotiationId").doesNotExist()) .andExpect(jsonPath("$.resources[0].id", is("biobank:1:collection:1"))); - assertEquals(repository.count(), previousCount); } // This is no longer possible since when the negotiation is assigned the request is removed diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java index ccb4259e1..212bab4cd 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java @@ -106,7 +106,6 @@ private Negotiation saveNegotiation(Person author) { .payload(payload) .build(); negotiation.setCreatedBy(author); - request.setNegotiation(negotiation); negotiationRepository.save(negotiation); return negotiation; } From 6facaf86d0eeb6981cddae06a911131a50105d3f Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 13:08:26 +0200 Subject: [PATCH 13/29] refactor: removes resources from NegotiationDTO --- .../bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java index e7db688f7..8cc74be05 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/dto/NegotiationDTO.java @@ -3,11 +3,9 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.JsonNode; -import eu.bbmri_eric.negotiator.governance.resource.dto.ResourceDTO; import eu.bbmri_eric.negotiator.user.UserResponseModel; import jakarta.validation.constraints.NotNull; import java.time.LocalDateTime; -import java.util.Set; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Getter; @@ -33,7 +31,6 @@ public class NegotiationDTO { @NotNull private boolean privatePostsEnabled; @NotNull private LocalDateTime creationDate; @NotNull private LocalDateTime modifiedDate; - @NotNull private Set resources; @JsonIgnore public String getStatusForResource(String resourceId) { From 72996b128b3f8a50821b9576c436310dd287dd4d Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 13:52:42 +0200 Subject: [PATCH 14/29] refactor: adds discovey service to negotiation --- .../negotiator/negotiation/Negotiation.java | 11 +++++++++++ .../negotiation/NegotiationServiceImpl.java | 1 + ...esources_and_human_readable_to_negotiation.sql | 8 +++++--- .../db/test/migration/R__Initial_data.sql | 15 +++++++-------- .../repository/AttachmentRepositoriesTest.java | 3 ++- .../repository/NegotiationRepositoryTest.java | 5 +++-- .../repository/NotificationRepositoryTest.java | 1 + .../repository/ResourceRepositoryTest.java | 1 + .../NegotiationLifecycleServiceImplTest.java | 7 +++++++ .../service/UserNotificationServiceTest.java | 1 + 10 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index bb8f9c3c2..ad9499507 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -1,8 +1,10 @@ package eu.bbmri_eric.negotiator.negotiation; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.vladmihalcea.hibernate.type.json.JsonType; import eu.bbmri_eric.negotiator.attachment.Attachment; import eu.bbmri_eric.negotiator.common.AuditEntity; +import eu.bbmri_eric.negotiator.discovery.DiscoveryService; import eu.bbmri_eric.negotiator.governance.resource.Resource; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationLifecycleRecord; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; @@ -16,11 +18,13 @@ import jakarta.persistence.Entity; import jakarta.persistence.EnumType; import jakarta.persistence.Enumerated; +import jakarta.persistence.FetchType; import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; import jakarta.persistence.JoinColumn; import jakarta.persistence.JoinTable; import jakarta.persistence.ManyToMany; +import jakarta.persistence.ManyToOne; import jakarta.persistence.MapKeyColumn; import jakarta.persistence.OneToMany; import jakarta.persistence.Table; @@ -115,6 +119,13 @@ public class Negotiation extends AuditEntity { private Set negotiationResourceLifecycleRecords = new HashSet<>(); + @ManyToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "discovery_service_id") + @JsonIgnore + @NotNull + @Exclude + private DiscoveryService discoveryService; + private static Set creteInitialHistory() { Set history = new HashSet<>(); history.add(NegotiationLifecycleRecord.builder().changedTo(NegotiationState.SUBMITTED).build()); diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java index 4c8966590..1885f66d3 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java @@ -127,6 +127,7 @@ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorI Negotiation negotiation = modelMapper.map(negotiationBody, Negotiation.class); negotiation.setResources(new HashSet<>(request.getResources())); negotiation.setHumanReadable(request.getHumanReadable()); + negotiation.setDiscoveryService(request.getDiscoveryService()); Negotiation savedNegotiation; try { diff --git a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql index 1edb38f39..e21340c24 100644 --- a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql @@ -1,5 +1,7 @@ alter table negotiation -add column human_readable text not null; +add column human_readable text not null default '', +add column discovery_service_id bigint not null default 1, +add constraint fkey_discovery_service_id FOREIGN KEY (discovery_service_id) REFERENCES discovery_service(id); create table negotiation_resources_link ( negotiation_id character varying(255) not null references negotiation(id), @@ -8,8 +10,8 @@ create table negotiation_resources_link ( ); update negotiation -set human_readable = ( - select human_readable +set (human_readable, discovery_service_id) = ( + select human_readable, discovery_service_id from request where negotiation.id = request.negotiation_id ); diff --git a/src/main/resources/db/test/migration/R__Initial_data.sql b/src/main/resources/db/test/migration/R__Initial_data.sql index 641acb60f..0f0c1424a 100644 --- a/src/main/resources/db/test/migration/R__Initial_data.sql +++ b/src/main/resources/db/test/migration/R__Initial_data.sql @@ -58,26 +58,25 @@ values (4, 103), (8, 105), (9, 105); -insert into negotiation (id, creation_date, current_state, modified_date, created_by, modified_by, human_readable, payload, private_posts_enabled, public_posts_enabled) +insert into negotiation (id, creation_date, current_state, modified_date, created_by, modified_by, human_readable, payload, private_posts_enabled, public_posts_enabled, discovery_service_id) values ('negotiation-1', '2024-10-12', 'IN_PROGRESS', '2024-10-12', 108, 108, '#1 Material Type: DNA', '{"project":{"title":"Biobanking project","description":"desc"},"samples":{"sample-type":"DNA","num-of-subjects": 10,"num-of-sample": "100","volume":3},"ethics-vote":{"ethics-vote":"My ethics"}}', - true, true), + true, true, 1), ('negotiation-2', '2024-03-12', 'SUBMITTED', '2024-04-02', 108, 108, '#1 Material Type: DNA; #2 Diagnosis: C18.2', '{"project":{"title":"Interesting project","description":"desc"},"samples":{"sample-type":"Plasma","num-of-subjects": 10,"num-of-sample": "100","volume":3},"ethics-vote":{"ethics-vote":"My ethics"}}', - false, true), + false, true, 1), ('negotiation-v2', '2023-04-12', 'ABANDONED', '2024-10-12', 108, 108, '#1 Diagnosis: C18.2', '{"project":{"title":"A Project 3","description":"Project 3 desc"},"samples":{"sample-type":"Blood","num-of-subjects": 5,"num-of-sample": "10","volume":4},"ethics-vote":{"ethics-vote":"My ethics"}}', - false, false), + false, false, 1), ('negotiation-3', '2024-02-24', 'IN_PROGRESS', '2024-02-24', 105, 105, '#1 Type: RD', '{"project":{"title":"Project 3","description":"Project 3 desc"},"samples":{"sample-type":"Blood","num-of-subjects": 5,"num-of-sample": "10","volume":4},"ethics-vote":{"ethics-vote":"My ethics"}}', - true, true), + true, true, 1), ('negotiation-4', '2024-01-10', 'ABANDONED', '2024-01-10', 105, 105, '#1 Type: Cohort', '{"project":{"title":"Project 3","description":"Project 3 desc"},"samples":{"sample-type":"Blood","num-of-subjects": 5,"num-of-sample": "10","volume":4},"ethics-vote":{"ethics-vote":"My ethics"}}', - false, false), + false, false, 1), ('negotiation-5', '2024-03-11', 'SUBMITTED', '2024-04-12', 108, 108, '#1 Quality: ISO', '{"project":{"title":"Yet another important project","description":"desc"},"samples":{"sample-type":"Plasma","num-of-subjects": 10,"num-of-sample": "100","volume":3},"ethics-vote":{"ethics-vote":"My ethics"}}', - false, true); - + false, true, 1); insert into negotiation_resource_lifecycle_record (created_by, creation_date, modified_by, modified_date, changed_to, negotiation_id, resource_id) values (101, '2024-03-11', 101, '2024-03-31', 'REPRESENTATIVE_CONTACTED', 'negotiation-1', 4), diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java index bcef008a4..38bfcc660 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java @@ -87,7 +87,8 @@ private Negotiation createNegotiation(String negotiationId, Resource resource) { Negotiation.builder() .id(negotiationId) .humanReadable("#1 Material Type: DNA") - .payload("{\"project\":{\"title\":\"title\"} }") + .payload("{\"project\":{\"title\":\"title\"}}") + .discoveryService(discoveryService) .resources(Set.of(resource)) .build(); return negotiationRepository.save(negotiation); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java index cd50edaee..6a5be308c 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java @@ -12,7 +12,6 @@ import eu.bbmri_eric.negotiator.negotiation.Negotiation; import eu.bbmri_eric.negotiator.negotiation.NegotiationRepository; import eu.bbmri_eric.negotiator.negotiation.NegotiationSpecification; -import eu.bbmri_eric.negotiator.negotiation.request.Request; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; import eu.bbmri_eric.negotiator.user.Person; import eu.bbmri_eric.negotiator.user.PersonRepository; @@ -164,6 +163,7 @@ void save_10000differentResources_ok() { .resources(new HashSet<>(resources)) .publicPostsEnabled(false) .humanReadable("#1 Material Type: DNA") + .discoveryService(discoveryService) .payload(payload) .build(); negotiation = negotiationRepository.save(negotiation); @@ -322,6 +322,7 @@ private void saveNegotiation() { .currentState(NegotiationState.SUBMITTED) .resources(resources) .humanReadable("#1 Material Type: DNA") + .discoveryService(discoveryService) .publicPostsEnabled(false) .payload(payload) .build(); @@ -331,7 +332,6 @@ private void saveNegotiation() { } private void saveNegotiation(Person author) { - Set requests = new HashSet<>(); Set resources = new HashSet<>(); resources.add(resource); Negotiation negotiation = @@ -339,6 +339,7 @@ private void saveNegotiation(Person author) { .currentState(NegotiationState.SUBMITTED) .resources(resources) .humanReadable("everything") + .discoveryService(discoveryService) .publicPostsEnabled(false) .payload(payload) .build(); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java index 212bab4cd..b4913dea8 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java @@ -103,6 +103,7 @@ private Negotiation saveNegotiation(Person author) { .resources(resources) .humanReadable("#1 Material Type: DNA") .publicPostsEnabled(false) + .discoveryService(discoveryService) .payload(payload) .build(); negotiation.setCreatedBy(author); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java index c5b85e9ef..dd4f70d2a 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java @@ -119,6 +119,7 @@ void findAllByNegotiationId() { .currentState(NegotiationState.SUBMITTED) .resources(Set.of(res1, res2)) .humanReadable("#1 MaterialType: DNA") + .discoveryService(discoveryService) .publicPostsEnabled(false) .payload(payload) .build(); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java index 6440ecf03..2cb6defce 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java @@ -8,6 +8,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import eu.bbmri_eric.negotiator.common.exceptions.EntityNotFoundException; +import eu.bbmri_eric.negotiator.discovery.DiscoveryService; +import eu.bbmri_eric.negotiator.discovery.DiscoveryServiceRepository; import eu.bbmri_eric.negotiator.form.AccessForm; import eu.bbmri_eric.negotiator.form.repository.AccessFormRepository; import eu.bbmri_eric.negotiator.governance.resource.Resource; @@ -67,6 +69,7 @@ public class NegotiationLifecycleServiceImplTest { @Autowired private InformationSubmissionRepository informationSubmissionRepository; @Autowired private ResourceRepository resourceRepository; @Autowired ApplicationEvents events; + @Autowired private DiscoveryServiceRepository discoveryServiceRepository; private void checkNegotiationResourceRecordPresenceWithAssignedState( String negotiationId, NegotiationResourceState negotiationResourceState) { @@ -138,11 +141,15 @@ void sendEvent_abandonNegotiation_to_inProcess_Negotiation() throws IOException private NegotiationDTO saveNegotiation() throws IOException { NegotiationCreateDTO negotiationCreateDTO = TestUtils.createNegotiation("request-2"); + DiscoveryService discoveryService = + discoveryServiceRepository.findById(1L).orElseThrow(TestAbortedException::new); + Request request = requestRepository.findById("request-2").orElseThrow(TestAbortedException::new); Negotiation negotiation = Negotiation.builder() .resources(new HashSet<>(request.getResources())) + .discoveryService(discoveryService) .humanReadable("#1 MaterialType: DNA") .payload(negotiationCreateDTO.getPayload().toString()) .build(); diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java index 4b16b0597..727dcfba4 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java @@ -134,6 +134,7 @@ void notifyRepresentatives_sameRepFor2Resources_oneNotification() { Negotiation negotiation = Negotiation.builder() .humanReadable("query") + .discoveryService(resource1.getDiscoveryService()) .resources(Set.of(resource1, resource2)) .payload("{\"project\":{\"title\":\"A Project 3\",\"description\":\"Project 3 desc\"}}") .build(); From 7c57191c3adec19682a2aec2b7f6f5a0ecae1c3f Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Wed, 25 Sep 2024 17:35:25 +0200 Subject: [PATCH 15/29] refactor: wip removing currentStatePerResource --- .../resource/ResourceRepository.java | 2 +- .../negotiator/negotiation/Negotiation.java | 53 +++++++++++++++---- .../negotiation/NegotiationRepository.java | 11 ++-- .../negotiation/NegotiationResourceLink.java | 38 +++++++++++++ .../NegotiationResourceLinkId.java | 22 ++++++++ .../negotiator/user/PersonRepository.java | 2 +- ...rces_and_human_readable_to_negotiation.sql | 6 ++- .../db/test/migration/R__Initial_data.sql | 2 +- .../AttachmentRepositoriesTest.java | 2 +- .../repository/NegotiationRepositoryTest.java | 6 +-- .../NotificationRepositoryTest.java | 2 +- .../repository/ResourceRepositoryTest.java | 2 +- .../service/UserNotificationServiceTest.java | 2 +- .../unit/mappers/NegotiationMapperTest.java | 2 +- .../unit/service/PostServiceTest.java | 2 +- 15 files changed, 126 insertions(+), 28 deletions(-) create mode 100644 src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java create mode 100644 src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java diff --git a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java index 744569a9d..2941731e2 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java @@ -28,7 +28,7 @@ public interface ResourceRepository o.external_id as organizationExternalId, o.id as organizationId from resource rs - join public.negotiation_resources_link nrl on rs.id = nrl.resource_id + join public.negotiation_resource_link nrl on rs.id = nrl.resource_id join public.organization o on o.id = rs.organization_id left join public.resource_state_per_negotiation rspn on rs.source_id = rspn.resource_id and nrl.negotiation_id = rspn.negotiation_id where diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index ad9499507..7a0cfac3f 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -22,8 +22,6 @@ import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; import jakarta.persistence.JoinColumn; -import jakarta.persistence.JoinTable; -import jakarta.persistence.ManyToMany; import jakarta.persistence.ManyToOne; import jakarta.persistence.MapKeyColumn; import jakarta.persistence.OneToMany; @@ -34,6 +32,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.Builder; @@ -71,14 +70,8 @@ public class Negotiation extends AuditEntity { @Column(columnDefinition = "TEXT") private String humanReadable = ""; - @ManyToMany - @JoinTable( - name = "negotiation_resources_link", - joinColumns = @JoinColumn(name = "negotiation_id"), - inverseJoinColumns = @JoinColumn(name = "resource_id")) - @Exclude - @NotNull - private Set resources = new HashSet<>(); + @OneToMany @Exclude @NotNull @Builder.Default + private Set resourcesLink = new HashSet<>(); @Formula(value = "JSONB_EXTRACT_PATH_TEXT(payload, 'project', 'title')") private String title; @@ -149,6 +142,16 @@ public void setStateForResource(String resourceId, NegotiationResourceState stat } } + public static CustomNegotiationBuilder builder() { + return new CustomNegotiationBuilder(); + } + + public Set getResources() { + return getResourcesLink().stream() + .map(NegotiationResourceLink::getResource) + .collect(Collectors.toSet()); + } + @Override public boolean equals(Object o) { if (this == o) { @@ -172,4 +175,34 @@ private Resource lookupResource(Set resources, String resourceId) { .findFirst() .orElse(null); } + + public void setResources(Set resources) { + resources.forEach( + resource -> this.resourcesLink.add(new NegotiationResourceLink(this, resource, null))); + } + + public static class NegotiationBuilder { + public NegotiationBuilder resources(Set resources) { + return this; + } + } + + public static class CustomNegotiationBuilder extends NegotiationBuilder { + private Set resources; + + @Override + public CustomNegotiationBuilder resources(Set resources) { + this.resources = resources; + return this; + } + + @Override + public Negotiation build() { + Negotiation negotiation = super.build(); + if (this.resources != null) { + negotiation.setResources(this.resources); + } + return negotiation; + } + } } diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java index 11f78f78c..8e4d83e9e 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java @@ -41,7 +41,7 @@ List findByModifiedDateBeforeAndCurrentState( "SELECT EXISTS (" + "SELECT distinct(n.id) " + "FROM negotiation n " - + " JOIN negotiation_resources_link rrl ON rrl.negotiation_id = n.id " + + " JOIN negotiation_resource_link rrl ON rrl.negotiation_id = n.id " + " JOIN resource rs ON rrl.resource_id = rs.id " + " JOIN organization o ON rs.organization_id = o.id " + "WHERE n.id = :negotiationId and o.external_id = :organizationExternalId)", @@ -52,7 +52,8 @@ List findByModifiedDateBeforeAndCurrentState( value = "SELECT distinct (n) " + "FROM Negotiation n " - + "JOIN n.resources rs " + + "JOIN n.resourcesLink rl " + + "JOIN rl.id.resource rs " + "JOIN rs.networks net " + "WHERE net.id = :networkId") Page findAllForNetwork(Long networkId, Pageable pageable); @@ -61,7 +62,8 @@ List findByModifiedDateBeforeAndCurrentState( value = "select count (distinct n.id) " + "FROM Negotiation n " - + "JOIN n.resources rs " + + "JOIN n.resourcesLink rl " + + "JOIN rl.id.resource rs " + "JOIN rs.networks net " + "WHERE net.id = :networkId") Integer countAllForNetwork(Long networkId); @@ -70,7 +72,8 @@ List findByModifiedDateBeforeAndCurrentState( value = "select n.currentState, COUNT ( distinct n.id)" + "FROM Negotiation n " - + "JOIN n.resources rs " + + "JOIN n.resourcesLink rl " + + "JOIN rl.id.resource rs " + "JOIN rs.networks net " + "WHERE net.id = :networkId group by n.currentState") List countStatusDistribution(Long networkId); diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java new file mode 100644 index 000000000..faca70b08 --- /dev/null +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java @@ -0,0 +1,38 @@ +package eu.bbmri_eric.negotiator.negotiation; + +import eu.bbmri_eric.negotiator.governance.resource.Resource; +import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; +import jakarta.persistence.EmbeddedId; +import jakarta.persistence.Entity; +import jakarta.persistence.Table; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +@Getter +@Setter +@NoArgsConstructor(access = AccessLevel.PROTECTED) +@AllArgsConstructor(access = AccessLevel.PROTECTED) +@Entity +@Table(name = "negotiation_resource_link") +public class NegotiationResourceLink { + @EmbeddedId private NegotiationResourceLinkId id; + + private NegotiationResourceState currentState; + + public NegotiationResourceLink( + Negotiation negotiation, Resource resource, NegotiationResourceState currentState) { + this.id = new NegotiationResourceLinkId(negotiation, resource); + this.currentState = currentState; + } + + public Negotiation getNegotiation() { + return this.id.getNegotiation(); + } + + public Resource getResource() { + return this.id.getResource(); + } +} diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java new file mode 100644 index 000000000..f662f5ed0 --- /dev/null +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java @@ -0,0 +1,22 @@ +package eu.bbmri_eric.negotiator.negotiation; + +import eu.bbmri_eric.negotiator.governance.resource.Resource; +import jakarta.persistence.Embeddable; +import jakarta.persistence.ManyToOne; +import lombok.AllArgsConstructor; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +@Embeddable +@Getter +@Setter +@NoArgsConstructor +@AllArgsConstructor +@EqualsAndHashCode +public class NegotiationResourceLinkId { + @ManyToOne private Negotiation negotiation; + + @ManyToOne private Resource resource; +} diff --git a/src/main/java/eu/bbmri_eric/negotiator/user/PersonRepository.java b/src/main/java/eu/bbmri_eric/negotiator/user/PersonRepository.java index b265994de..0b75af660 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/user/PersonRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/user/PersonRepository.java @@ -42,7 +42,7 @@ public interface PersonRepository value = "select exists ( " + "select nrl.resource_id " - + "from negotiation_resources_link nrl " + + "from negotiation_resource_link nrl " + "where nrl.negotiation_id = :negotiationId and " + " nrl.resource_id in ( " + " select rrl.resource_id " diff --git a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql index e21340c24..1d7da439a 100644 --- a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql @@ -1,11 +1,13 @@ alter table negotiation add column human_readable text not null default '', add column discovery_service_id bigint not null default 1, + add constraint fkey_discovery_service_id FOREIGN KEY (discovery_service_id) REFERENCES discovery_service(id); -create table negotiation_resources_link ( +create table negotiation_resource_link ( negotiation_id character varying(255) not null references negotiation(id), resource_id bigint not null references resource(id), + current_state varchar(255), primary key (negotiation_id, resource_id) ); @@ -16,7 +18,7 @@ set (human_readable, discovery_service_id) = ( where negotiation.id = request.negotiation_id ); -insert into negotiation_resources_link (negotiation_id, resource_id) +insert into negotiation_resource_link (negotiation_id, resource_id) select r.negotiation_id, rrl.resource_id from request_resources_link rrl join request r on rrl.request_id = r.id; delete from request where negotiation_id is null; diff --git a/src/main/resources/db/test/migration/R__Initial_data.sql b/src/main/resources/db/test/migration/R__Initial_data.sql index 0f0c1424a..f43bd022f 100644 --- a/src/main/resources/db/test/migration/R__Initial_data.sql +++ b/src/main/resources/db/test/migration/R__Initial_data.sql @@ -103,7 +103,7 @@ values ('request-1', 4), ('request-5', 5), ('request-5', 7); -insert into negotiation_resources_link (negotiation_id, resource_id) +insert into negotiation_resource_link (negotiation_id, resource_id) values ('negotiation-1', 4), ('negotiation-v2', 7), ('negotiation-3', 5), diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java index 38bfcc660..77d1f6f54 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java @@ -85,11 +85,11 @@ private Resource createResource(Organization organization, String resourceId) { private Negotiation createNegotiation(String negotiationId, Resource resource) { Negotiation negotiation = Negotiation.builder() + .resources(Set.of(resource)) .id(negotiationId) .humanReadable("#1 Material Type: DNA") .payload("{\"project\":{\"title\":\"title\"}}") .discoveryService(discoveryService) - .resources(Set.of(resource)) .build(); return negotiationRepository.save(negotiation); } diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java index 6a5be308c..ac2acf037 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NegotiationRepositoryTest.java @@ -159,8 +159,8 @@ void save_10000differentResources_ok() { Negotiation negotiation = Negotiation.builder() - .currentState(NegotiationState.SUBMITTED) .resources(new HashSet<>(resources)) + .currentState(NegotiationState.SUBMITTED) .publicPostsEnabled(false) .humanReadable("#1 Material Type: DNA") .discoveryService(discoveryService) @@ -319,8 +319,8 @@ private void saveNegotiation() { resources.add(resource); Negotiation negotiation = Negotiation.builder() - .currentState(NegotiationState.SUBMITTED) .resources(resources) + .currentState(NegotiationState.SUBMITTED) .humanReadable("#1 Material Type: DNA") .discoveryService(discoveryService) .publicPostsEnabled(false) @@ -336,8 +336,8 @@ private void saveNegotiation(Person author) { resources.add(resource); Negotiation negotiation = Negotiation.builder() - .currentState(NegotiationState.SUBMITTED) .resources(resources) + .currentState(NegotiationState.SUBMITTED) .humanReadable("everything") .discoveryService(discoveryService) .publicPostsEnabled(false) diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java index b4913dea8..1daecc85f 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/NotificationRepositoryTest.java @@ -99,8 +99,8 @@ private Negotiation saveNegotiation(Person author) { requests.add(request); Negotiation negotiation = Negotiation.builder() - .currentState(NegotiationState.SUBMITTED) .resources(resources) + .currentState(NegotiationState.SUBMITTED) .humanReadable("#1 Material Type: DNA") .publicPostsEnabled(false) .discoveryService(discoveryService) diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java index dd4f70d2a..cea5abbdf 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/ResourceRepositoryTest.java @@ -116,8 +116,8 @@ void findAllByNegotiationId() { Negotiation negotiation = Negotiation.builder() - .currentState(NegotiationState.SUBMITTED) .resources(Set.of(res1, res2)) + .currentState(NegotiationState.SUBMITTED) .humanReadable("#1 MaterialType: DNA") .discoveryService(discoveryService) .publicPostsEnabled(false) diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java index 727dcfba4..ad873c7a3 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java @@ -133,9 +133,9 @@ void notifyRepresentatives_sameRepFor2Resources_oneNotification() { Negotiation negotiation = Negotiation.builder() + .resources(Set.of(resource1, resource2)) .humanReadable("query") .discoveryService(resource1.getDiscoveryService()) - .resources(Set.of(resource1, resource2)) .payload("{\"project\":{\"title\":\"A Project 3\",\"description\":\"Project 3 desc\"}}") .build(); negotiation.setStateForResource(resource2.getSourceId(), NegotiationResourceState.SUBMITTED); diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/mappers/NegotiationMapperTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/mappers/NegotiationMapperTest.java index bc95863e4..96f782165 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/mappers/NegotiationMapperTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/mappers/NegotiationMapperTest.java @@ -38,8 +38,8 @@ private static Negotiation buildNegotiation() { .build()); Negotiation negotiation = Negotiation.builder() - .resources(resources) .humanReadable("#1 Material Type: DNA") + .resources(resources) .currentState(NegotiationState.SUBMITTED) .build(); negotiation.setCreationDate(LocalDateTime.of(2023, Month.SEPTEMBER, 19, 00, 00)); diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java index af55fc51b..e8d7d22fe 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/service/PostServiceTest.java @@ -153,8 +153,8 @@ void before() { negotiation = Negotiation.builder() - .id(NEG_1) .resources(Set.of(resource1, resource2)) + .id(NEG_1) .humanReadable("#1 Material Type: DNA") .build(); negotiation.setCreatedBy(researcher); From e7651eacb15df5b89532cfc2e69ec06c78e2d92b Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Wed, 25 Sep 2024 22:36:42 +0200 Subject: [PATCH 16/29] fix: move resources directly to negotiation Signed-off-by: RadovanTomik --- .../negotiator/negotiation/Negotiation.java | 13 ++++++++- .../negotiation/NegotiationController.java | 1 - .../negotiation/NegotiationResourceLink.java | 2 -- .../NegotiationResourceLinkId.java | 29 +++++++++++++++---- .../negotiation/NegotiationSpecification.java | 4 +-- .../AttachmentRepositoriesTest.java | 1 - 6 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index 7a0cfac3f..2f7efdb9a 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -70,7 +70,10 @@ public class Negotiation extends AuditEntity { @Column(columnDefinition = "TEXT") private String humanReadable = ""; - @OneToMany @Exclude @NotNull @Builder.Default + @OneToMany(mappedBy = "id.negotiation", + fetch = FetchType.EAGER, + cascade = CascadeType.ALL) + @Exclude @NotNull @Builder.Default private Set resourcesLink = new HashSet<>(); @Formula(value = "JSONB_EXTRACT_PATH_TEXT(payload, 'project', 'title')") @@ -169,6 +172,13 @@ public int hashCode() { return Objects.hash(getId()); } + @Override + public String toString() { + return "Negotiation{" + + "id='" + id + '\'' + + '}'; + } + private Resource lookupResource(Set resources, String resourceId) { return resources.stream() .filter(r -> r.getSourceId().equals(resourceId)) @@ -177,6 +187,7 @@ private Resource lookupResource(Set resources, String resourceId) { } public void setResources(Set resources) { + this.resourcesLink.clear(); resources.forEach( resource -> this.resourcesLink.add(new NegotiationResourceLink(this, resource, null))); } diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationController.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationController.java index bd8084bd1..a54ca2a83 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationController.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationController.java @@ -333,7 +333,6 @@ public CollectionModel> findResourcesForNegot @SecurityRequirement(name = "security_auth") public CollectionModel> updateResources( @PathVariable String id, @RequestBody @Valid UpdateResourcesDTO updateResourcesDTO) { - log.info(updateResourcesDTO.toString()); return resourceWithStatusAssembler.toCollectionModel( resourceService.updateResourcesInANegotiation(id, updateResourcesDTO)); } diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java index faca70b08..58ab50382 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java @@ -4,7 +4,6 @@ import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; import jakarta.persistence.EmbeddedId; import jakarta.persistence.Entity; -import jakarta.persistence.Table; import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.Getter; @@ -16,7 +15,6 @@ @NoArgsConstructor(access = AccessLevel.PROTECTED) @AllArgsConstructor(access = AccessLevel.PROTECTED) @Entity -@Table(name = "negotiation_resource_link") public class NegotiationResourceLink { @EmbeddedId private NegotiationResourceLinkId id; diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java index f662f5ed0..142598d1c 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java @@ -2,21 +2,40 @@ import eu.bbmri_eric.negotiator.governance.resource.Resource; import jakarta.persistence.Embeddable; +import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; +import java.io.Serializable; +import java.util.Objects; import lombok.AllArgsConstructor; -import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; +import lombok.ToString; @Embeddable @Getter @Setter @NoArgsConstructor @AllArgsConstructor -@EqualsAndHashCode -public class NegotiationResourceLinkId { - @ManyToOne private Negotiation negotiation; +@ToString +public class NegotiationResourceLinkId implements Serializable { + @ManyToOne(optional = false) + @JoinColumn + private Negotiation negotiation; - @ManyToOne private Resource resource; + @JoinColumn + @ManyToOne(optional = false) private Resource resource; + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + NegotiationResourceLinkId that = (NegotiationResourceLinkId) o; + return Objects.equals(negotiation, that.negotiation) && Objects.equals(resource, that.resource); + } + + @Override + public int hashCode() { + return Objects.hash(negotiation, resource); + } } diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java index b961b26f6..a8f24eed1 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java @@ -105,7 +105,7 @@ public Predicate toPredicate( query.distinct(true); Predicate authorPredicate = criteriaBuilder.equal(root.get("createdBy"), person); if (!person.getResources().isEmpty()) { - Predicate involvedInResources = root.joinSet("resources").in(person.getResources()); + Predicate involvedInResources = root.joinSet("resourcesLink").join("id").join("resource").in(person.getResources()); return criteriaBuilder.or(authorPredicate, involvedInResources); } return authorPredicate; @@ -147,7 +147,7 @@ public Predicate toPredicate( @Nonnull CriteriaQuery query, @Nonnull CriteriaBuilder criteriaBuilder) { query.distinct(true); - return root.joinSet("resources").in(resources); + return root.joinSet("resourcesLink").join("id").join("resource").in(resources); } }; } diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java index 77d1f6f54..bef09b5bc 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/repository/AttachmentRepositoriesTest.java @@ -86,7 +86,6 @@ private Negotiation createNegotiation(String negotiationId, Resource resource) { Negotiation negotiation = Negotiation.builder() .resources(Set.of(resource)) - .id(negotiationId) .humanReadable("#1 Material Type: DNA") .payload("{\"project\":{\"title\":\"title\"}}") .discoveryService(discoveryService) From 9ffdb52cba27ecef2741623f8f321d39a824abd7 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Wed, 25 Sep 2024 22:36:46 +0200 Subject: [PATCH 17/29] fix: move resources directly to negotiation Signed-off-by: RadovanTomik --- .../negotiator/negotiation/Negotiation.java | 12 +++++------- .../negotiation/NegotiationResourceLinkId.java | 3 ++- .../negotiation/NegotiationSpecification.java | 3 ++- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index 2f7efdb9a..ff8d7a982 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -70,10 +70,10 @@ public class Negotiation extends AuditEntity { @Column(columnDefinition = "TEXT") private String humanReadable = ""; - @OneToMany(mappedBy = "id.negotiation", - fetch = FetchType.EAGER, - cascade = CascadeType.ALL) - @Exclude @NotNull @Builder.Default + @OneToMany(mappedBy = "id.negotiation", fetch = FetchType.EAGER, cascade = CascadeType.ALL) + @Exclude + @NotNull + @Builder.Default private Set resourcesLink = new HashSet<>(); @Formula(value = "JSONB_EXTRACT_PATH_TEXT(payload, 'project', 'title')") @@ -174,9 +174,7 @@ public int hashCode() { @Override public String toString() { - return "Negotiation{" + - "id='" + id + '\'' + - '}'; + return "Negotiation{" + "id='" + id + '\'' + '}'; } private Resource lookupResource(Set resources, String resourceId) { diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java index 142598d1c..d9aa81ed3 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLinkId.java @@ -24,7 +24,8 @@ public class NegotiationResourceLinkId implements Serializable { private Negotiation negotiation; @JoinColumn - @ManyToOne(optional = false) private Resource resource; + @ManyToOne(optional = false) + private Resource resource; @Override public boolean equals(Object o) { diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java index a8f24eed1..61dbfbe55 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationSpecification.java @@ -105,7 +105,8 @@ public Predicate toPredicate( query.distinct(true); Predicate authorPredicate = criteriaBuilder.equal(root.get("createdBy"), person); if (!person.getResources().isEmpty()) { - Predicate involvedInResources = root.joinSet("resourcesLink").join("id").join("resource").in(person.getResources()); + Predicate involvedInResources = + root.joinSet("resourcesLink").join("id").join("resource").in(person.getResources()); return criteriaBuilder.or(authorPredicate, involvedInResources); } return authorPredicate; From 1f3db98f82ce461dddf9e50843fbe5907cf73796 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Thu, 26 Sep 2024 09:18:12 +0200 Subject: [PATCH 18/29] refactor: removes the default values from new negotiation's columns --- ...ove_resources_and_human_readable_to_negotiation.sql | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql index 1d7da439a..b02fae3e9 100644 --- a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql @@ -1,7 +1,6 @@ alter table negotiation -add column human_readable text not null default '', -add column discovery_service_id bigint not null default 1, - +add column human_readable text, +add column discovery_service_id bigint, add constraint fkey_discovery_service_id FOREIGN KEY (discovery_service_id) REFERENCES discovery_service(id); create table negotiation_resource_link ( @@ -18,9 +17,12 @@ set (human_readable, discovery_service_id) = ( where negotiation.id = request.negotiation_id ); +alter table negotiation alter column human_readable set not null; +alter table negotiation alter column discovery_service_id set not null; + insert into negotiation_resource_link (negotiation_id, resource_id) select r.negotiation_id, rrl.resource_id from request_resources_link rrl join request r on rrl.request_id = r.id; delete from request where negotiation_id is null; +alter table request drop column negotiation_id; -alter table request drop column negotiation_id; \ No newline at end of file From 8dd3353bf7eac56442ec8974f5d34cd524b6d661 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Thu, 26 Sep 2024 09:25:08 +0200 Subject: [PATCH 19/29] fix: move edit resource link on status change Signed-off-by: RadovanTomik --- .../resource/ResourceServiceImpl.java | 8 ++-- .../negotiator/negotiation/Negotiation.java | 48 +++++++++---------- .../unit/model/NegotiationTest.java | 18 +++++-- 3 files changed, 43 insertions(+), 31 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java index 6dc8febda..63c8f7d4f 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java @@ -115,8 +115,8 @@ public List updateResourcesInANegotiation( private void updateResources(String negotiationId, UpdateResourcesDTO updateResourcesDTO) { Negotiation negotiation = getNegotiation(negotiationId); Set resources = getResources(negotiationId, updateResourcesDTO, negotiation); - initializeStateForNewResources(negotiation, resources, updateResourcesDTO.getState()); - persistChanges(negotiation, resources); + // initializeStateForNewResources(negotiation, resources, updateResourcesDTO.getState()); + persistChanges(negotiation, resources, updateResourcesDTO.getState()); } private @NonNull List getResourceWithStatusDTOS(String negotiationId) { @@ -129,9 +129,11 @@ private void updateResources(String negotiationId, UpdateResourcesDTO updateReso .toList(); } - private void persistChanges(Negotiation negotiation, Set resources) { + private void persistChanges( + Negotiation negotiation, Set resources, NegotiationResourceState state) { resources.addAll(negotiation.getResources()); negotiation.setResources(resources); + initializeStateForNewResources(negotiation, resources, state); negotiationRepository.saveAndFlush(negotiation); if (negotiation.getCurrentState().equals(NegotiationState.IN_PROGRESS)) { applicationEventPublisher.publishEvent(new NewResourcesAddedEvent(this, negotiation.getId())); diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index ff8d7a982..e8e262445 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -1,6 +1,5 @@ package eu.bbmri_eric.negotiator.negotiation; -import com.fasterxml.jackson.annotation.JsonIgnore; import com.vladmihalcea.hibernate.type.json.JsonType; import eu.bbmri_eric.negotiator.attachment.Attachment; import eu.bbmri_eric.negotiator.common.AuditEntity; @@ -25,7 +24,6 @@ import jakarta.persistence.ManyToOne; import jakarta.persistence.MapKeyColumn; import jakarta.persistence.OneToMany; -import jakarta.persistence.Table; import jakarta.validation.constraints.NotNull; import java.util.HashMap; import java.util.HashSet; @@ -39,19 +37,18 @@ import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; -import lombok.ToString.Exclude; import org.hibernate.annotations.Formula; import org.hibernate.annotations.JdbcTypeCode; import org.hibernate.annotations.UuidGenerator; import org.hibernate.type.SqlTypes; +/** A Core Entity representing a Negotiation between multiple parties about access to resources. */ @Entity @NoArgsConstructor @AllArgsConstructor @Getter @Setter @Builder -@Table(name = "negotiation") @Convert(converter = JsonType.class, attributeName = "json") public class Negotiation extends AuditEntity { @@ -71,7 +68,6 @@ public class Negotiation extends AuditEntity { private String humanReadable = ""; @OneToMany(mappedBy = "id.negotiation", fetch = FetchType.EAGER, cascade = CascadeType.ALL) - @Exclude @NotNull @Builder.Default private Set resourcesLink = new HashSet<>(); @@ -117,9 +113,7 @@ public class Negotiation extends AuditEntity { @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "discovery_service_id") - @JsonIgnore @NotNull - @Exclude private DiscoveryService discoveryService; private static Set creteInitialHistory() { @@ -135,6 +129,11 @@ public void setCurrentState(NegotiationState negotiationState) { public void setStateForResource(String resourceId, NegotiationResourceState state) { currentStatePerResource.put(resourceId, state); + this.resourcesLink.stream() + .filter(link -> link.getResource().getSourceId().equals(resourceId)) + .findFirst() + .orElseThrow(IllegalArgumentException::new) + .setCurrentState(state); if (!state.equals(NegotiationResourceState.SUBMITTED)) { NegotiationResourceLifecycleRecord record = NegotiationResourceLifecycleRecord.builder() @@ -155,6 +154,23 @@ public Set getResources() { .collect(Collectors.toSet()); } + private Resource lookupResource(Set resources, String resourceId) { + return resources.stream() + .filter(r -> r.getSourceId().equals(resourceId)) + .findFirst() + .orElse(null); + } + + public void setResources(Set resources) { + this.resourcesLink.clear(); + resources.forEach( + resource -> this.resourcesLink.add(new NegotiationResourceLink(this, resource, null))); + } + + public void addResource(Resource resource) { + this.resourcesLink.add(new NegotiationResourceLink(this, resource, null)); + } + @Override public boolean equals(Object o) { if (this == o) { @@ -172,24 +188,6 @@ public int hashCode() { return Objects.hash(getId()); } - @Override - public String toString() { - return "Negotiation{" + "id='" + id + '\'' + '}'; - } - - private Resource lookupResource(Set resources, String resourceId) { - return resources.stream() - .filter(r -> r.getSourceId().equals(resourceId)) - .findFirst() - .orElse(null); - } - - public void setResources(Set resources) { - this.resourcesLink.clear(); - resources.forEach( - resource -> this.resourcesLink.add(new NegotiationResourceLink(this, resource, null))); - } - public static class NegotiationBuilder { public NegotiationBuilder resources(Set resources) { return this; diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java index 2738115c1..b3782f727 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; import eu.bbmri_eric.negotiator.governance.resource.Resource; import eu.bbmri_eric.negotiator.negotiation.Negotiation; @@ -61,12 +62,23 @@ void getCurrentStatesPerResource_defaultConstructor_isNull() { } @Test - void setResourcesStates_oneResource_Ok() { + void setResourceState_resourceNotLinked_throwsIllegalArg() { Negotiation negotiation = Negotiation.builder().build(); - negotiation.setStateForResource("collection:1", NegotiationResourceState.SUBMITTED); + assertThrows( + IllegalArgumentException.class, + () -> negotiation.setStateForResource("collection:1", NegotiationResourceState.SUBMITTED)); + } + + @Test + void setResourceState_resourceLinked_ok() { + Negotiation negotiation = Negotiation.builder().build(); + Resource resource = new Resource(); + resource.setSourceId("fancyId"); + negotiation.addResource(resource); + negotiation.setStateForResource("fancyId", NegotiationResourceState.SUBMITTED); assertEquals( NegotiationResourceState.SUBMITTED, - negotiation.getCurrentStatePerResource().get("collection:1")); + negotiation.getCurrentStatePerResource().get("fancyId")); } @Test From d7025e18b9d5f16bc8cde8143dca9f0bf4cca142 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Thu, 26 Sep 2024 11:11:28 +0200 Subject: [PATCH 20/29] wip: move resource status to resource links Signed-off-by: RadovanTomik --- .../NonRepresentedResourcesHandlerImpl.java | 10 ++-- .../resource/ResourceRepository.java | 5 +- .../negotiator/negotiation/Negotiation.java | 25 ++++------ .../negotiation/NegotiationRepository.java | 10 ++-- .../negotiation/NegotiationResourceLink.java | 3 ++ .../mappers/NegotiationModelMapper.java | 37 --------------- .../UserNotificationServiceImpl.java | 10 ++-- ...rces_and_human_readable_to_negotiation.sql | 46 +++++++++++-------- .../db/test/migration/R__Initial_data.sql | 21 ++++----- .../api/v3/NegotiationControllerTests.java | 2 +- .../handler/ResourcesHandlerTest.java | 5 +- .../NegotiationLifecycleServiceImplTest.java | 22 ++++----- .../service/UserNotificationServiceTest.java | 5 +- .../unit/model/NegotiationTest.java | 9 +--- 14 files changed, 80 insertions(+), 130 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/NonRepresentedResourcesHandlerImpl.java b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/NonRepresentedResourcesHandlerImpl.java index 5724740ef..05e56f241 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/NonRepresentedResourcesHandlerImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/NonRepresentedResourcesHandlerImpl.java @@ -30,9 +30,13 @@ public NonRepresentedResourcesHandlerImpl( public void updateResourceInOngoingNegotiations(Long resourceId, String sourceId) { for (Negotiation negotiation : negotiationRepository.findAllByCurrentState(NegotiationState.IN_PROGRESS)) { - if (Objects.equals( - negotiation.getCurrentStatePerResource().get(sourceId), - NegotiationResourceState.REPRESENTATIVE_UNREACHABLE)) { + NegotiationResourceState state = null; + try { + state = negotiation.getCurrentStateForResource(sourceId); + } catch (IllegalArgumentException e) { + continue; + } + if (Objects.equals(state, NegotiationResourceState.REPRESENTATIVE_UNREACHABLE)) { updateResourceStatus(negotiation.getId(), sourceId); } } diff --git a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java index 2941731e2..874696532 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceRepository.java @@ -23,14 +23,13 @@ public interface ResourceRepository nrl.negotiation_id as negotiationId, rs.name as name, rs.source_id as sourceId, - rspn.current_state as currentState, + nrl.current_state as currentState, o.name as organizationName, o.external_id as organizationExternalId, o.id as organizationId from resource rs - join public.negotiation_resource_link nrl on rs.id = nrl.resource_id + left join public.negotiation_resource_link nrl on rs.id = nrl.resource_id join public.organization o on o.id = rs.organization_id - left join public.resource_state_per_negotiation rspn on rs.source_id = rspn.resource_id and nrl.negotiation_id = rspn.negotiation_id where nrl.negotiation_id = :negotiationId order by rs.source_id; diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index e8e262445..0770ac331 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -10,10 +10,8 @@ import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceLifecycleRecord; import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; import jakarta.persistence.CascadeType; -import jakarta.persistence.CollectionTable; import jakarta.persistence.Column; import jakarta.persistence.Convert; -import jakarta.persistence.ElementCollection; import jakarta.persistence.Entity; import jakarta.persistence.EnumType; import jakarta.persistence.Enumerated; @@ -22,12 +20,9 @@ import jakarta.persistence.Id; import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; -import jakarta.persistence.MapKeyColumn; import jakarta.persistence.OneToMany; import jakarta.validation.constraints.NotNull; -import java.util.HashMap; import java.util.HashSet; -import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -87,17 +82,6 @@ public class Negotiation extends AuditEntity { @Enumerated(EnumType.STRING) private NegotiationState currentState = NegotiationState.SUBMITTED; - @ElementCollection - @CollectionTable( - name = "resource_state_per_negotiation", - joinColumns = {@JoinColumn(name = "negotiation_id", referencedColumnName = "id")}) - @MapKeyColumn(name = "resource_id") - @Enumerated(EnumType.STRING) - @Column(name = "current_state") - @Setter(AccessLevel.NONE) - @Builder.Default - private Map currentStatePerResource = new HashMap<>(); - @OneToMany(cascade = {CascadeType.ALL}) @JoinColumn(name = "negotiation_id", referencedColumnName = "id") @Setter(AccessLevel.NONE) @@ -127,8 +111,15 @@ public void setCurrentState(NegotiationState negotiationState) { this.lifecycleHistory.add(NegotiationLifecycleRecord.builder().changedTo(currentState).build()); } + public NegotiationResourceState getCurrentStateForResource(String resourceId) { + return this.resourcesLink.stream() + .filter(link -> link.getResource().getSourceId().equals(resourceId)) + .findFirst() + .orElseThrow(IllegalArgumentException::new) + .getCurrentState(); + } + public void setStateForResource(String resourceId, NegotiationResourceState state) { - currentStatePerResource.put(resourceId, state); this.resourcesLink.stream() .filter(link -> link.getResource().getSourceId().equals(resourceId)) .findFirst() diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java index 8e4d83e9e..ad8f72bef 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java @@ -24,10 +24,12 @@ public interface NegotiationRepository List findAllByCurrentState(NegotiationState state); @Query( - "SELECT n.currentStatePerResource " - + "FROM Negotiation n " - + "JOIN n.currentStatePerResource currentState " - + "WHERE n.id = :negotiationId AND KEY(currentState) = :resourceId") + """ + SELECT rl.currentState + FROM Negotiation n + JOIN n.resourcesLink rl + JOIN rl.id.resource + WHERE n.id = :negotiationId AND rl.id.resource.sourceId = :resourceId""") Optional findNegotiationResourceStateById( String negotiationId, String resourceId); diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java index 58ab50382..6d8e9d1d6 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationResourceLink.java @@ -4,6 +4,8 @@ import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; import jakarta.persistence.EmbeddedId; import jakarta.persistence.Entity; +import jakarta.persistence.EnumType; +import jakarta.persistence.Enumerated; import lombok.AccessLevel; import lombok.AllArgsConstructor; import lombok.Getter; @@ -18,6 +20,7 @@ public class NegotiationResourceLink { @EmbeddedId private NegotiationResourceLinkId id; + @Enumerated(EnumType.STRING) private NegotiationResourceState currentState; public NegotiationResourceLink( diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java index 7d55074a5..69f7434f8 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/mappers/NegotiationModelMapper.java @@ -3,18 +3,11 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import eu.bbmri_eric.negotiator.governance.organization.OrganizationDTO; -import eu.bbmri_eric.negotiator.governance.resource.Resource; -import eu.bbmri_eric.negotiator.governance.resource.dto.ResourceWithStatusDTO; import eu.bbmri_eric.negotiator.negotiation.Negotiation; import eu.bbmri_eric.negotiator.negotiation.dto.NegotiationDTO; import eu.bbmri_eric.negotiator.negotiation.state_machine.negotiation.NegotiationState; -import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; import jakarta.annotation.PostConstruct; -import java.util.Map; import java.util.Objects; -import java.util.Set; -import java.util.stream.Collectors; import lombok.extern.apachecommons.CommonsLog; import org.modelmapper.Converter; import org.modelmapper.ModelMapper; @@ -40,9 +33,6 @@ public void addMappings() { Converter negotiationStatusConverter = status -> negotiationStatusConverter(status.getSource()); - Converter> resourcesConverter = - negotiation -> resourceConverter(negotiation.getSource()); - Converter payloadConverter = p -> { try { @@ -68,33 +58,6 @@ public void addMappings() { .map(Negotiation::getCurrentState, NegotiationDTO::setStatus)); } - private Set resourceConverter(Negotiation negotiation) { - final Map statePerResource = - negotiation.getCurrentStatePerResource(); - - return negotiation.getResources().stream() - .map(resource -> buildResourceWithStatus(resource, statePerResource, negotiation.getId())) - .collect(Collectors.toSet()); - } - - private ResourceWithStatusDTO buildResourceWithStatus( - Resource resource, - Map statePerResource, - String negotiationId) { - ResourceWithStatusDTO.ResourceWithStatusDTOBuilder builder = - ResourceWithStatusDTO.builder() - .id(resource.getId()) - .sourceId(resource.getSourceId()) - .negotiationId(negotiationId) - .name(resource.getName()) - .organization(modelMapper.map(resource.getOrganization(), OrganizationDTO.class)); - NegotiationResourceState state = statePerResource.get(resource.getSourceId()); - if (state != null) { - builder.currentState(state); - } - return builder.build(); - } - private JsonNode payloadConverter(String jsonPayload) throws JsonProcessingException { ObjectMapper mapper = new ObjectMapper(); if (jsonPayload == null) { diff --git a/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java index 52bb7aebc..58af354de 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/notification/UserNotificationServiceImpl.java @@ -100,11 +100,9 @@ private static Set getRepresentativesForNegotiation(Negotiation negotiat return negotiation.getResources().stream() .filter( resource -> - negotiation - .getCurrentStatePerResource() - .getOrDefault( - resource.getSourceId(), NegotiationResourceState.REPRESENTATIVE_CONTACTED) - .equals(NegotiationResourceState.SUBMITTED)) + Objects.equals( + negotiation.getCurrentStateForResource(resource.getSourceId()), + NegotiationResourceState.SUBMITTED)) .map(Resource::getRepresentatives) .flatMap(Set::stream) .collect(Collectors.toSet()); @@ -198,7 +196,7 @@ public void notifyRequesterAboutStatusChange(Negotiation negotiation, Resource r .formatted( negotiation.getId(), resource.getSourceId(), - negotiation.getCurrentStatePerResource().get(resource.getSourceId()))) + negotiation.getCurrentStateForResource(resource.getSourceId()))) .build()); } diff --git a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql index b02fae3e9..9ec3574a6 100644 --- a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql @@ -1,28 +1,36 @@ alter table negotiation -add column human_readable text, -add column discovery_service_id bigint, -add constraint fkey_discovery_service_id FOREIGN KEY (discovery_service_id) REFERENCES discovery_service(id); + add column human_readable text, + add column discovery_service_id bigint, + add constraint fkey_discovery_service_id FOREIGN KEY (discovery_service_id) REFERENCES discovery_service (id); -create table negotiation_resource_link ( - negotiation_id character varying(255) not null references negotiation(id), - resource_id bigint not null references resource(id), - current_state varchar(255), +create table negotiation_resource_link +( + negotiation_id character varying(255) not null references negotiation (id), + resource_id bigint not null references resource (id), + current_state varchar(255), primary key (negotiation_id, resource_id) ); update negotiation -set (human_readable, discovery_service_id) = ( - select human_readable, discovery_service_id - from request - where negotiation.id = request.negotiation_id -); - -alter table negotiation alter column human_readable set not null; -alter table negotiation alter column discovery_service_id set not null; +set (human_readable, discovery_service_id) = (select human_readable, discovery_service_id + from request + where negotiation.id = request.negotiation_id); -insert into negotiation_resource_link (negotiation_id, resource_id) -select r.negotiation_id, rrl.resource_id from request_resources_link rrl join request r on rrl.request_id = r.id; +alter table negotiation + alter column human_readable set not null; +alter table negotiation + alter column discovery_service_id set not null; -delete from request where negotiation_id is null; -alter table request drop column negotiation_id; +insert into negotiation_resource_link (negotiation_id, resource_id, current_state) +select r.negotiation_id, rrl.resource_id, rspn.current_state +from request_resources_link rrl + join request r on rrl.request_id = r.id + join public.resource r2 on r2.id = rrl.resource_id + join public.resource_state_per_negotiation rspn on r2.source_id = rspn.resource_id; +delete +from request +where negotiation_id is null; +alter table request + drop column negotiation_id; +drop table resource_state_per_negotiation; \ No newline at end of file diff --git a/src/main/resources/db/test/migration/R__Initial_data.sql b/src/main/resources/db/test/migration/R__Initial_data.sql index f43bd022f..c2cf7150d 100644 --- a/src/main/resources/db/test/migration/R__Initial_data.sql +++ b/src/main/resources/db/test/migration/R__Initial_data.sql @@ -103,19 +103,14 @@ values ('request-1', 4), ('request-5', 5), ('request-5', 7); -insert into negotiation_resource_link (negotiation_id, resource_id) -values ('negotiation-1', 4), - ('negotiation-v2', 7), - ('negotiation-3', 5), - ('negotiation-4', 5), - ('negotiation-4', 7), - ('negotiation-5', 5), - ('negotiation-5', 7); - -insert into resource_state_per_negotiation (negotiation_id, resource_id, current_state) -values ('negotiation-1', 'biobank:1:collection:1', 'SUBMITTED'), - ('negotiation-v2', 'biobank:3:collection:1', 'SUBMITTED'), - ('negotiation-3', 'biobank:1:collection:2', 'RESOURCE_UNAVAILABLE'); +insert into negotiation_resource_link (negotiation_id, resource_id, current_state) +values ('negotiation-1', 4, 'SUBMITTED'), + ('negotiation-v2', 7, 'SUBMITTED'), + ('negotiation-3', 5, 'RESOURCE_UNAVAILABLE'), + ('negotiation-4', 5, null), + ('negotiation-4', 7, null), + ('negotiation-5', 5, null), + ('negotiation-5', 7, null); insert into post (id, creation_date, modified_date, text, created_by, modified_by, negotiation_id, organization_id, type) diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java index fbb269f43..e8daf6a6f 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/api/v3/NegotiationControllerTests.java @@ -1372,7 +1372,7 @@ void addResources_resourcesAlreadyPresent_noChange() throws Exception { JsonNode resourcesAsJson = response.get("_embedded").get("resources"); for (JsonNode resourceAsJson : resourcesAsJson) { assertEquals( - negotiation.getCurrentStatePerResource().get(resourceAsJson.get("sourceId").asText()), + negotiation.getCurrentStateForResource(resourceAsJson.get("sourceId").asText()), NegotiationResourceState.valueOf(resourceAsJson.get("currentState").asText())); } } diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/handler/ResourcesHandlerTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/handler/ResourcesHandlerTest.java index acd4d4398..f8c422e8b 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/handler/ResourcesHandlerTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/handler/ResourcesHandlerTest.java @@ -25,6 +25,7 @@ public class ResourcesHandlerTest { @Transactional void updateState_1negotiation1Resource_updated() { Negotiation negotiation = negotiationRepository.findAll().iterator().next(); + assertEquals("negotiation-1", negotiation.getId()); assertEquals(NegotiationState.IN_PROGRESS, negotiation.getCurrentState()); Resource resource = negotiation.getResources().iterator().next(); negotiation.setStateForResource( @@ -32,7 +33,7 @@ void updateState_1negotiation1Resource_updated() { handler.updateResourceInOngoingNegotiations(resource.getId(), resource.getSourceId()); assertEquals( NegotiationResourceState.REPRESENTATIVE_CONTACTED, - negotiation.getCurrentStatePerResource().get(resource.getSourceId())); + negotiation.getCurrentStateForResource(resource.getSourceId())); } @Test @@ -47,7 +48,7 @@ void updateState_abandonedNegotiation_noChange() { handler.updateResourceInOngoingNegotiations(resource.getId(), resource.getSourceId()); assertEquals( NegotiationResourceState.REPRESENTATIVE_UNREACHABLE, - negotiation.getCurrentStatePerResource().get(resource.getSourceId())); + negotiation.getCurrentStateForResource(resource.getSourceId())); } @Test diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java index 2cb6defce..ec847dcf8 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/NegotiationLifecycleServiceImplTest.java @@ -41,7 +41,6 @@ import jakarta.transaction.Transactional; import java.io.IOException; import java.util.HashSet; -import java.util.Map; import java.util.Objects; import java.util.Set; import org.junit.jupiter.api.Assertions; @@ -210,23 +209,18 @@ void sendEvent_approveCorrectly_historyIsUpdated() throws IOException { .anyMatch(record -> record.getChangedTo().equals(NegotiationState.IN_PROGRESS))); } - @Test - void createNegotiation_statePerResource_isEmpty() throws IOException { - NegotiationDTO negotiationDTO = saveNegotiation(); - Map statePerResource = - negotiationRepository.findById(negotiationDTO.getId()).get().getCurrentStatePerResource(); - assertTrue(statePerResource.isEmpty()); - } - @Test @WithMockUser(roles = "ADMIN") void createNegotiation_approve_eachResourceHasState() throws IOException { NegotiationDTO negotiationDTO = saveNegotiation(); negotiationLifecycleService.sendEvent(negotiationDTO.getId(), NegotiationEvent.APPROVE); - Map states = - negotiationRepository.findById(negotiationDTO.getId()).get().getCurrentStatePerResource(); - assertTrue(states.containsKey("biobank:1:collection:2")); - assertNotEquals(NegotiationResourceState.SUBMITTED, states.get("biobank:1:collection:2")); + NegotiationResourceState state = + negotiationRepository + .findById(negotiationDTO.getId()) + .get() + .getCurrentStateForResource("biobank:1:collection:2"); + Assertions.assertNotNull(state); + assertNotEquals(NegotiationResourceState.SUBMITTED, state); } @Test @@ -301,7 +295,7 @@ void sendEventForResource_approvedNegotiationMultipleCorrectEvents_ok() throws I void sendEventForResource_notAuthorized_noChange() { Negotiation negotiation = negotiationRepository.findById("negotiation-1").get(); assertEquals( - negotiation.getCurrentStatePerResource().get("biobank:1:collection:1"), + negotiation.getCurrentStateForResource("biobank:1:collection:1"), resourceLifecycleService.sendEvent( negotiation.getId(), "biobank:1:collection:1", diff --git a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java index ad873c7a3..d2a4a7068 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/integration/service/UserNotificationServiceTest.java @@ -149,7 +149,7 @@ void notifyRepresentatives_sameRepFor2Resources_oneNotification() { Negotiation updatedNegotiation = negotiationRepository.findById(negotiation.getId()).get(); assertEquals( NegotiationResourceState.REPRESENTATIVE_CONTACTED, - updatedNegotiation.getCurrentStatePerResource().get("biobank:1:collection:2")); + updatedNegotiation.getCurrentStateForResource("biobank:1:collection:2")); } @Test @@ -174,8 +174,7 @@ void notifyRepresentatives_resWithNoRep_markedAsUnreachable() { negotiationRepository .findById(negotiation.getId()) .get() - .getCurrentStatePerResource() - .get(resourceWithoutReps.getSourceId())); + .getCurrentStateForResource(resourceWithoutReps.getSourceId())); } @Test diff --git a/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java b/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java index b3782f727..b804f3147 100644 --- a/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java +++ b/src/test/java/eu/bbmri_eric/negotiator/unit/model/NegotiationTest.java @@ -10,7 +10,6 @@ import eu.bbmri_eric.negotiator.negotiation.state_machine.resource.NegotiationResourceState; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import org.junit.jupiter.api.Test; @@ -56,11 +55,6 @@ void setCurrentState_Ok() { assertEquals(NegotiationState.APPROVED, negotiation.getCurrentState()); } - @Test - void getCurrentStatesPerResource_defaultConstructor_isNull() { - assertEquals(Map.of(), new Negotiation().getCurrentStatePerResource()); - } - @Test void setResourceState_resourceNotLinked_throwsIllegalArg() { Negotiation negotiation = Negotiation.builder().build(); @@ -77,8 +71,7 @@ void setResourceState_resourceLinked_ok() { negotiation.addResource(resource); negotiation.setStateForResource("fancyId", NegotiationResourceState.SUBMITTED); assertEquals( - NegotiationResourceState.SUBMITTED, - negotiation.getCurrentStatePerResource().get("fancyId")); + NegotiationResourceState.SUBMITTED, negotiation.getCurrentStateForResource(("fancyId"))); } @Test From 917962b9684e5f05b268a477ba09164c7b7bb69d Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Thu, 26 Sep 2024 11:32:02 +0200 Subject: [PATCH 21/29] style: minor changes --- .../NonRepresentedResourcesHandlerImpl.java | 2 +- .../negotiator/negotiation/Negotiation.java | 3 +-- .../negotiation/NegotiationRepository.java | 14 ++++++++------ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/NonRepresentedResourcesHandlerImpl.java b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/NonRepresentedResourcesHandlerImpl.java index 05e56f241..1ca1402dc 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/NonRepresentedResourcesHandlerImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/NonRepresentedResourcesHandlerImpl.java @@ -30,7 +30,7 @@ public NonRepresentedResourcesHandlerImpl( public void updateResourceInOngoingNegotiations(Long resourceId, String sourceId) { for (Negotiation negotiation : negotiationRepository.findAllByCurrentState(NegotiationState.IN_PROGRESS)) { - NegotiationResourceState state = null; + NegotiationResourceState state; try { state = negotiation.getCurrentStateForResource(sourceId); } catch (IllegalArgumentException e) { diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index 0770ac331..d6e538c1a 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -154,8 +154,7 @@ private Resource lookupResource(Set resources, String resourceId) { public void setResources(Set resources) { this.resourcesLink.clear(); - resources.forEach( - resource -> this.resourcesLink.add(new NegotiationResourceLink(this, resource, null))); + resources.forEach(this::addResource); } public void addResource(Resource resource) { diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java index ad8f72bef..f87042a86 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationRepository.java @@ -24,12 +24,14 @@ public interface NegotiationRepository List findAllByCurrentState(NegotiationState state); @Query( - """ - SELECT rl.currentState - FROM Negotiation n - JOIN n.resourcesLink rl - JOIN rl.id.resource - WHERE n.id = :negotiationId AND rl.id.resource.sourceId = :resourceId""") + value = + """ + SELECT rl.currentState + FROM Negotiation n + JOIN n.resourcesLink rl + JOIN rl.id.resource + WHERE n.id = :negotiationId AND rl.id.resource.sourceId = :resourceId + """) Optional findNegotiationResourceStateById( String negotiationId, String resourceId); From 63c582eefe19178829f1b9a8a0d50a00f494e024 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Thu, 26 Sep 2024 11:42:41 +0200 Subject: [PATCH 22/29] wip: move resource status to resource links Signed-off-by: RadovanTomik --- .../negotiator/negotiation/Negotiation.java | 75 +++++++++++-------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index d6e538c1a..240c3f704 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -62,7 +62,9 @@ public class Negotiation extends AuditEntity { @Column(columnDefinition = "TEXT") private String humanReadable = ""; - @OneToMany(mappedBy = "id.negotiation", fetch = FetchType.EAGER, cascade = CascadeType.ALL) + @OneToMany( + mappedBy = "id.negotiation", + cascade = {CascadeType.PERSIST, CascadeType.REMOVE, CascadeType.MERGE}) @NotNull @Builder.Default private Set resourcesLink = new HashSet<>(); @@ -82,13 +84,13 @@ public class Negotiation extends AuditEntity { @Enumerated(EnumType.STRING) private NegotiationState currentState = NegotiationState.SUBMITTED; - @OneToMany(cascade = {CascadeType.ALL}) + @OneToMany(cascade = {CascadeType.PERSIST, CascadeType.REMOVE, CascadeType.MERGE}) @JoinColumn(name = "negotiation_id", referencedColumnName = "id") @Setter(AccessLevel.NONE) @Builder.Default private Set lifecycleHistory = creteInitialHistory(); - @OneToMany(cascade = {CascadeType.ALL}) + @OneToMany(cascade = {CascadeType.PERSIST, CascadeType.REMOVE, CascadeType.MERGE}) @JoinColumn(name = "negotiation_id", referencedColumnName = "id") @Setter(AccessLevel.NONE) @Builder.Default @@ -100,12 +102,6 @@ public class Negotiation extends AuditEntity { @NotNull private DiscoveryService discoveryService; - private static Set creteInitialHistory() { - Set history = new HashSet<>(); - history.add(NegotiationLifecycleRecord.builder().changedTo(NegotiationState.SUBMITTED).build()); - return history; - } - public void setCurrentState(NegotiationState negotiationState) { this.currentState = negotiationState; this.lifecycleHistory.add(NegotiationLifecycleRecord.builder().changedTo(currentState).build()); @@ -120,23 +116,13 @@ public NegotiationResourceState getCurrentStateForResource(String resourceId) { } public void setStateForResource(String resourceId, NegotiationResourceState state) { - this.resourcesLink.stream() - .filter(link -> link.getResource().getSourceId().equals(resourceId)) - .findFirst() - .orElseThrow(IllegalArgumentException::new) - .setCurrentState(state); - if (!state.equals(NegotiationResourceState.SUBMITTED)) { - NegotiationResourceLifecycleRecord record = - NegotiationResourceLifecycleRecord.builder() - .changedTo(state) - .resource(lookupResource(getResources(), resourceId)) - .build(); - this.negotiationResourceLifecycleRecords.add(record); - } - } - - public static CustomNegotiationBuilder builder() { - return new CustomNegotiationBuilder(); + NegotiationResourceLink link = + this.resourcesLink.stream() + .filter(resourceLink -> resourceLink.getResource().getSourceId().equals(resourceId)) + .findFirst() + .orElseThrow(IllegalArgumentException::new); + link.setCurrentState(state); + buildResourceStateChangeRecord(link.getResource(), state); } public Set getResources() { @@ -145,22 +131,45 @@ public Set getResources() { .collect(Collectors.toSet()); } - private Resource lookupResource(Set resources, String resourceId) { - return resources.stream() - .filter(r -> r.getSourceId().equals(resourceId)) - .findFirst() - .orElse(null); - } - + /** + * Set all the resources linked to the Negotiation. This operation will also clear their + * state.Consider using {@link + * #addResource(eu.bbmri_eric.negotiator.governance.resource.Resource)} + * + * @param resources to be linked. + */ public void setResources(Set resources) { this.resourcesLink.clear(); resources.forEach(this::addResource); } + /** + * Link a resource to the Negotiation with null state. + * + * @param resource to be linked. + */ public void addResource(Resource resource) { this.resourcesLink.add(new NegotiationResourceLink(this, resource, null)); } + private void buildResourceStateChangeRecord(Resource resource, NegotiationResourceState state) { + if (!state.equals(NegotiationResourceState.SUBMITTED)) { + NegotiationResourceLifecycleRecord record = + NegotiationResourceLifecycleRecord.builder().changedTo(state).resource(resource).build(); + this.negotiationResourceLifecycleRecords.add(record); + } + } + + private static Set creteInitialHistory() { + Set history = new HashSet<>(); + history.add(NegotiationLifecycleRecord.builder().changedTo(NegotiationState.SUBMITTED).build()); + return history; + } + + public static CustomNegotiationBuilder builder() { + return new CustomNegotiationBuilder(); + } + @Override public boolean equals(Object o) { if (this == o) { From 975422b33a55e0394910e704f8c00e4118c5ff70 Mon Sep 17 00:00:00 2001 From: Vittorio Meloni Date: Thu, 26 Sep 2024 13:47:13 +0200 Subject: [PATCH 23/29] refactor: remove not null from negotiatio human readable --- .../eu/bbmri_eric/negotiator/negotiation/Negotiation.java | 1 - ...6.0__move_resources_and_human_readable_to_negotiation.sql | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index 240c3f704..0afd87005 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -58,7 +58,6 @@ public class Negotiation extends AuditEntity { cascade = {CascadeType.MERGE}) private Set attachments; - @NotNull @Column(columnDefinition = "TEXT") private String humanReadable = ""; diff --git a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql index 9ec3574a6..fe7a0cfa3 100644 --- a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql @@ -16,10 +16,7 @@ set (human_readable, discovery_service_id) = (select human_readable, discovery_s from request where negotiation.id = request.negotiation_id); -alter table negotiation - alter column human_readable set not null; -alter table negotiation - alter column discovery_service_id set not null; +alter table negotiation alter column discovery_service_id set not null; insert into negotiation_resource_link (negotiation_id, resource_id, current_state) select r.negotiation_id, rrl.resource_id, rspn.current_state From 9be77e62e86099d068eb9f11981df156c7765d04 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Thu, 26 Sep 2024 17:05:46 +0200 Subject: [PATCH 24/29] wip: move resource status to resource links Signed-off-by: RadovanTomik --- ...rces_and_human_readable_to_negotiation.sql | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql index fe7a0cfa3..77fdae132 100644 --- a/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql +++ b/src/main/resources/db/migration/V16.0__move_resources_and_human_readable_to_negotiation.sql @@ -15,19 +15,27 @@ update negotiation set (human_readable, discovery_service_id) = (select human_readable, discovery_service_id from request where negotiation.id = request.negotiation_id); +update negotiation +set discovery_service_id = 1 +where negotiation.discovery_service_id is null; +alter table negotiation + alter column discovery_service_id set not null; +INSERT INTO negotiation_resource_link (negotiation_id, resource_id, current_state) +SELECT n.id, r.id, rspn.current_state +FROM resource_state_per_negotiation rspn + JOIN public.resource r ON r.source_id = rspn.resource_id + JOIN public.negotiation n ON n.id = rspn.negotiation_id + JOIN public.request r2 ON n.id = r2.negotiation_id; +INSERT INTO negotiation_resource_link (negotiation_id, resource_id, current_state) +SELECT n.id, r2.id, NULL +FROM negotiation n + JOIN public.request r ON n.id = r.negotiation_id + JOIN public.request_resources_link rrl ON r.id = rrl.request_id + JOIN public.resource r2 ON r2.id = rrl.resource_id +where n.current_state = 'SUBMITTED' + or n.current_state = 'DECLINED'; -alter table negotiation alter column discovery_service_id set not null; - -insert into negotiation_resource_link (negotiation_id, resource_id, current_state) -select r.negotiation_id, rrl.resource_id, rspn.current_state -from request_resources_link rrl - join request r on rrl.request_id = r.id - join public.resource r2 on r2.id = rrl.resource_id - join public.resource_state_per_negotiation rspn on r2.source_id = rspn.resource_id; -delete -from request -where negotiation_id is null; alter table request drop column negotiation_id; drop table resource_state_per_negotiation; \ No newline at end of file From a550e882ea4e377209908b168bb508ab5c357d13 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Thu, 26 Sep 2024 22:06:01 +0200 Subject: [PATCH 25/29] refactor: NegotiationServiceImpl Signed-off-by: RadovanTomik --- .../negotiator/negotiation/Negotiation.java | 10 +++- .../negotiation/NegotiationServiceImpl.java | 49 ++++++++++--------- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index 0afd87005..b2dc978ac 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -56,7 +56,8 @@ public class Negotiation extends AuditEntity { @OneToMany( mappedBy = "negotiation", cascade = {CascadeType.MERGE}) - private Set attachments; + @Builder.Default + private Set attachments = new HashSet<>(); @Column(columnDefinition = "TEXT") private String humanReadable = ""; @@ -83,6 +84,13 @@ public class Negotiation extends AuditEntity { @Enumerated(EnumType.STRING) private NegotiationState currentState = NegotiationState.SUBMITTED; + public void setAttachments(Set attachments) { + if (attachments != null) { + attachments.forEach(attachment -> attachment.setNegotiation(this)); + this.attachments = attachments; + } + } + @OneToMany(cascade = {CascadeType.PERSIST, CascadeType.REMOVE, CascadeType.MERGE}) @JoinColumn(name = "negotiation_id", referencedColumnName = "id") @Setter(AccessLevel.NONE) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java index 1885f66d3..09dc677fa 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java @@ -90,9 +90,9 @@ public boolean isOrganizationPartOfNegotiation( negotiationId, organizationExternalId); } - private Request findRequests(String requestsId) { + private Request getRequest(String requestId) { return requestRepository - .findById(requestsId) + .findById(requestId) .orElseThrow( () -> new WrongRequestException("One or more of the specified requests do not exist")); } @@ -113,7 +113,7 @@ public boolean exists(String negotiationId) { } /** - * Creates a Negotiation into the repository. + * Creates a Negotiation. * * @param negotiationBody the NegotiationCreateDTO DTO sent from to the endpoint * @param creatorId the ID of the Person that creates the Negotiation (i.e., the authenticated @@ -121,36 +121,39 @@ public boolean exists(String negotiationId) { * @return the created Negotiation entity */ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorId) { - log.debug("Getting request entities"); - Request request = findRequests(negotiationBody.getRequest()); - - Negotiation negotiation = modelMapper.map(negotiationBody, Negotiation.class); - negotiation.setResources(new HashSet<>(request.getResources())); - negotiation.setHumanReadable(request.getHumanReadable()); - negotiation.setDiscoveryService(request.getDiscoveryService()); + Request request = getRequest(negotiationBody.getRequest()); + Negotiation negotiation = buildNegotiation(negotiationBody, request); + negotiation = persistNegotiation(negotiationBody, negotiation); + requestRepository.delete(request); + userNotificationService.notifyAdmins(negotiation); + return modelMapper.map(negotiation, NegotiationDTO.class); + } - Negotiation savedNegotiation; + private Negotiation persistNegotiation(NegotiationCreateDTO negotiationBody, Negotiation negotiation) { try { - // Finally, save the negotiation. NB: it also cascades operations for other Requests - savedNegotiation = negotiationRepository.save(negotiation); + negotiation = negotiationRepository.save(negotiation); } catch (DataException | DataIntegrityViolationException ex) { - log.error("Error while saving the Negotiation into db. Some db constraint violated"); - log.error(ex); + log.error("Error while saving the Negotiation into db. Some db constraint violated", ex); throw new EntityNotStorableException(); } - if (negotiationBody.getAttachments() != null) { - List attachments = findAttachments(negotiationBody.getAttachments()); - negotiation.setAttachments(new HashSet<>(attachments)); - attachments.forEach(attachment -> attachment.setNegotiation(negotiation)); + linkAttachments(negotiationBody, negotiation); } eventPublisher.publishEvent(new NewNegotiationEvent(this, negotiation.getId())); + return negotiation; + } - requestRepository.delete(request); + private void linkAttachments(NegotiationCreateDTO negotiationBody, Negotiation negotiation) { + List attachments = findAttachments(negotiationBody.getAttachments()); + negotiation.setAttachments(new HashSet<>(attachments)); + } - // TODO: Add call to send email. - userNotificationService.notifyAdmins(negotiation); - return modelMapper.map(savedNegotiation, NegotiationDTO.class); + private Negotiation buildNegotiation(NegotiationCreateDTO negotiationBody, Request request) { + Negotiation negotiation = modelMapper.map(negotiationBody, Negotiation.class); + negotiation.setResources(new HashSet<>(request.getResources())); + negotiation.setHumanReadable(request.getHumanReadable()); + negotiation.setDiscoveryService(request.getDiscoveryService()); + return negotiation; } private NegotiationDTO update( From 5101e154aabecc0b392b294ec8fd9805383b3b24 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Thu, 26 Sep 2024 22:06:04 +0200 Subject: [PATCH 26/29] refactor: NegotiationServiceImpl Signed-off-by: RadovanTomik --- .../negotiator/negotiation/NegotiationServiceImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java index 09dc677fa..ed729874e 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/NegotiationServiceImpl.java @@ -129,7 +129,8 @@ public NegotiationDTO create(NegotiationCreateDTO negotiationBody, Long creatorI return modelMapper.map(negotiation, NegotiationDTO.class); } - private Negotiation persistNegotiation(NegotiationCreateDTO negotiationBody, Negotiation negotiation) { + private Negotiation persistNegotiation( + NegotiationCreateDTO negotiationBody, Negotiation negotiation) { try { negotiation = negotiationRepository.save(negotiation); } catch (DataException | DataIntegrityViolationException ex) { From 0180c319c8d1e324f2e6791f4bb157ffffacd5d6 Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Thu, 26 Sep 2024 22:08:38 +0200 Subject: [PATCH 27/29] refactor: Negotiation entity Signed-off-by: RadovanTomik --- .../negotiation/CustomNegotiationBuilder.java | 23 +++++++++++++++++++ .../negotiator/negotiation/Negotiation.java | 18 --------------- 2 files changed, 23 insertions(+), 18 deletions(-) create mode 100644 src/main/java/eu/bbmri_eric/negotiator/negotiation/CustomNegotiationBuilder.java diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/CustomNegotiationBuilder.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/CustomNegotiationBuilder.java new file mode 100644 index 000000000..3b25527e0 --- /dev/null +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/CustomNegotiationBuilder.java @@ -0,0 +1,23 @@ +package eu.bbmri_eric.negotiator.negotiation; + +import eu.bbmri_eric.negotiator.governance.resource.Resource; +import java.util.Set; + +public class CustomNegotiationBuilder extends Negotiation.NegotiationBuilder { + private Set resources; + + @Override + public CustomNegotiationBuilder resources(Set resources) { + this.resources = resources; + return this; + } + + @Override + public Negotiation build() { + Negotiation negotiation = super.build(); + if (this.resources != null) { + negotiation.setResources(this.resources); + } + return negotiation; + } +} diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index b2dc978ac..bd1fece85 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -200,22 +200,4 @@ public NegotiationBuilder resources(Set resources) { } } - public static class CustomNegotiationBuilder extends NegotiationBuilder { - private Set resources; - - @Override - public CustomNegotiationBuilder resources(Set resources) { - this.resources = resources; - return this; - } - - @Override - public Negotiation build() { - Negotiation negotiation = super.build(); - if (this.resources != null) { - negotiation.setResources(this.resources); - } - return negotiation; - } - } } From 7dd954c020fce7def2c2b266d51c675c709dbefb Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Thu, 26 Sep 2024 22:14:50 +0200 Subject: [PATCH 28/29] refactor: Negotiation entity Signed-off-by: RadovanTomik --- .../negotiator/negotiation/Negotiation.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java index bd1fece85..135d3d9f8 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java +++ b/src/main/java/eu/bbmri_eric/negotiator/negotiation/Negotiation.java @@ -114,6 +114,11 @@ public void setCurrentState(NegotiationState negotiationState) { this.lifecycleHistory.add(NegotiationLifecycleRecord.builder().changedTo(currentState).build()); } + /** + * Gets the current state for a liked Resource. + * + * @param resourceId the source/external ID of the Resource. Not the internal ID! + */ public NegotiationResourceState getCurrentStateForResource(String resourceId) { return this.resourcesLink.stream() .filter(link -> link.getResource().getSourceId().equals(resourceId)) @@ -122,6 +127,12 @@ public NegotiationResourceState getCurrentStateForResource(String resourceId) { .getCurrentState(); } + /** + * Sets the current state for a liked Resource. + * + * @param resourceId the source/external ID of the Resource. Not the internal ID! + * @param state to be set. + */ public void setStateForResource(String resourceId, NegotiationResourceState state) { NegotiationResourceLink link = this.resourcesLink.stream() @@ -199,5 +210,4 @@ public NegotiationBuilder resources(Set resources) { return this; } } - } From 58a4a0f2ef201e49f9782e41e6372b39a286386c Mon Sep 17 00:00:00 2001 From: RadovanTomik Date: Thu, 26 Sep 2024 22:35:17 +0200 Subject: [PATCH 29/29] fix: resource state on updates Signed-off-by: RadovanTomik --- .../governance/resource/ResourceServiceImpl.java | 1 - .../negotiator/user/NewResourcesListener.java | 16 +++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java index 63c8f7d4f..a89603f8e 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java +++ b/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java @@ -115,7 +115,6 @@ public List updateResourcesInANegotiation( private void updateResources(String negotiationId, UpdateResourcesDTO updateResourcesDTO) { Negotiation negotiation = getNegotiation(negotiationId); Set resources = getResources(negotiationId, updateResourcesDTO, negotiation); - // initializeStateForNewResources(negotiation, resources, updateResourcesDTO.getState()); persistChanges(negotiation, resources, updateResourcesDTO.getState()); } diff --git a/src/main/java/eu/bbmri_eric/negotiator/user/NewResourcesListener.java b/src/main/java/eu/bbmri_eric/negotiator/user/NewResourcesListener.java index ab907f566..6299879ff 100644 --- a/src/main/java/eu/bbmri_eric/negotiator/user/NewResourcesListener.java +++ b/src/main/java/eu/bbmri_eric/negotiator/user/NewResourcesListener.java @@ -2,22 +2,24 @@ import eu.bbmri_eric.negotiator.negotiation.NewResourcesAddedEvent; import eu.bbmri_eric.negotiator.notification.UserNotificationService; -import lombok.NonNull; -import org.springframework.context.ApplicationListener; -import org.springframework.scheduling.annotation.Async; +import jakarta.transaction.Transactional; +import org.springframework.context.event.EventListener; import org.springframework.stereotype.Component; +import org.springframework.transaction.event.TransactionPhase; +import org.springframework.transaction.event.TransactionalEventListener; @Component -public class NewResourcesListener implements ApplicationListener { +public class NewResourcesListener { private final UserNotificationService notificationService; public NewResourcesListener(UserNotificationService notificationService) { this.notificationService = notificationService; } - @Override - @Async - public void onApplicationEvent(@NonNull NewResourcesAddedEvent event) { + @EventListener(value = NewResourcesAddedEvent.class) + @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) + @Transactional(Transactional.TxType.REQUIRES_NEW) + public void onApplicationEvent(NewResourcesAddedEvent event) { notificationService.notifyRepresentativesAboutNewNegotiation(event.getNegotiationId()); } }