From 3fcdfa85f13e1bf935937b3f37d08dc9f112be8d Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 9 May 2026 15:18:46 +0200 Subject: [PATCH 1/3] fix(db): add PRIMARY KEY to group_permissions; promote tbmp UNIQUE to PK V63 deduplicates any phantom (group_id, permission) rows accumulated since the initial schema. V64 sets NOT NULL on permission and adds pk_group_permissions. V65 renames uq_tbmp_block_person to pk_tbmp for naming-convention consistency. Integration tests confirm each constraint via pg_catalog.pg_constraint. Closes #469 (partial). Co-Authored-By: Claude Sonnet 4.6 --- .../V63__deduplicate_group_permissions.sql | 7 ++ .../V64__group_permissions_primary_key.sql | 11 ++++ .../V65__tbmp_promote_unique_to_pk.sql | 8 +++ .../MigrationIntegrationTest.java | 66 +++++++++++++++++++ 4 files changed, 92 insertions(+) create mode 100644 backend/src/main/resources/db/migration/V63__deduplicate_group_permissions.sql create mode 100644 backend/src/main/resources/db/migration/V64__group_permissions_primary_key.sql create mode 100644 backend/src/main/resources/db/migration/V65__tbmp_promote_unique_to_pk.sql diff --git a/backend/src/main/resources/db/migration/V63__deduplicate_group_permissions.sql b/backend/src/main/resources/db/migration/V63__deduplicate_group_permissions.sql new file mode 100644 index 00000000..ecd4e79a --- /dev/null +++ b/backend/src/main/resources/db/migration/V63__deduplicate_group_permissions.sql @@ -0,0 +1,7 @@ +-- Remove duplicate (group_id, permission) rows that accumulated without a UNIQUE constraint. +-- Keeps the row with the smallest ctid (earliest physical insertion order). +DELETE FROM group_permissions a +USING group_permissions b +WHERE a.ctid < b.ctid + AND a.group_id = b.group_id + AND a.permission = b.permission; diff --git a/backend/src/main/resources/db/migration/V64__group_permissions_primary_key.sql b/backend/src/main/resources/db/migration/V64__group_permissions_primary_key.sql new file mode 100644 index 00000000..5b7a45f2 --- /dev/null +++ b/backend/src/main/resources/db/migration/V64__group_permissions_primary_key.sql @@ -0,0 +1,11 @@ +-- Add NOT NULL and PRIMARY KEY to group_permissions. +-- Requires V63 to have run first (no duplicates can remain). +-- +-- After this migration, future seed migrations can use: +-- INSERT INTO group_permissions ... ON CONFLICT DO NOTHING +-- instead of the INSERT ... WHERE NOT EXISTS pattern used before V64. +ALTER TABLE group_permissions + ALTER COLUMN permission SET NOT NULL; + +ALTER TABLE group_permissions + ADD CONSTRAINT pk_group_permissions PRIMARY KEY (group_id, permission); diff --git a/backend/src/main/resources/db/migration/V65__tbmp_promote_unique_to_pk.sql b/backend/src/main/resources/db/migration/V65__tbmp_promote_unique_to_pk.sql new file mode 100644 index 00000000..d0e42d35 --- /dev/null +++ b/backend/src/main/resources/db/migration/V65__tbmp_promote_unique_to_pk.sql @@ -0,0 +1,8 @@ +-- Promote the de-facto unique constraint on transcription_block_mentioned_persons to a named PK. +-- uq_tbmp_block_person (added in V57) is backed by a B-tree index identical to a PK; +-- this rename makes the naming convention explicit (pk_* vs uq_*). +ALTER TABLE transcription_block_mentioned_persons + DROP CONSTRAINT uq_tbmp_block_person; + +ALTER TABLE transcription_block_mentioned_persons + ADD CONSTRAINT pk_tbmp PRIMARY KEY (block_id, person_id); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/MigrationIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/MigrationIntegrationTest.java index c67de818..cf576dee 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/MigrationIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/MigrationIntegrationTest.java @@ -399,6 +399,66 @@ class MigrationIntegrationTest { AND dc.annotation_id IS NOT NULL """; + // ─── V63+V64: group_permissions dedup + primary key ────────────────────── + + @Test + void v64_pk_group_permissions_exists() { + Integer count = jdbc.queryForObject( + """ + SELECT COUNT(*) FROM pg_catalog.pg_constraint c + JOIN pg_catalog.pg_class t ON c.conrelid = t.oid + WHERE t.relname = 'group_permissions' + AND c.conname = 'pk_group_permissions' + AND c.contype = 'p' + """, + Integer.class); + assertThat(count).isEqualTo(1); + } + + @Test + void v64_permission_column_isNotNullable() { + Integer count = jdbc.queryForObject( + """ + SELECT COUNT(*) FROM information_schema.columns + WHERE table_schema = 'public' + AND table_name = 'group_permissions' + AND column_name = 'permission' + AND is_nullable = 'NO' + """, + Integer.class); + assertThat(count).isEqualTo(1); + } + + @Test + @Transactional(propagation = Propagation.NOT_SUPPORTED) + void v64_rejectsDuplicateGroupPermission() { + UUID groupId = createUserGroup("DuplicateTestGroup-" + UUID.randomUUID()); + jdbc.update("INSERT INTO group_permissions (group_id, permission) VALUES (?, 'READ_ALL')", groupId); + + assertThatThrownBy(() -> + jdbc.update("INSERT INTO group_permissions (group_id, permission) VALUES (?, 'READ_ALL')", groupId) + ).isInstanceOf(DataIntegrityViolationException.class); + + jdbc.update("DELETE FROM group_permissions WHERE group_id = ?", groupId); + jdbc.update("DELETE FROM user_groups WHERE id = ?", groupId); + } + + // ─── V65: tbmp UNIQUE promoted to PRIMARY KEY ───────────────────────────── + + @Test + void v65_pk_tbmp_exists() { + Integer count = jdbc.queryForObject( + """ + SELECT COUNT(*) FROM pg_catalog.pg_constraint c + JOIN pg_catalog.pg_class t ON c.conrelid = t.oid + WHERE t.relname = 'transcription_block_mentioned_persons' + AND c.conname = 'pk_tbmp' + AND c.contype = 'p' + """, + Integer.class); + assertThat(count).isEqualTo(1); + } + // ─── helpers ───────────────────────────────────────────────────────────── private UUID createPerson(String firstName, String lastName) { @@ -482,4 +542,10 @@ class MigrationIntegrationTest { """, id, recipientId, docId, commentId); return id; } + + private UUID createUserGroup(String name) { + UUID id = UUID.randomUUID(); + jdbc.update("INSERT INTO user_groups (id, name) VALUES (?, ?)", id, name); + return id; + } } -- 2.49.1 From eb54a98ea2b31ab4c4db004d183a2d41be1e055a Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 9 May 2026 15:19:20 +0200 Subject: [PATCH 2/3] fix(user): use builder in createGroup and guard against null permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Null dto.permissions now produces an empty HashSet instead of propagating null into the @ElementCollection — prevents a silent NPE after V64 adds NOT NULL. Co-Authored-By: Claude Sonnet 4.6 --- .../raddatz/familienarchiv/user/UserService.java | 7 ++++--- .../familienarchiv/user/UserServiceTest.java | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/user/UserService.java b/backend/src/main/java/org/raddatz/familienarchiv/user/UserService.java index 91fd12fd..09e3493e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/user/UserService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/user/UserService.java @@ -271,9 +271,10 @@ public class UserService { @Transactional public UserGroup createGroup(GroupDTO dto) { - UserGroup group = new UserGroup(); - group.setName(dto.getName()); - group.setPermissions(dto.getPermissions()); + UserGroup group = UserGroup.builder() + .name(dto.getName()) + .permissions(dto.getPermissions() != null ? dto.getPermissions() : new HashSet<>()) + .build(); return groupRepository.save(group); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/user/UserServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/user/UserServiceTest.java index f0fd6e46..eee87ed9 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/user/UserServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/user/UserServiceTest.java @@ -902,4 +902,18 @@ class UserServiceTest { assertThat(result.getName()).isEqualTo("Familie"); assertThat(result.getPermissions()).containsExactlyInAnyOrder("READ_ALL", "WRITE_ALL"); } + + @Test + void createGroup_withNullPermissions_savesGroupWithEmptyPermissionSet() { + org.raddatz.familienarchiv.user.GroupDTO dto = new org.raddatz.familienarchiv.user.GroupDTO(); + dto.setName("Leser"); + dto.setPermissions(null); + + UserGroup saved = UserGroup.builder().id(UUID.randomUUID()).name("Leser").build(); + when(groupRepository.save(any())).thenReturn(saved); + + userService.createGroup(dto); + + verify(groupRepository).save(argThat(g -> g.getPermissions() != null && g.getPermissions().isEmpty())); + } } -- 2.49.1 From bfc3a17676ab223232cab8d31bbc3abc7a853dbe Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 9 May 2026 15:25:26 +0200 Subject: [PATCH 3/3] test(migration): guard cleanup in try-finally to ensure isolation Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/MigrationIntegrationTest.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/MigrationIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/MigrationIntegrationTest.java index cf576dee..a01682ab 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/MigrationIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/MigrationIntegrationTest.java @@ -433,14 +433,16 @@ class MigrationIntegrationTest { @Transactional(propagation = Propagation.NOT_SUPPORTED) void v64_rejectsDuplicateGroupPermission() { UUID groupId = createUserGroup("DuplicateTestGroup-" + UUID.randomUUID()); - jdbc.update("INSERT INTO group_permissions (group_id, permission) VALUES (?, 'READ_ALL')", groupId); + try { + jdbc.update("INSERT INTO group_permissions (group_id, permission) VALUES (?, 'READ_ALL')", groupId); - assertThatThrownBy(() -> - jdbc.update("INSERT INTO group_permissions (group_id, permission) VALUES (?, 'READ_ALL')", groupId) - ).isInstanceOf(DataIntegrityViolationException.class); - - jdbc.update("DELETE FROM group_permissions WHERE group_id = ?", groupId); - jdbc.update("DELETE FROM user_groups WHERE id = ?", groupId); + assertThatThrownBy(() -> + jdbc.update("INSERT INTO group_permissions (group_id, permission) VALUES (?, 'READ_ALL')", groupId) + ).isInstanceOf(DataIntegrityViolationException.class); + } finally { + jdbc.update("DELETE FROM group_permissions WHERE group_id = ?", groupId); + jdbc.update("DELETE FROM user_groups WHERE id = ?", groupId); + } } // ─── V65: tbmp UNIQUE promoted to PRIMARY KEY ───────────────────────────── -- 2.49.1