From ee9de0c89555cec8135bd33544c71f39fd8d73ac Mon Sep 17 00:00:00 2001 From: Ivo Spijkerman Date: Thu, 23 Oct 2025 20:34:57 +0200 Subject: [PATCH] implement service.upsert --- .../threekidfamily/domain/person/Person.java | 48 +++++- .../domain/person/PersonController.java | 16 +- .../domain/person/PersonEntity.java | 45 ------ .../domain/person/PersonRepository.java | 2 +- .../domain/person/PersonService.java | 149 +++++++++++++++++- .../person/dto/PersonUpsertRequest.java | 40 ++++- .../person/dto/PersonUpsertResponse.java | 15 +- .../ivo/threekidfamily/ThreeKidFamilyIT.java | 4 +- .../domain/person/PersonServiceTest.java | 132 ++++++++++++++++ 9 files changed, 393 insertions(+), 58 deletions(-) delete mode 100644 src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonEntity.java create mode 100644 src/test/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonServiceTest.java diff --git a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/Person.java b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/Person.java index 5541530..2a0081b 100644 --- a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/Person.java +++ b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/Person.java @@ -1,4 +1,50 @@ package com.spijkerman.ivo.threekidfamily.domain.person; -public record Person() { +import jakarta.annotation.Nullable; +import jakarta.persistence.ElementCollection; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.Id; +import lombok.*; +import lombok.experimental.Accessors; + +import java.time.LocalDate; +import java.util.HashSet; +import java.util.Set; + +@Data +@Entity +@Accessors(fluent = true) +@Builder(toBuilder = true) +@NoArgsConstructor +@AllArgsConstructor +public class Person { + + @Id + @NonNull + private Integer id; + + @Nullable + private String name; + + @Nullable + private LocalDate birthDate; + + // Store only relation IDs instead of JPA mappings to reduce JPA/Hibernate tomfoolery + @Nullable + private Integer partnerId; + + @NonNull + @Singular + @ElementCollection(fetch = FetchType.EAGER) + private Set childIds = new HashSet<>(); + + @NonNull + @Singular + @ElementCollection(fetch = FetchType.EAGER) + private Set parentIds = new HashSet<>(); + + public Person(int id) { + this.id = id; + } } diff --git a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonController.java b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonController.java index 2a5956d..7412e9c 100644 --- a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonController.java +++ b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonController.java @@ -3,9 +3,11 @@ package com.spijkerman.ivo.threekidfamily.domain.person; import com.spijkerman.ivo.threekidfamily.domain.person.dto.PersonUpsertRequest; import com.spijkerman.ivo.threekidfamily.domain.person.dto.PersonUpsertResponse; import lombok.RequiredArgsConstructor; +import lombok.val; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.*; +import java.util.ArrayList; import java.util.List; @RestController @@ -19,7 +21,19 @@ public class PersonController { public ResponseEntity> upsertPerson( @RequestBody PersonUpsertRequest request ) { - throw new UnsupportedOperationException(); + val person = request.toDomain(); + personService.upsertPerson(person); + + val parentsInValidFamilies = personService.getParentsInValidFamilies(); + if (parentsInValidFamilies.isEmpty()) { + // Specs state HTTP 444, but since this is a custom nginx status + not a client error, I cannot do this + return ResponseEntity.noContent().build(); // 204 instead + } + val result = new ArrayList(); + for (val parent : parentsInValidFamilies) { + result.add(PersonUpsertResponse.fromDomain(parent)); + } + return ResponseEntity.ok(result); } @GetMapping("/{id}") diff --git a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonEntity.java b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonEntity.java deleted file mode 100644 index d25e826..0000000 --- a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonEntity.java +++ /dev/null @@ -1,45 +0,0 @@ -package com.spijkerman.ivo.threekidfamily.domain.person; - -import jakarta.annotation.Nullable; -import jakarta.persistence.ElementCollection; -import jakarta.persistence.Entity; -import jakarta.persistence.FetchType; -import jakarta.persistence.Id; -import lombok.*; - -import java.time.LocalDate; -import java.util.HashSet; -import java.util.Set; - -@Data -@Entity -@Builder -@NoArgsConstructor -@AllArgsConstructor -public class PersonEntity { - - @Id - @NonNull - private Integer id; - - @Nullable - private String name; - - @Nullable - private LocalDate birthDate; - - // Store only relation IDs instead of JPA mappings to reduce JPA/Hibernate tomfoolery - @Nullable - private Integer partnerId; - - @NonNull - @Singular - @ElementCollection(fetch = FetchType.EAGER) - private Set childIds = new HashSet<>(); - - @NonNull - @Singular - @ElementCollection(fetch = FetchType.EAGER) - private Set parentIds = new HashSet<>(); - -} diff --git a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonRepository.java b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonRepository.java index c6ea303..b07ed12 100644 --- a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonRepository.java +++ b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonRepository.java @@ -4,5 +4,5 @@ import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.stereotype.Repository; @Repository -public interface PersonRepository extends JpaRepository { +public interface PersonRepository extends JpaRepository { } diff --git a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonService.java b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonService.java index 6106fe7..adb560e 100644 --- a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonService.java +++ b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonService.java @@ -1,11 +1,15 @@ package com.spijkerman.ivo.threekidfamily.domain.person; +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; +import jakarta.transaction.Transactional; import lombok.Locked; -import lombok.NonNull; import lombok.RequiredArgsConstructor; +import lombok.val; +import org.apache.commons.lang3.tuple.Pair; import org.springframework.stereotype.Service; -import java.util.Set; +import java.util.*; @Service @RequiredArgsConstructor @@ -16,10 +20,142 @@ public class PersonService { /** * Upserts a person to storage, overwriting any existing or conflicting data. * - * @param person The Person data to be persisted - * @return The IDs of the Persons that have been modified because of this operation. + * @param newPerson The Person data to be persisted */ - public @NonNull Set upsertPerson(Person person) { + @Locked.Write + @Transactional + public void upsertPerson(Person newPerson) { + int id = newPerson.id(); + val relatedPersons = retrieveRelatedOf(newPerson); + val oldPerson = relatedPersons.getLeft(); + val related = relatedPersons.getRight(); + + val modifiedPersons = oldPerson == null ? modifiedPersonSetOf( + setPartner(id, related, newPerson.partnerId(), null), + setParents(id, related, newPerson.parentIds(), Set.of()), + setChildren(id, related, newPerson.childIds(), Set.of()) + ) : modifiedPersonSetOf( + setPartner(id, related, newPerson.partnerId(), oldPerson.partnerId()), + setParents(id, related, newPerson.parentIds(), oldPerson.parentIds()), + setChildren(id, related, newPerson.childIds(), oldPerson.childIds()) + ); + + if (!Objects.equals(newPerson, oldPerson)) { + modifiedPersons.add(newPerson); + } + if (!modifiedPersons.isEmpty()) { + personRepository.saveAll(modifiedPersons); + } + } + + private Pair> retrieveRelatedOf(Person newPerson) { + val personsToConsider = Lists.newArrayList(newPerson); + val oldPerson = personRepository.findById(newPerson.id()); + oldPerson.ifPresent(personsToConsider::add); + + val ids = new HashSet(); + for (val person : personsToConsider) { + if (person.partnerId() != null) { + ids.add(person.partnerId()); + } + ids.addAll(person.parentIds()); + ids.addAll(person.childIds()); + } + + val related = new HashMap(); + for (val foundPerson : personRepository.findAllById(ids)) { + related.put(foundPerson.id(), foundPerson); + } + + // Need to copy, because .difference returns an unmodifiable view + val missingIds = Set.copyOf(Sets.difference(ids, related.keySet())); + for (val missingId : missingIds) { + related.put(missingId, new Person(missingId)); + } + + return Pair.of( + oldPerson.orElse(null), + Collections.unmodifiableMap(related) + ); + } + + private Set modifiedPersonSetOf( + Set modifiedPartners, + Set modifiedParents, + Set modifiedChildren + ) { + val result = new HashSet(); + result.addAll(modifiedPartners); + result.addAll(modifiedParents); + result.addAll(modifiedChildren); + return result; + } + + private Set setPartner( + int selfId, + Map related, + Integer newPartnerId, + Integer oldPartnerId + ) { + val modifiedPersons = new HashSet(); + if (Objects.equals(newPartnerId, oldPartnerId)) { + return Set.of(); + } + if (newPartnerId != null) { + val newPartner = related.get(newPartnerId); + newPartner.partnerId(selfId); + modifiedPersons.add(newPartner); + } + if (oldPartnerId != null) { + val oldPartner = related.get(oldPartnerId); + oldPartner.partnerId(null); + modifiedPersons.add(oldPartner); + } + return modifiedPersons; + } + + private Set setParents( + int childId, + Map related, + Set newParentIds, + Set oldParentIds + ) { + val modifiedPersons = new HashSet(); + for (val removeFromId : Sets.difference(oldParentIds, newParentIds)) { + val removeFrom = related.get(removeFromId); + removeFrom.childIds().remove(childId); + modifiedPersons.add(removeFrom); + } + for (val addToId : Sets.difference(newParentIds, oldParentIds)) { + val addTo = related.get(addToId); + addTo.childIds().add(childId); + modifiedPersons.add(addTo); + } + return modifiedPersons; + } + + private Set setChildren( + int parentId, + Map related, + Set newChildIds, + Set oldChildIds + ) { + val modifiedPersons = new HashSet(); + for (val removeFromId : Sets.difference(oldChildIds, newChildIds)) { + val removeFrom = related.get(removeFromId); + removeFrom.parentIds().remove(parentId); + modifiedPersons.add(removeFrom); + } + for (val addToId : Sets.difference(newChildIds, oldChildIds)) { + val addTo = related.get(addToId); + addTo.parentIds().add(parentId); + modifiedPersons.add(addTo); + } + return modifiedPersons; + } + + @Locked.Read + public SortedSet getParentsInValidFamilies() { throw new UnsupportedOperationException(); } @@ -29,14 +165,17 @@ public class PersonService { * @param id The ID of the person to be retrieved. * @return A Person object, may contain all information, or just the ID. */ + @Locked.Read public Person getPersonById(int id) { throw new UnsupportedOperationException(); } /** * Deletes all data related to the specified user, and blacklists that ID from later use. + * * @param id The ID of the person to be deleted and blacklisted. */ + @Locked.Write public void deletePersonById(int id) { throw new UnsupportedOperationException(); } diff --git a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/dto/PersonUpsertRequest.java b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/dto/PersonUpsertRequest.java index 57dc347..3610b0b 100644 --- a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/dto/PersonUpsertRequest.java +++ b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/dto/PersonUpsertRequest.java @@ -1,4 +1,42 @@ package com.spijkerman.ivo.threekidfamily.domain.person.dto; -public record PersonUpsertRequest() { +import com.spijkerman.ivo.threekidfamily.domain.person.Person; +import jakarta.annotation.Nullable; +import lombok.val; + +import java.time.LocalDate; +import java.util.List; + +public record PersonUpsertRequest( + int id, + @Nullable String name, + @Nullable LocalDate birthDate, + @Nullable PersonReference parent1, + @Nullable PersonReference parent2, + @Nullable PersonReference partner, + @Nullable List children +) { + public record PersonReference(int id){} + + public Person toDomain() { + var builder = Person.builder() + .id(id) + .name(name) + .parentId(idOf(parent1)) + .parentId(idOf(parent2)) + .partnerId(idOf(partner)); + if (children != null) { + for (val child : children) { + builder = builder.childId(idOf(child)); + } + } + return builder.build(); + } + + private static @Nullable Integer idOf(@Nullable PersonReference personReference) { + if (personReference == null) { + return null; + } + return personReference.id; + } } diff --git a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/dto/PersonUpsertResponse.java b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/dto/PersonUpsertResponse.java index b2c1571..58f5e3d 100644 --- a/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/dto/PersonUpsertResponse.java +++ b/src/main/java/com/spijkerman/ivo/threekidfamily/domain/person/dto/PersonUpsertResponse.java @@ -1,4 +1,17 @@ package com.spijkerman.ivo.threekidfamily.domain.person.dto; -public record PersonUpsertResponse() { +import com.spijkerman.ivo.threekidfamily.domain.person.Person; +import jakarta.annotation.Nullable; + +import java.time.LocalDate; + +public record PersonUpsertResponse( + int id, + @Nullable String name, + @Nullable LocalDate birthDate + // Not sure if relations should be returned as well, decided to not +) { + public static PersonUpsertResponse fromDomain(Person domain) { + return new PersonUpsertResponse(domain.id(), domain.name(), domain.birthDate()); + } } diff --git a/src/test/java/com/spijkerman/ivo/threekidfamily/ThreeKidFamilyIT.java b/src/test/java/com/spijkerman/ivo/threekidfamily/ThreeKidFamilyIT.java index 4e0c8a8..47d854b 100644 --- a/src/test/java/com/spijkerman/ivo/threekidfamily/ThreeKidFamilyIT.java +++ b/src/test/java/com/spijkerman/ivo/threekidfamily/ThreeKidFamilyIT.java @@ -1,9 +1,7 @@ package com.spijkerman.ivo.threekidfamily; import com.spijkerman.ivo.threekidfamily.domain.person.Person; -import com.spijkerman.ivo.threekidfamily.domain.person.PersonEntity; import com.spijkerman.ivo.threekidfamily.domain.person.PersonRepository; -import com.spijkerman.ivo.threekidfamily.domain.person.PersonService; import lombok.val; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -41,7 +39,7 @@ class ThreeKidFamilyIT { @Test void testDbRoundTrip() { - val given = PersonEntity.builder().id(12).build(); + val given = Person.builder().id(12).build(); val saved = personRepository.save(given); assertThat(saved).isEqualTo(given); diff --git a/src/test/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonServiceTest.java b/src/test/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonServiceTest.java new file mode 100644 index 0000000..44af357 --- /dev/null +++ b/src/test/java/com/spijkerman/ivo/threekidfamily/domain/person/PersonServiceTest.java @@ -0,0 +1,132 @@ +package com.spijkerman.ivo.threekidfamily.domain.person; + +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import lombok.val; +import org.assertj.core.util.Sets; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.time.LocalDate; +import java.util.*; + +import static org.mockito.Mockito.*; + +class PersonServiceTest { + + @Test + void testUpsertPerson_forFullPerson_verifyFamilySaved() { + val repository = repository(); + val sut = new PersonService(repository); + + val given = Person.builder() + .id(1) + .name("Alice") + .birthDate(LocalDate.now()) + .parentId(2) + .childId(10) + .partnerId(20) + .build(); + + sut.upsertPerson(given); + + verify(repository).findById(1); + verify(repository).findAllById(Set.of(2, 10, 20)); + verify(repository).saveAll(Set.of( + given, // Alice + Person.builder().id(2).childId(1).build(), // Parent + Person.builder().id(10).parentId(1).build(), // Child + Person.builder().id(20).partnerId(1).build() // Partner + )); + verifyNoMoreInteractions(repository); + } + + @Test + void testUpsertPerson_forFullThenSlimPerson_expectFamilyStripped() { + val repository = repository(); + val sut = new PersonService(repository); + + val fullGiven = Person.builder() + .id(1) + .name("Alice") + .birthDate(LocalDate.now()) + .parentId(2) + .childId(10) + .partnerId(20) + .build(); + + val slimGiven = Person.builder() + .id(1) + .build(); + + sut.upsertPerson(fullGiven); + clearInvocations(repository); // So we only have to verify the invocations of inserting slim alice + sut.upsertPerson(slimGiven); + + verify(repository).findById(1); + verify(repository).findAllById(Set.of(2, 10, 20)); + verify(repository).saveAll(Set.of( + slimGiven, + Person.builder().id(2).build(), // Parent + Person.builder().id(10).build(), // Child + Person.builder().id(20).build() // Partner + )); + verifyNoMoreInteractions(repository); + } + + @Test + void testUpsertPerson_forTwoPartners_expectOneSave() { + val repository = repository(); + val sut = new PersonService(repository); + + val alice = Person.builder() + .id(1) + .partnerId(2) + .build(); + val bob = Person.builder() + .id(2) + .partnerId(1) + .build(); + + sut.upsertPerson(alice); + sut.upsertPerson(bob); + + verify(repository).findById(1); + verify(repository).findById(2); + verify(repository).findAllById(Set.of(1)); + verify(repository).findAllById(Set.of(2)); + verify(repository).saveAll(Set.of(alice, bob)); + verifyNoMoreInteractions(repository); + } + + private static PersonRepository repository() { + val mockDb = new HashMap(); + val repository = mock(PersonRepository.class); + + when(repository.save(any())).then(inv -> { + Person person = inv.getArgument(0); + mockDb.put(person.id(), person); + return person; + }); + + when(repository.saveAll(any())).then(inv -> { + Iterable persons = inv.getArgument(0); + for (val person : persons) { + mockDb.put(person.id(), person); + } + return Lists.newArrayList(persons); + }); + + when(repository.findById(any())).then(inv -> { + int id = inv.getArgument(0); + return Optional.ofNullable(mockDb.get(id)); + }); + + when(repository.findAllById(any())).then(inv -> { + val ids = Sets.newHashSet(inv.getArgument(0)); + return List.copyOf(Maps.filterKeys(mockDb, ids::contains).values()); + }); + + return repository; + } +} \ No newline at end of file