From 3e1d66bb964580c43b93d38d04c155a387846860 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 23 Jul 2024 11:16:24 +0200 Subject: [PATCH 01/14] CTE query formatting and slow-query-logging --- .aliases | 2 +- Dockerfile | 10 + doc/rbac-performance-analysis.md | 247 ++++++++++++++++++ docker-compose.yml | 19 ++ etc/postgresql-log-slow-queries.conf | 10 + .../090-log-slow-queries-extensions.sql | 13 + .../db/changelog/1-rbac/1050-rbac-base.sql | 183 +++++++------ .../7010-hs-hosting-asset.sql | 2 + .../db/changelog/db.changelog-master.yaml | 2 + src/test/resources/application.yml | 8 +- 10 files changed, 398 insertions(+), 98 deletions(-) create mode 100644 Dockerfile create mode 100644 doc/rbac-performance-analysis.md create mode 100644 docker-compose.yml create mode 100644 etc/postgresql-log-slow-queries.conf create mode 100644 src/main/resources/db/changelog/0-basis/090-log-slow-queries-extensions.sql diff --git a/.aliases b/.aliases index f215442d..cabdb7d4 100644 --- a/.aliases +++ b/.aliases @@ -71,7 +71,7 @@ alias podman-stop='systemctl --user disable --now podman.socket && systemctl --u alias podman-use='export DOCKER_HOST="unix:///run/user/$UID/podman/podman.sock"; export TESTCONTAINERS_RYUK_DISABLED=true' alias gw=gradleWrapper -alias pg-sql-run='docker run --name hsadmin-ng-postgres -e POSTGRES_PASSWORD=password -p 5432:5432 -d postgres:15.5-bookworm' +alias pg-sql-run='docker run --name hsadmin-ng-postgres -e POSTGRES_PASSWORD=password -v COPY etc/postgresql-log-slow-queries.conf:/etc/postgresql/postgresql.conf -p 5432:5432 -d postgres:15.5-bookworm' alias pg-sql-stop='docker stop hsadmin-ng-postgres' alias pg-sql-start='docker container start hsadmin-ng-postgres' alias pg-sql-remove='docker rm hsadmin-ng-postgres' diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 00000000..8406f976 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,10 @@ +# build using: +# docker build -t postgres-with-contrib:15.5-bookworm . + +FROM postgres:15.5-bookworm + +RUN apt-get update && \ + apt-get install -y postgresql-contrib && \ + apt-get clean + +COPY etc/postgresql-log-slow-queries.conf /etc/postgresql/postgresql.conf diff --git a/doc/rbac-performance-analysis.md b/doc/rbac-performance-analysis.md new file mode 100644 index 00000000..de86bdb5 --- /dev/null +++ b/doc/rbac-performance-analysis.md @@ -0,0 +1,247 @@ +# RBAC Performance Analysis + +This describes the analysis of the legacy-data-import which took way too long, which turned out to be a problem in the RBAC-access-rights-check. + + +## Our Performance-Problem + +During the legacy data import for hosting assets we noticed massive performance problems. The import of about 2200 hosting-assets (IP-numbers, managed-webspaces, managed- and cloud-servers) as well as the creation of booking-items and booking-projects as well as necessary office-data entities (persons, contacts, partners, debitors, relations) **took 10-25 minutes**. + +We could not find a pattern, why the import mostly took about 25 minutes, but sometimes took *just* 10 minutes. The impression that it had to do with too many other parallel processes, e.g. browser with BBB or IntelliJ IDEA was proved wrong, but stopping all unnecessary processes and performing the import again. + + +## Preparation + +### Configuring PostgreSQL + +The pg_stat_statements PostgreSQL-Extension can be used to measure how long queries take and how often they are called. + +The module auto_explain can be used to automatically run EXPLAIN on long-running queries. + +To use this extension and module, we extended the PostgreSQL-Docker-image: + +```Dockerfile +FROM postgres:15.5-bookworm + +RUN apt-get update && \ + apt-get install -y postgresql-contrib && \ + apt-get clean + +COPY etc/postgresql-log-slow-queries.conf /etc/postgresql/postgresql.conf +``` + +And create an image from it: + +```sh +docker build -t postgres-with-contrib:15.5-bookworm . +``` + +Then we created a config file for PostgreSQL in `etc/postgresql-log-slow-queries.conf`: + +``` +shared_preload_libraries = 'pg_stat_statements,auto_explain' +log_min_duration_statement = 1000 +log_statement = 'all' +log_duration = on +pg_stat_statements.track = all +auto_explain.log_min_duration = '1s' # Logs queries taking longer than 1 second +auto_explain.log_analyze = on # Include actual run times +auto_explain.log_buffers = on # Include buffer usage statistics +auto_explain.log_format = 'json' # Format the log output in JSON +listen_addresses = '*' +``` + +And a Docker-Compose config in 'docker-compose.yml': + +``` +version: '3.8' + +services: + postgres: + image: postgres-with-contrib:15.5-bookworm + container_name: custom-postgres + environment: + POSTGRES_PASSWORD: password + volumes: + - /home/mi/Projekte/Hostsharing/hsadmin-ng/etc/postgresql-log-slow-queries.conf:/etc/postgresql/postgresql.conf + ports: + - "5432:5432" + command: + - bash + - -c + - > + apt-get update && + apt-get install -y postgresql-contrib && + docker-entrypoint.sh postgres -c config_file=/etc/postgresql/postgresql.conf +``` + +### Activate the pg_stat_statements Extension + +The pg_stat_statements extension was activated in our Liquibase-scripts: + +``` +create extension if not exists "pg_stat_statements"; +``` + +### Running the Tweaked PostgreSQL + +Now we can run PostgreSQL with activated slow-query-logging: + +```shell +docker-compose up -d +``` + +### Running the Import + +Using an environment like this: + +```shell +export HSADMINNG_POSTGRES_JDBC_URL=jdbc:postgresql://localhost:5432/postgres +export HSADMINNG_POSTGRES_ADMIN_USERNAME=postgres +export HSADMINNG_POSTGRES_ADMIN_PASSWORD=password +export HSADMINNG_POSTGRES_RESTRICTED_USERNAME=restricted +export HSADMINNG_SUPERUSER=superuser-alex@hostsharing.net +``` + +We can now run the hosting-assets-import: + +```shell +time gw-importHostingAssets +``` + +### Fetch the Query Statistics + +And afterward we can query the statistics in PostgreSQL: + +```SQL +SELECT pg_stat_statements_reset(); +``` + + +## Analysis Result + +### RBAC-Access-Rights Detection query + +This CTE query was run over 4000 times during a single import and takes in total the whole execution time of the import process: + +```SQL +WITH RECURSIVE grants AS ( + SELECT descendantUuid, ascendantUuid, $5 AS level + FROM RbacGrants + WHERE assumed + AND ascendantUuid = any(subjectIds) + UNION ALL + SELECT g.descendantUuid, g.ascendantUuid, grants.level + $6 AS level + FROM RbacGrants g + INNER JOIN grants ON grants.descendantUuid = g.ascendantUuid + WHERE g.assumed +), +granted AS ( + SELECT DISTINCT descendantUuid + FROM grants +) +SELECT DISTINCT perm.objectUuid + FROM granted + JOIN RbacPermission perm ON granted.descendantUuid = perm.uuid + JOIN RbacObject obj ON obj.uuid = perm.objectUuid + WHERE (requiredOp = $7 OR perm.op = requiredOp) + AND obj.objectTable = forObjectTable + LIMIT maxObjects+$8 +``` + +That query is used to determine access rights of the currently active RBAC-subject(s). + +We used `EXPLAIN` with a concrete version (parameters substituted with real values) of that query and got this result: + +``` +QUERY PLAN +Limit (cost=6549.08..6549.35 rows=54 width=16) + CTE grants + -> Recursive Union (cost=4.32..5845.97 rows=1103 width=36) + -> Bitmap Heap Scan on rbacgrants (cost=4.32..15.84 rows=3 width=36) + Recheck Cond: (ascendantuuid = ANY ('{ad1133dc-fbb7-43c9-8c20-0da3f89a2388}'::uuid[])) + Filter: assumed + -> Bitmap Index Scan on rbacgrants_ascendantuuid_idx (cost=0.00..4.32 rows=3 width=0) + Index Cond: (ascendantuuid = ANY ('{ad1133dc-fbb7-43c9-8c20-0da3f89a2388}'::uuid[])) + -> Nested Loop (cost=0.29..580.81 rows=110 width=36) + -> WorkTable Scan on grants grants_1 (cost=0.00..0.60 rows=30 width=20) + -> Index Scan using rbacgrants_ascendantuuid_idx on rbacgrants g (cost=0.29..19.29 rows=4 width=32) + Index Cond: (ascendantuuid = grants_1.descendantuuid) + Filter: assumed + -> Unique (cost=703.11..703.38 rows=54 width=16) + -> Sort (cost=703.11..703.25 rows=54 width=16) + Sort Key: perm.objectuuid + -> Nested Loop (cost=31.60..701.56 rows=54 width=16) + -> Hash Join (cost=31.32..638.78 rows=200 width=16) + Hash Cond: (perm.uuid = grants.descendantuuid) + -> Seq Scan on rbacpermission perm (cost=0.00..532.92 rows=28392 width=32) + -> Hash (cost=28.82..28.82 rows=200 width=16) + -> HashAggregate (cost=24.82..26.82 rows=200 width=16) + Group Key: grants.descendantuuid + -> CTE Scan on grants (cost=0.00..22.06 rows=1103 width=16) + -> Index Only Scan using rbacobject_objecttable_uuid_key on rbacobject obj (cost=0.28..0.31 rows=1 width=16) + Index Cond: ((objecttable = 'hs_hosting_asset'::text) AND (uuid = perm.objectuuid)) +``` + +### Office-Relations-Query + +```SQL +SELECT hore1_0.uuid,a1_0.uuid,a1_0.familyname,a1_0.givenname,a1_0.persontype,a1_0.salutation,a1_0.title,a1_0.tradename,a1_0.version,c1_0.uuid,c1_0.caption,c1_0.emailaddresses,c1_0.phonenumbers,c1_0.postaladdress,c1_0.version,h1_0.uuid,h1_0.familyname,h1_0.givenname,h1_0.persontype,h1_0.salutation,h1_0.title,h1_0.tradename,h1_0.version,hore1_0.mark,hore1_0.type,hore1_0.version + FROM hs_office_relation_rv hore1_0 + LEFT JOIN hs_office_person_rv a1_0 ON a1_0.uuid=hore1_0.anchoruuid + LEFT JOIN hs_office_contact_rv c1_0 ON c1_0.uuid=hore1_0.contactuuid + LEFT JOIN hs_office_person_rv h1_0 ON h1_0.uuid=hore1_0.holderuuid + WHERE hore1_0.uuid=$1 +``` + +That query into the `hs_office_relation_rv`-table is presumably the query to determine the partner of a debitor, which requires two of such queries each (Debitor -> Debitor-Relation -> Debitor-Anchor == Partner-Holder -> Partner-Relation -> Partner). + +### Total-Query-Time > Total-Import-Runtime + +That both queries total up to more than the runtime of the import-process is most likely due to internal parallel query processing. + +## Attempts to Mitigate the Problem + +### VACUUM ANALYZE + +In the middle of the import, we updated the PostgreSQL statistics to recalibrate the query optimizer: + +```SQL +VACUUM ANALYZE; +``` + +This did not improve the performance. + + +### Improving Indexes + +The sequentiap + +create index on RbacPermission (objectUuid, op); +create index on RbacPermission (opTableName, op); + +### + +We were suspicious about the sequential scan over all `rbacpermission` rows which was done by PostgreSQL to execute a HashJoin strategy. Turning off that strategy by + +```SQL +ALTER FUNCTION queryAccessibleObjectUuidsOfSubjectIds SET enable_hashjoin = off; +``` + +did not improve the performance though. The HashJoin was actually still applied, but no full table scan anymore: + +``` +[...] + QUERY PLAN + -> Hash Join (cost=36.02..40.78 rows=1 width=16) + Hash Cond: (grants.descendantuuid = perm.uuid) + -> HashAggregate (cost=13.32..15.32 rows=200 width=16) + Group Key: grants.descendantuuid + -> CTE Scan on grants (cost=0.00..11.84 rows=592 width=16) +[...] +``` + +The HashJoin strategy could be great if the hash-map could be kept for multiple invocations. But during an import process, of course, there are always new rows in the underlying table and the hash-map would be outdated immediately. + + + diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 00000000..974104bb --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,19 @@ +version: '3.8' + +services: + postgres: + image: postgres-with-contrib:15.5-bookworm + container_name: custom-postgres + environment: + POSTGRES_PASSWORD: password + volumes: + - /home/mi/Projekte/Hostsharing/hsadmin-ng/etc/postgresql-log-slow-queries.conf:/etc/postgresql/postgresql.conf + ports: + - "5432:5432" + command: + - bash + - -c + - > + apt-get update && + apt-get install -y postgresql-contrib && + docker-entrypoint.sh postgres -c config_file=/etc/postgresql/postgresql.conf diff --git a/etc/postgresql-log-slow-queries.conf b/etc/postgresql-log-slow-queries.conf new file mode 100644 index 00000000..a466c127 --- /dev/null +++ b/etc/postgresql-log-slow-queries.conf @@ -0,0 +1,10 @@ +shared_preload_libraries = 'pg_stat_statements,auto_explain' +log_min_duration_statement = 1000 +log_statement = 'all' +log_duration = on +pg_stat_statements.track = all +auto_explain.log_min_duration = '1s' # Logs queries taking longer than 1 second +auto_explain.log_analyze = on # Include actual run times +auto_explain.log_buffers = on # Include buffer usage statistics +auto_explain.log_format = 'json' # Format the log output in JSON +listen_addresses = '*' diff --git a/src/main/resources/db/changelog/0-basis/090-log-slow-queries-extensions.sql b/src/main/resources/db/changelog/0-basis/090-log-slow-queries-extensions.sql new file mode 100644 index 00000000..dd8aa74f --- /dev/null +++ b/src/main/resources/db/changelog/0-basis/090-log-slow-queries-extensions.sql @@ -0,0 +1,13 @@ +--liquibase formatted sql + + +-- ============================================================================ +-- PG-STAT-STATEMENTS-EXTENSION +--changeset pg-stat-statements-extension:1 runOnChange=true endDelimiter:--// +-- ---------------------------------------------------------------------------- +/* + Makes improved uuid generation available. + */ +create extension if not exists "pg_stat_statements"; +--// + diff --git a/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql b/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql index 6de59816..a2a8f750 100644 --- a/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql +++ b/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql @@ -372,6 +372,8 @@ create table RbacPermission op RbacOp not null, opTableName varchar(60) ); +create index on RbacPermission (objectUuid, op); +create index on RbacPermission (opTableName, op); ALTER TABLE RbacPermission ADD CONSTRAINT RbacPermission_uc UNIQUE NULLS NOT DISTINCT (objectUuid, op, opTableName); @@ -495,78 +497,68 @@ create index on RbacGrants (ascendantUuid); create index on RbacGrants (descendantUuid); call create_journal('RbacGrants'); - create or replace function findGrantees(grantedId uuid) returns setof RbacReference returns null on null input language sql as $$ -select reference.* - from (with recursive grants as (select descendantUuid, - ascendantUuid - from RbacGrants - where descendantUuid = grantedId - union all - select "grant".descendantUuid, - "grant".ascendantUuid - from RbacGrants "grant" - inner join grants recur on recur.ascendantUuid = "grant".descendantUuid) - select ascendantUuid - from grants) as grantee - join RbacReference reference on reference.uuid = grantee.ascendantUuid; +with recursive grants as ( + select descendantUuid, ascendantUuid + from RbacGrants + where descendantUuid = grantedId + union all + select g.descendantUuid, g.ascendantUuid + from RbacGrants g + inner join grants on grants.ascendantUuid = g.descendantUuid +) +select ref.* + from grants + join RbacReference ref on ref.uuid = grants.ascendantUuid; +$$; + +create or replace function isGranted(granteeIds uuid[], grantedId uuid) + returns bool + returns null on null input + language sql as $$ +with recursive grants as ( + select descendantUuid, ascendantUuid + from RbacGrants + where descendantUuid = grantedId + union all + select "grant".descendantUuid, "grant".ascendantUuid + from RbacGrants "grant" + inner join grants recur on recur.ascendantUuid = "grant".descendantUuid +) +select exists ( + select true + from grants + where ascendantUuid = any(granteeIds) +) or grantedId = any(granteeIds); $$; create or replace function isGranted(granteeId uuid, grantedId uuid) returns bool returns null on null input language sql as $$ -select granteeId = grantedId or granteeId in (with recursive grants as (select descendantUuid, ascendantUuid - from RbacGrants - where descendantUuid = grantedId - union all - select "grant".descendantUuid, "grant".ascendantUuid - from RbacGrants "grant" - inner join grants recur on recur.ascendantUuid = "grant".descendantUuid) - select ascendantUuid - from grants); +select * from isGranted(array[granteeId], grantedId); $$; - -create or replace function isGranted(granteeIds uuid[], grantedId uuid) - returns bool - returns null on null input - language plpgsql as $$ -declare - granteeId uuid; -begin - -- TODO.perf: needs optimization - foreach granteeId in array granteeIds - loop - if isGranted(granteeId, grantedId) then - return true; - end if; - end loop; - return false; -end; $$; - create or replace function isPermissionGrantedToSubject(permissionId uuid, subjectId uuid) returns BOOL stable -- leakproof language sql as $$ +with recursive grants as ( + select descendantUuid, ascendantUuid + from RbacGrants + where descendantUuid = permissionId + union all + select g.descendantUuid, g.ascendantUuid + from RbacGrants g + inner join grants on grants.ascendantUuid = g.descendantUuid +) select exists( - select * - from RbacUser - where uuid in (with recursive grants as (select descendantUuid, - ascendantUuid - from RbacGrants g - where g.descendantUuid = permissionId - union all - select g.descendantUuid, - g.ascendantUuid - from RbacGrants g - inner join grants recur on recur.ascendantUuid = g.descendantUuid) - select ascendantUuid - from grants - where ascendantUuid = subjectId) - ); + select true + from grants + where ascendantUuid = subjectId +); $$; create or replace function hasInsertPermission(objectUuid uuid, tableName text ) @@ -708,14 +700,14 @@ begin end; $$; -- ============================================================================ ---changeset rbac-base-QUERY-ACCESSIBLE-OBJECT-UUIDS:1 endDelimiter:--// +--changeset rbac-base-QUERY-ACCESSIBLE-OBJECT-UUIDS:1 runOnChange=true endDelimiter:--// -- ---------------------------------------------------------------------------- /* */ create or replace function queryAccessibleObjectUuidsOfSubjectIds( requiredOp RbacOp, - forObjectTable varchar, -- reduces the result set, but is not really faster when used in restricted view + forObjectTable varchar, subjectIds uuid[], maxObjects integer = 8000) returns setof uuid @@ -724,23 +716,29 @@ create or replace function queryAccessibleObjectUuidsOfSubjectIds( declare foundRows bigint; begin - return query select distinct perm.objectUuid - from (with recursive grants as (select descendantUuid, ascendantUuid, 1 as level - from RbacGrants - where assumed - and ascendantUuid = any (subjectIds) - union - distinct - select "grant".descendantUuid, "grant".ascendantUuid, level + 1 as level - from RbacGrants "grant" - inner join grants recur on recur.descendantUuid = "grant".ascendantUuid - where assumed) - select descendantUuid - from grants) as granted - join RbacPermission perm - on granted.descendantUuid = perm.uuid and (requiredOp = 'SELECT' or perm.op = requiredOp) - join RbacObject obj on obj.uuid = perm.objectUuid and obj.objectTable = forObjectTable - limit maxObjects + 1; + return query + WITH RECURSIVE grants AS ( + SELECT descendantUuid, ascendantUuid, 1 AS level + FROM RbacGrants + WHERE assumed + AND ascendantUuid = any(subjectIds) + UNION ALL + SELECT g.descendantUuid, g.ascendantUuid, grants.level + 1 AS level + FROM RbacGrants g + INNER JOIN grants ON grants.descendantUuid = g.ascendantUuid + WHERE g.assumed + ), + granted AS ( + SELECT DISTINCT descendantUuid + FROM grants + ) + SELECT DISTINCT perm.objectUuid + FROM granted + JOIN RbacPermission perm ON granted.descendantUuid = perm.uuid + JOIN RbacObject obj ON obj.uuid = perm.objectUuid + WHERE (requiredOp = 'SELECT' OR perm.op = requiredOp) + AND obj.objectTable = forObjectTable + LIMIT maxObjects+1; foundRows = lastRowCount(); if foundRows > maxObjects then @@ -751,7 +749,7 @@ begin end if; end; $$; - +ALTER FUNCTION queryAccessibleObjectUuidsOfSubjectIds SET enable_hashjoin = off; --// -- ============================================================================ @@ -764,24 +762,23 @@ create or replace function queryPermissionsGrantedToSubjectId(subjectId uuid) returns setof RbacPermission strict language sql as $$ - -- @formatter:off -select * - from RbacPermission - where uuid in ( - with recursive grants as ( - select distinct descendantUuid, ascendantUuid - from RbacGrants - where ascendantUuid = subjectId - union all - select "grant".descendantUuid, "grant".ascendantUuid - from RbacGrants "grant" - inner join grants recur on recur.descendantUuid = "grant".ascendantUuid - ) - select descendantUuid - from grants - ); --- @formatter:on +with recursive grants as ( + select descendantUuid, ascendantUuid + from RbacGrants + where ascendantUuid = subjectId + union all + select g.descendantUuid, g.ascendantUuid + from RbacGrants g + inner join grants on grants.descendantUuid = g.ascendantUuid +) +select perm.* + from RbacPermission perm + where perm.uuid in ( + select descendantUuid + from grants + ); $$; + --// -- ============================================================================ diff --git a/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7010-hs-hosting-asset.sql b/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7010-hs-hosting-asset.sql index 3b1b54d1..c1c127f0 100644 --- a/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7010-hs-hosting-asset.sql +++ b/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7010-hs-hosting-asset.sql @@ -44,6 +44,8 @@ create table if not exists hs_hosting_asset constraint chk_hs_hosting_asset_has_booking_item_or_parent_asset check (bookingItemUuid is not null or parentAssetUuid is not null or type in ('DOMAIN_SETUP', 'IPV4_NUMBER', 'IPV6_NUMBER')) ); +-- create index on hs_hosting_asset (identifier); +-- create index on hs_hosting_asset (type); --// diff --git a/src/main/resources/db/changelog/db.changelog-master.yaml b/src/main/resources/db/changelog/db.changelog-master.yaml index d6b4942b..a9c6711d 100644 --- a/src/main/resources/db/changelog/db.changelog-master.yaml +++ b/src/main/resources/db/changelog/db.changelog-master.yaml @@ -21,6 +21,8 @@ databaseChangeLog: file: db/changelog/0-basis/010-context.sql - include: file: db/changelog/0-basis/020-audit-log.sql + - include: + file: db/changelog/0-basis/090-log-slow-queries-extensions.sql - include: file: db/changelog/1-rbac/1050-rbac-base.sql - include: diff --git a/src/test/resources/application.yml b/src/test/resources/application.yml index 40ae85bb..696dfbd9 100644 --- a/src/test/resources/application.yml +++ b/src/test/resources/application.yml @@ -28,7 +28,7 @@ spring: change-log: classpath:/db/changelog/db.changelog-master.yaml contexts: tc,test,dev -logging: - level: - liquibase: INFO - net.ttddyy.dsproxy.listener: DEBUG +#logging: +# level: +# liquibase: INFO +# net.ttddyy.dsproxy.listener: DEBUG -- 2.39.5 From ac956d3b0582d3db6c912b80e1f5524a96976eb7 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 23 Jul 2024 13:00:47 +0200 Subject: [PATCH 02/14] move recursive CTE query queryAccessibleObjectUuidsOfSubjectIds to the rbac-generator for *_rv --- .../db/changelog/1-rbac/1050-rbac-base.sql | 8 ++--- .../changelog/1-rbac/1058-rbac-generators.sql | 34 +++++++++++++++---- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql b/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql index a2a8f750..5722c26e 100644 --- a/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql +++ b/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql @@ -728,10 +728,10 @@ begin INNER JOIN grants ON grants.descendantUuid = g.ascendantUuid WHERE g.assumed ), - granted AS ( - SELECT DISTINCT descendantUuid - FROM grants - ) + granted AS ( + SELECT DISTINCT descendantUuid + FROM grants + ) SELECT DISTINCT perm.objectUuid FROM granted JOIN RbacPermission perm ON granted.descendantUuid = perm.uuid diff --git a/src/main/resources/db/changelog/1-rbac/1058-rbac-generators.sql b/src/main/resources/db/changelog/1-rbac/1058-rbac-generators.sql index 016b8f89..86d9b673 100644 --- a/src/main/resources/db/changelog/1-rbac/1058-rbac-generators.sql +++ b/src/main/resources/db/changelog/1-rbac/1058-rbac-generators.sql @@ -175,16 +175,38 @@ begin Creates a restricted view based on the 'SELECT' permission of the current subject. */ sql := format($sql$ - set session session authorization default; - create view %1$s_rv as - with accessibleObjects as ( - select queryAccessibleObjectUuidsOfSubjectIds('SELECT', '%1$s', currentSubjectsUuids()) + create or replace view %1$s_rv as + with accessible_%1$s_uuids as ( + + with recursive grants as ( + select descendantUuid, ascendantUuid, 1 as level + from RbacGrants + where assumed + and ascendantUuid = any (currentSubjectsuUids()) + union all + select g.descendantUuid, g.ascendantUuid, level + 1 as level + from RbacGrants g + inner join grants on grants.descendantUuid = g.ascendantUuid + where g.assumed + ), + granted as ( + select distinct descendantUuid + from grants + ) + select distinct perm.objectUuid as objectUuid + from granted + join RbacPermission perm on granted.descendantUuid = perm.uuid + join RbacObject obj on obj.uuid = perm.objectUuid + where perm.op = 'SELECT' + and obj.objectTable = '%1$s' + limit 8001 ) select target.* from %1$s as target - where target.uuid in (select * from accessibleObjects) + where target.uuid in (select * from accessible_%1$s_uuids) order by %2$s; - grant all privileges on %1$s_rv to ${HSADMINNG_POSTGRES_RESTRICTED_USERNAME}; + + grant all privileges on %1$s_rv to ${HSADMINNG_POSTGRES_RESTRICTED_USERNAME}; $sql$, targetTable, orderBy); execute sql; -- 2.39.5 From b50bcbbca0d29e37f70990b7dc66705995fc8131 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 23 Jul 2024 14:33:48 +0200 Subject: [PATCH 03/14] add partnerNumber to HsOfficeDebitorEntity --- .../office/debitor/HsOfficeDebitorEntity.java | 24 ++------------ .../partner/HsOfficePartnerRepository.java | 11 +++++++ .../relation/HsOfficeRelationEntity.java | 6 ++-- .../506-debitor/5060-hs-office-debitor.sql | 1 + .../5068-hs-office-debitor-test-data.sql | 13 ++++---- .../6100-hs-booking-debitor.sql | 10 ++---- ...HostingAssetRepositoryIntegrationTest.java | 2 +- .../hs/migration/ImportOfficeData.java | 2 +- .../HsOfficeDebitorEntityUnitTest.java | 31 +++---------------- ...fficeDebitorRepositoryIntegrationTest.java | 3 -- .../office/debitor/TestHsOfficeDebitor.java | 2 +- ...ceSepaMandateControllerAcceptanceTest.java | 9 ++++-- 12 files changed, 43 insertions(+), 71 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java index 9cf134c9..ad5857ab 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java @@ -15,9 +15,6 @@ import net.hostsharing.hsadminng.rbac.rbacdef.RbacView.SQL; import net.hostsharing.hsadminng.stringify.Stringify; import net.hostsharing.hsadminng.stringify.Stringifyable; import org.hibernate.annotations.GenericGenerator; -import org.hibernate.annotations.JoinFormula; -import org.hibernate.annotations.NotFound; -import org.hibernate.annotations.NotFoundAction; import jakarta.persistence.Column; import jakarta.persistence.Entity; @@ -77,22 +74,8 @@ public class HsOfficeDebitorEntity implements RbacObject, Stringifyable { @Version private int version; - @ManyToOne - @JoinFormula( - referencedColumnName = "uuid", - value = """ - ( - SELECT DISTINCT partner.uuid - FROM hs_office_partner_rv partner - JOIN hs_office_relation_rv dRel - ON dRel.uuid = debitorreluuid AND dRel.type = 'DEBITOR' - JOIN hs_office_relation_rv pRel - ON pRel.uuid = partner.partnerRelUuid AND pRel.type = 'PARTNER' - WHERE pRel.holderUuid = dRel.anchorUuid - ) - """) - @NotFound(action = NotFoundAction.IGNORE) - private HsOfficePartnerEntity partner; + @Column(name = "partnernumber", columnDefinition = "numeric(5) not null") + private Integer partnerNumber; // redundant to HsOfficePartnerEntity.partnerNumber for performance reasons @Column(name = "debitornumbersuffix", length = 2) @Pattern(regexp = TWO_DECIMAL_DIGITS) @@ -125,9 +108,8 @@ public class HsOfficeDebitorEntity implements RbacObject, Stringifyable { private String defaultPrefix; private String getDebitorNumberString() { - return ofNullable(partner) + return ofNullable(partnerNumber) .filter(partner -> debitorNumberSuffix != null) - .map(HsOfficePartnerEntity::getPartnerNumber) .map(Object::toString) .map(partnerNumber -> partnerNumber + debitorNumberSuffix) .orElse(null); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepository.java b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepository.java index 2ae260bd..e32abb12 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepository.java @@ -28,6 +28,17 @@ public interface HsOfficePartnerRepository extends Repository findPartnerByOptionalNameLike(String name); HsOfficePartnerEntity findPartnerByPartnerNumber(Integer partnerNumber); + @Query(""" + SELECT DISTINCT partner + FROM HsOfficePartnerEntity partner + JOIN HsOfficeRelationEntity dRel + ON dRel.uuid = :debitorUuid AND dRel.type = 'DEBITOR' + JOIN HsOfficeRelationEntity pRel + ON pRel.uuid = partner.partnerRel.uuid AND pRel.type = 'PARTNER' + WHERE pRel.holder.uuid = dRel.anchor.uuid + """) + HsOfficePartnerEntity findPartnerByDebitorUuid(UUID debitorUuid); + HsOfficePartnerEntity save(final HsOfficePartnerEntity entity); long count(); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java index 7c8cd78e..92e6ab0c 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java @@ -56,15 +56,15 @@ public class HsOfficeRelationEntity implements RbacObject, Stringifyable { @Version private int version; - @ManyToOne + @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "anchoruuid") private HsOfficePersonEntity anchor; - @ManyToOne + @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "holderuuid") private HsOfficePersonEntity holder; - @ManyToOne + @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "contactuuid") private HsOfficeContactEntity contact; diff --git a/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql b/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql index bbf72543..0a64321a 100644 --- a/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql +++ b/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql @@ -8,6 +8,7 @@ create table hs_office_debitor ( uuid uuid unique references RbacObject (uuid) initially deferred, version int not null default 0, + partnerNumber numeric(5) not null, -- redundant to hs_office_partner.partnerNumber for performance reasons debitorNumberSuffix char(2) not null check (debitorNumberSuffix::text ~ '^[0-9][0-9]$'), debitorRelUuid uuid not null references hs_office_relation(uuid), billable boolean not null default true, diff --git a/src/main/resources/db/changelog/5-hs-office/506-debitor/5068-hs-office-debitor-test-data.sql b/src/main/resources/db/changelog/5-hs-office/506-debitor/5068-hs-office-debitor-test-data.sql index 2e888e29..6c5cde10 100644 --- a/src/main/resources/db/changelog/5-hs-office/506-debitor/5068-hs-office-debitor-test-data.sql +++ b/src/main/resources/db/changelog/5-hs-office/506-debitor/5068-hs-office-debitor-test-data.sql @@ -9,7 +9,8 @@ Creates a single debitor test record. */ create or replace procedure createHsOfficeDebitorTestData( - withDebitorNumberSuffix numeric(5), + forPartnerNumber numeric(5), + withDebitorNumberSuffix numeric(2), forPartnerPersonName varchar, forBillingContactCaption varchar, withDefaultPrefix varchar @@ -42,8 +43,8 @@ begin -- raise exception 'creating test debitor: (uuid=%, debitorRelUuid=%, debitornumbersuffix=%, billable=%, vatbusiness=%, vatreversecharge=%, refundbankaccountuuid=%, defaultprefix=%)', -- uuid_generate_v4(), relatedDebitorRelUuid, withDebitorNumberSuffix, true, true, false, relatedBankAccountUuid, withDefaultPrefix; insert - into hs_office_debitor (uuid, debitorRelUuid, debitornumbersuffix, billable, vatbusiness, vatreversecharge, refundbankaccountuuid, defaultprefix) - values (uuid_generate_v4(), relatedDebitorRelUuid, withDebitorNumberSuffix, true, true, false, relatedBankAccountUuid, withDefaultPrefix); + into hs_office_debitor (uuid, debitorRelUuid, partnerNumber, debitornumbersuffix, billable, vatbusiness, vatreversecharge, refundbankaccountuuid, defaultprefix) + values (uuid_generate_v4(), relatedDebitorRelUuid, forPartnerNumber, withDebitorNumberSuffix, true, true, false, relatedBankAccountUuid, withDefaultPrefix); end; $$; --// @@ -54,9 +55,9 @@ end; $$; do language plpgsql $$ begin - call createHsOfficeDebitorTestData(11, 'First GmbH', 'first contact', 'fir'); - call createHsOfficeDebitorTestData(12, 'Second e.K.', 'second contact', 'sec'); - call createHsOfficeDebitorTestData(13, 'Third OHG', 'third contact', 'thi'); + call createHsOfficeDebitorTestData( 10001, 11, 'First GmbH', 'first contact', 'fir'); + call createHsOfficeDebitorTestData( 10002, 12, 'Second e.K.', 'second contact', 'sec'); + call createHsOfficeDebitorTestData( 10003, 13, 'Third OHG', 'third contact', 'thi'); end; $$; --// diff --git a/src/main/resources/db/changelog/6-hs-booking/610-booking-debitor/6100-hs-booking-debitor.sql b/src/main/resources/db/changelog/6-hs-booking/610-booking-debitor/6100-hs-booking-debitor.sql index 72d9563f..d38640fc 100644 --- a/src/main/resources/db/changelog/6-hs-booking/610-booking-debitor/6100-hs-booking-debitor.sql +++ b/src/main/resources/db/changelog/6-hs-booking/610-booking-debitor/6100-hs-booking-debitor.sql @@ -1,17 +1,13 @@ --liquibase formatted sql -- ============================================================================ ---changeset hs-booking-debitor-RESTRICTED-VIEW:1 endDelimiter:--// +--changeset hs-booking-debitor-EXTRACTED-VIEW:1 endDelimiter:--// -- ---------------------------------------------------------------------------- create view hs_booking_debitor_xv as select debitor.uuid, debitor.version, - (partner.partnerNumber::varchar || debitor.debitorNumberSuffix)::numeric as debitorNumber, + (debitor.partnerNumber::varchar || debitor.debitorNumberSuffix)::numeric as debitorNumber, debitor.defaultPrefix - from hs_office_debitor debitor - -- RBAC for debitor is sufficient, for faster access we are bypassing RBAC for the join tables - join hs_office_relation debitorRel on debitor.debitorReluUid=debitorRel.uuid - join hs_office_relation partnerRel on partnerRel.holderUuid=debitorRel.anchorUuid - join hs_office_partner partner on partner.partnerReluUid=partnerRel.uuid; + from hs_office_debitor debitor -- not from _rv for performance, nothing really secret here --// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java index 99f0efd6..0025d8d5 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java @@ -208,8 +208,8 @@ class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanu // then exactlyTheseAssetsAreReturned( result, - "HsHostingAssetEntity(MANAGED_WEBSPACE, sec01, some Webspace, MANAGED_SERVER:vm1012, D-1000212:D-1000212 default project:separate ManagedWebspace)", "HsHostingAssetEntity(MANAGED_WEBSPACE, fir01, some Webspace, MANAGED_SERVER:vm1011, D-1000111:D-1000111 default project:separate ManagedWebspace)", + "HsHostingAssetEntity(MANAGED_WEBSPACE, sec01, some Webspace, MANAGED_SERVER:vm1012, D-1000212:D-1000212 default project:separate ManagedWebspace)", "HsHostingAssetEntity(MANAGED_WEBSPACE, thi01, some Webspace, MANAGED_SERVER:vm1013, D-1000313:D-1000313 default project:separate ManagedWebspace)"); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java b/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java index dd1f7d2b..ec179f33 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java @@ -759,7 +759,7 @@ public class ImportOfficeData extends CsvDataImport { final var debitor = HsOfficeDebitorEntity.builder() .debitorNumberSuffix("00") - .partner(partner) + .partnerNumber(partner.getPartnerNumber()) .debitorRel(debitorRel) .defaultPrefix(rec.getString("member_code").replace("hsh00-", "")) .billable(rec.isEmpty("free") || rec.getString("free").equals("f")) diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java index e1250775..193a0d59 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java @@ -29,9 +29,7 @@ class HsOfficeDebitorEntityUnitTest { .debitorNumberSuffix("67") .debitorRel(givenDebitorRel) .defaultPrefix("som") - .partner(HsOfficePartnerEntity.builder() - .partnerNumber(12345) - .build()) + .partnerNumber(12345) .build(); final var result = given.toString(); @@ -44,9 +42,7 @@ class HsOfficeDebitorEntityUnitTest { final var given = HsOfficeDebitorEntity.builder() .debitorRel(givenDebitorRel) .debitorNumberSuffix("67") - .partner(HsOfficePartnerEntity.builder() - .partnerNumber(12345) - .build()) + .partnerNumber(12345) .build(); final var result = given.toShortString(); @@ -59,9 +55,7 @@ class HsOfficeDebitorEntityUnitTest { final var given = HsOfficeDebitorEntity.builder() .debitorRel(givenDebitorRel) .debitorNumberSuffix("67") - .partner(HsOfficePartnerEntity.builder() - .partnerNumber(12345) - .build()) + .partnerNumber(12345) .build(); final var result = given.getDebitorNumber(); @@ -69,25 +63,12 @@ class HsOfficeDebitorEntityUnitTest { assertThat(result).isEqualTo(1234567); } - @Test - void getDebitorNumberWithoutPartnerReturnsNull() { - final var given = HsOfficeDebitorEntity.builder() - .debitorRel(givenDebitorRel) - .debitorNumberSuffix("67") - .partner(null) - .build(); - - final var result = given.getDebitorNumber(); - - assertThat(result).isNull(); - } - @Test void getDebitorNumberWithoutPartnerNumberReturnsNull() { final var given = HsOfficeDebitorEntity.builder() .debitorRel(givenDebitorRel) .debitorNumberSuffix("67") - .partner(HsOfficePartnerEntity.builder().build()) + .partnerNumber(null) .build(); final var result = given.getDebitorNumber(); @@ -100,9 +81,7 @@ class HsOfficeDebitorEntityUnitTest { final var given = HsOfficeDebitorEntity.builder() .debitorRel(givenDebitorRel) .debitorNumberSuffix(null) - .partner(HsOfficePartnerEntity.builder() - .partnerNumber(12345) - .build()) + .partnerNumber(12345) .build(); final var result = given.getDebitorNumber(); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java index dc1b3f61..e79644cf 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java @@ -485,9 +485,6 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean em.refresh(foundEntity); Hibernate.initialize(foundEntity); assertThat(foundEntity).isNotSameAs(saved); - if (withPartner) { - assertThat(foundEntity.getPartner()).isNotNull(); - } assertThat(foundEntity.getDebitorRel()).extracting(HsOfficeRelationEntity::toString) .isEqualTo(saved.getDebitorRel().toString()); }); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/TestHsOfficeDebitor.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/TestHsOfficeDebitor.java index b8ddf8b5..57b65678 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/TestHsOfficeDebitor.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/TestHsOfficeDebitor.java @@ -19,7 +19,7 @@ public class TestHsOfficeDebitor { .anchor(HsOfficePersonEntity.builder().build()) .contact(TEST_CONTACT) .build()) - .partner(TEST_PARTNER) + .partnerNumber(TEST_PARTNER.getPartnerNumber()) .defaultPrefix("abc") .build(); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java index c0f68451..40d9cee8 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java @@ -6,6 +6,7 @@ import io.restassured.http.ContentType; import net.hostsharing.hsadminng.HsadminNgApplication; import net.hostsharing.hsadminng.hs.office.bankaccount.HsOfficeBankAccountRepository; import net.hostsharing.hsadminng.hs.office.debitor.HsOfficeDebitorRepository; +import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerRepository; import net.hostsharing.hsadminng.rbac.test.ContextBasedTestWithCleanup; import net.hostsharing.hsadminng.rbac.test.JpaAttempt; import org.json.JSONException; @@ -42,6 +43,9 @@ class HsOfficeSepaMandateControllerAcceptanceTest extends ContextBasedTestWithCl @Autowired HsOfficeSepaMandateRepository sepaMandateRepo; + @Autowired + HsOfficePartnerRepository partnerRepo; + @Autowired HsOfficeDebitorRepository debitorRepo; @@ -493,8 +497,9 @@ class HsOfficeSepaMandateControllerAcceptanceTest extends ContextBasedTestWithCl return jpaAttempt.transacted(() -> { context.define("superuser-alex@hostsharing.net"); final var givenDebitor = debitorRepo.findDebitorByDebitorNumber(debitorNumber).get(0); - final var bankAccountHolder = ofNullable(givenDebitor.getPartner().getPartnerRel().getHolder().getTradeName()) - .orElse(givenDebitor.getPartner().getPartnerRel().getHolder().getFamilyName()); + final var givenPartner = partnerRepo.findPartnerByDebitorUuid(givenDebitor.getUuid()); + final var bankAccountHolder = ofNullable(givenPartner.getPartnerRel().getHolder().getTradeName()) + .orElse(givenPartner.getPartnerRel().getHolder().getFamilyName()); final var givenBankAccount = bankAccountRepo.findByOptionalHolderLike(bankAccountHolder).get(0); final var newSepaMandate = HsOfficeSepaMandateEntity.builder() .uuid(UUID.randomUUID()) -- 2.39.5 From b79a8ca22dc8f3d763b5c17f82f6516e6fde72d7 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Tue, 23 Jul 2024 17:08:06 +0200 Subject: [PATCH 04/14] introduce more lazy-loading references --- doc/rbac-performance-analysis.md | 14 ++ .../hs/booking/item/HsBookingItemEntity.java | 2 +- .../project/HsBookingProjectEntity.java | 2 +- .../hosting/asset/HsHostingAssetEntity.java | 2 +- .../HsOfficeBankAccountEntity.java | 2 +- .../office/contact/HsOfficeContactEntity.java | 2 +- .../HsOfficeCoopAssetsTransactionEntity.java | 2 +- .../HsOfficeCoopSharesTransactionEntity.java | 2 +- .../debitor/HsOfficeDebitorController.java | 7 + .../office/debitor/HsOfficeDebitorEntity.java | 14 +- .../membership/HsOfficeMembershipEntity.java | 9 +- .../partner/HsOfficePartnerDetailsEntity.java | 2 +- .../office/partner/HsOfficePartnerEntity.java | 6 +- .../partner/HsOfficePartnerRepository.java | 6 +- .../office/person/HsOfficePersonEntity.java | 2 +- .../relation/HsOfficeRelationEntity.java | 9 ++ .../HsOfficeSepaMandateEntity.java | 2 +- .../hsadminng/rbac/rbacobject/RbacObject.java | 10 +- .../rbac/test/cust/TestCustomerEntity.java | 2 +- .../rbac/test/dom/TestDomainEntity.java | 2 +- .../rbac/test/pac/TestPackageEntity.java | 2 +- ...ceBankAccountControllerAcceptanceTest.java | 5 +- ...OfficeContactControllerAcceptanceTest.java | 10 +- ...sTransactionRepositoryIntegrationTest.java | 2 +- ...sTransactionRepositoryIntegrationTest.java | 2 +- ...OfficeDebitorControllerAcceptanceTest.java | 127 +++--------------- ...fficeDebitorRepositoryIntegrationTest.java | 61 +++++---- ...ceMembershipRepositoryIntegrationTest.java | 20 +-- ...OfficePartnerControllerAcceptanceTest.java | 2 +- ...fficePartnerRepositoryIntegrationTest.java | 2 +- ...sOfficePersonControllerAcceptanceTest.java | 2 +- ...ficeRelationRepositoryIntegrationTest.java | 3 +- ...ceSepaMandateControllerAcceptanceTest.java | 2 +- 33 files changed, 158 insertions(+), 181 deletions(-) diff --git a/doc/rbac-performance-analysis.md b/doc/rbac-performance-analysis.md index de86bdb5..ad1db594 100644 --- a/doc/rbac-performance-analysis.md +++ b/doc/rbac-performance-analysis.md @@ -245,3 +245,17 @@ The HashJoin strategy could be great if the hash-map could be kept for multiple +### LAZY loading for Relation.anchorPerson/.holderPerson/ + +The slowest query now was fetching Relations joined with Contact, Anchor-Person and Holder-Person, for all tables using the restricted (RBAC) views (_rv). + +We changed these mappings from `EAGER` (default) to `LAZY` to `@ManyToOne(fetch = FetchType.LAZY)` and got this result: + +| query | calls | total (min) | mean (ms) | +|-------|-------|-------------|-----------| +| select hore1_0.uuid,hore1_0.anchoruuid,hore1_0.contactuuid,hore1_0.holderuuid,hore1_0.mark,hore1_0.type,hore1_0.version from hs_office_relation_rv hore1_0 where hore1_0.uuid=$1 | 517 | 5 | 565 | +| select hope1_0.uuid,hope1_0.familyname,hope1_0.givenname,hope1_0.persontype,hope1_0.salutation,hope1_0.title,hope1_0.tradename,hope1_0.version from hs_office_person_rv hope1_0 where hope1_0.uuid=$1 | 1015 | 4 | 240 | +| select hoce1_0.uuid,hoce1_0.caption,hoce1_0.emailaddresses,hoce1_0.phonenumbers,hoce1_0.postaladdress,hoce1_0.version from hs_office_contact_rv hoce1_0 where hoce1_0.uuid=$1 | 497 | 2 | 235 +select * from isGranted(array[granteeId], grantedId) | 44613 | 1 | 1 | + +Now, finally, the total runtime of the import was down to 12 minutes. diff --git a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java index 17b4fb65..5a0eb885 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/booking/item/HsBookingItemEntity.java @@ -70,7 +70,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @Setter @NoArgsConstructor @AllArgsConstructor -public class HsBookingItemEntity implements Stringifyable, RbacObject, PropertiesProvider { +public class HsBookingItemEntity implements Stringifyable, RbacObject, PropertiesProvider { private static Stringify stringify = stringify(HsBookingItemEntity.class) .withProp(HsBookingItemEntity::getProject) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/booking/project/HsBookingProjectEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/booking/project/HsBookingProjectEntity.java index b1cf4a41..8f5d1397 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/booking/project/HsBookingProjectEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/booking/project/HsBookingProjectEntity.java @@ -34,7 +34,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @Setter @NoArgsConstructor @AllArgsConstructor -public class HsBookingProjectEntity implements Stringifyable, RbacObject { +public class HsBookingProjectEntity implements Stringifyable, RbacObject { private static Stringify stringify = stringify(HsBookingProjectEntity.class) .withProp(HsBookingProjectEntity::getDebitor) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java index 96203a66..6965d82f 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetEntity.java @@ -70,7 +70,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @Setter @NoArgsConstructor @AllArgsConstructor -public class HsHostingAssetEntity implements Stringifyable, RbacObject, PropertiesProvider { +public class HsHostingAssetEntity implements Stringifyable, RbacObject, PropertiesProvider { private static Stringify stringify = stringify(HsHostingAssetEntity.class) .withProp(HsHostingAssetEntity::getType) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountEntity.java index 679e87a2..0e9ca079 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountEntity.java @@ -27,7 +27,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @AllArgsConstructor @FieldNameConstants @DisplayName("BankAccount") -public class HsOfficeBankAccountEntity implements RbacObject, Stringifyable { +public class HsOfficeBankAccountEntity implements RbacObject, Stringifyable { private static Stringify toString = stringify(HsOfficeBankAccountEntity.class, "bankAccount") .withIdProp(HsOfficeBankAccountEntity::getIban) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java index 1442e2cb..3bcaf140 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactEntity.java @@ -35,7 +35,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @AllArgsConstructor @FieldNameConstants @DisplayName("Contact") -public class HsOfficeContactEntity implements Stringifyable, RbacObject { +public class HsOfficeContactEntity implements Stringifyable, RbacObject { private static Stringify toString = stringify(HsOfficeContactEntity.class, "contact") .withProp(Fields.caption, HsOfficeContactEntity::getCaption) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java index 2cf4f089..6a366417 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java @@ -41,7 +41,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @NoArgsConstructor @AllArgsConstructor @DisplayName("CoopAssetsTransaction") -public class HsOfficeCoopAssetsTransactionEntity implements Stringifyable, RbacObject { +public class HsOfficeCoopAssetsTransactionEntity implements Stringifyable, RbacObject { private static Stringify stringify = stringify(HsOfficeCoopAssetsTransactionEntity.class) .withIdProp(HsOfficeCoopAssetsTransactionEntity::getTaggedMemberNumber) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java index c886170e..b1193945 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java @@ -39,7 +39,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @NoArgsConstructor @AllArgsConstructor @DisplayName("CoopShareTransaction") -public class HsOfficeCoopSharesTransactionEntity implements Stringifyable, RbacObject { +public class HsOfficeCoopSharesTransactionEntity implements Stringifyable, RbacObject { private static Stringify stringify = stringify(HsOfficeCoopSharesTransactionEntity.class) .withIdProp(HsOfficeCoopSharesTransactionEntity::getMemberNumberTagged) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java index 5455b99b..9aa0745a 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java @@ -5,6 +5,7 @@ import net.hostsharing.hsadminng.hs.office.generated.api.v1.api.HsOfficeDebitors import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeDebitorInsertResource; import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeDebitorPatchResource; import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeDebitorResource; +import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerRepository; import net.hostsharing.hsadminng.hs.office.relation.HsOfficeRelationEntity; import net.hostsharing.hsadminng.hs.office.relation.HsOfficeRelationRepository; import net.hostsharing.hsadminng.mapper.Mapper; @@ -37,6 +38,9 @@ public class HsOfficeDebitorController implements HsOfficeDebitorsApi { @Autowired private HsOfficeDebitorRepository debitorRepo; + @Autowired + private HsOfficePartnerRepository partnerRepo; + @Autowired private HsOfficeRelationRepository relRepo; @@ -91,6 +95,9 @@ public class HsOfficeDebitorController implements HsOfficeDebitorsApi { () -> { throw new EntityNotFoundException("ERROR: [400] debitorRelUuid not found: " + body.getDebitorRelUuid());}); } + final var relatedPartner = partnerRepo.findPartnerByPartnerPersonUuid(entityToSave.getDebitorRel().getHolder().getUuid()); + entityToSave.setPartnerNumber(relatedPartner.getPartnerNumber()); + final var savedEntity = debitorRepo.save(entityToSave); em.flush(); em.refresh(savedEntity); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java index ad5857ab..a759a975 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java @@ -7,13 +7,13 @@ import lombok.NoArgsConstructor; import lombok.Setter; import net.hostsharing.hsadminng.errors.DisplayName; import net.hostsharing.hsadminng.hs.office.bankaccount.HsOfficeBankAccountEntity; -import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerEntity; import net.hostsharing.hsadminng.hs.office.relation.HsOfficeRelationEntity; import net.hostsharing.hsadminng.rbac.rbacobject.RbacObject; import net.hostsharing.hsadminng.rbac.rbacdef.RbacView; import net.hostsharing.hsadminng.rbac.rbacdef.RbacView.SQL; import net.hostsharing.hsadminng.stringify.Stringify; import net.hostsharing.hsadminng.stringify.Stringifyable; +import org.hibernate.Hibernate; import org.hibernate.annotations.GenericGenerator; import jakarta.persistence.Column; @@ -54,7 +54,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @NoArgsConstructor @AllArgsConstructor @DisplayName("Debitor") -public class HsOfficeDebitorEntity implements RbacObject, Stringifyable { +public class HsOfficeDebitorEntity implements RbacObject, Stringifyable { public static final String DEBITOR_NUMBER_TAG = "D-"; public static final String TWO_DECIMAL_DIGITS = "^([0-9]{2})$"; @@ -107,6 +107,16 @@ public class HsOfficeDebitorEntity implements RbacObject, Stringifyable { @Column(name = "defaultprefix", columnDefinition = "char(3) not null") private String defaultPrefix; + @Override + public HsOfficeDebitorEntity load() { + RbacObject.super.load(); + debitorRel.load(); + if (refundBankAccount != null) { + refundBankAccount.load(); + } + return this; + } + private String getDebitorNumberString() { return ofNullable(partnerNumber) .filter(partner -> debitorNumberSuffix != null) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipEntity.java index f71408a7..fe55f653 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipEntity.java @@ -61,7 +61,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @NoArgsConstructor @AllArgsConstructor @DisplayName("Membership") -public class HsOfficeMembershipEntity implements RbacObject, Stringifyable { +public class HsOfficeMembershipEntity implements RbacObject, Stringifyable { public static final String MEMBER_NUMBER_TAG = "M-"; public static final String TWO_DECIMAL_DIGITS = "^([0-9]{2})$"; @@ -99,6 +99,13 @@ public class HsOfficeMembershipEntity implements RbacObject, Stringifyable { @Enumerated(EnumType.STRING) private HsOfficeMembershipStatus status; + @Override + public HsOfficeMembershipEntity load() { + RbacObject.super.load(); + partner.load(); + return this; + } + public void setValidFrom(final LocalDate validFrom) { setValidity(toPostgresDateRange(validFrom, getValidTo())); } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerDetailsEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerDetailsEntity.java index 49ba01c0..4935f591 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerDetailsEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerDetailsEntity.java @@ -26,7 +26,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @NoArgsConstructor @AllArgsConstructor @DisplayName("PartnerDetails") -public class HsOfficePartnerDetailsEntity implements RbacObject, Stringifyable { +public class HsOfficePartnerDetailsEntity implements RbacObject, Stringifyable { private static Stringify stringify = stringify( HsOfficePartnerDetailsEntity.class, diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java index 30d45bf7..b3c35880 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java @@ -40,7 +40,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @NoArgsConstructor @AllArgsConstructor @DisplayName("Partner") -public class HsOfficePartnerEntity implements Stringifyable, RbacObject { +public class HsOfficePartnerEntity implements Stringifyable, RbacObject { public static final String PARTNER_NUMBER_TAG = "P-"; @@ -66,11 +66,11 @@ public class HsOfficePartnerEntity implements Stringifyable, RbacObject { @Column(name = "partnernumber", columnDefinition = "numeric(5) not null") private Integer partnerNumber; - @ManyToOne(cascade = { PERSIST, MERGE, REFRESH, DETACH }, optional = false) + @ManyToOne(cascade = { PERSIST, MERGE, REFRESH, DETACH }, optional = false, fetch = FetchType.EAGER) @JoinColumn(name = "partnerreluuid", nullable = false) private HsOfficeRelationEntity partnerRel; - @ManyToOne(cascade = { PERSIST, MERGE, REFRESH, DETACH }, optional = true) + @ManyToOne(cascade = { PERSIST, MERGE, REFRESH, DETACH }, optional = true, fetch = FetchType.EAGER) @JoinColumn(name = "detailsuuid") @NotFound(action = NotFoundAction.IGNORE) private HsOfficePartnerDetailsEntity details; diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepository.java b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepository.java index e32abb12..7e33899f 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepository.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepository.java @@ -31,13 +31,11 @@ public interface HsOfficePartnerRepository extends Repository, Stringifyable { private static Stringify toString = stringify(HsOfficePersonEntity.class, "person") .withProp(Fields.personType, HsOfficePersonEntity::getPersonType) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java index 92e6ab0c..fd40ae17 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationEntity.java @@ -75,6 +75,15 @@ public class HsOfficeRelationEntity implements RbacObject, Stringifyable { @Column(name = "mark") private String mark; + @Override + public RbacObject load() { + RbacObject.super.load(); + anchor.load(); + holder.load(); + contact.load(); + return this; + } + @Override public String toString() { return toString.apply(this); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateEntity.java index ad3bf25a..7b0f9121 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateEntity.java @@ -40,7 +40,7 @@ import static net.hostsharing.hsadminng.stringify.Stringify.stringify; @NoArgsConstructor @AllArgsConstructor @DisplayName("SEPA-Mandate") -public class HsOfficeSepaMandateEntity implements Stringifyable, RbacObject { +public class HsOfficeSepaMandateEntity implements Stringifyable, RbacObject { private static Stringify stringify = stringify(HsOfficeSepaMandateEntity.class) .withProp(e -> e.getBankAccount().getIban()) diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/rbacobject/RbacObject.java b/src/main/java/net/hostsharing/hsadminng/rbac/rbacobject/RbacObject.java index 80927b61..31e9a85c 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/rbacobject/RbacObject.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/rbacobject/RbacObject.java @@ -1,10 +1,18 @@ package net.hostsharing.hsadminng.rbac.rbacobject; +import org.hibernate.Hibernate; + import java.util.UUID; -public interface RbacObject { +public interface RbacObject> { UUID getUuid(); int getVersion(); + + default T load() { + Hibernate.initialize(this); + //noinspection unchecked + return (T) this; + }; } diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/test/cust/TestCustomerEntity.java b/src/main/java/net/hostsharing/hsadminng/rbac/test/cust/TestCustomerEntity.java index 5fe2aad4..391a82a6 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/test/cust/TestCustomerEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/test/cust/TestCustomerEntity.java @@ -24,7 +24,7 @@ import static net.hostsharing.hsadminng.rbac.rbacdef.RbacView.rbacViewFor; @Setter @NoArgsConstructor @AllArgsConstructor -public class TestCustomerEntity implements RbacObject { +public class TestCustomerEntity implements RbacObject { @Id @GeneratedValue diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/test/dom/TestDomainEntity.java b/src/main/java/net/hostsharing/hsadminng/rbac/test/dom/TestDomainEntity.java index 167618ad..1dde65d7 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/test/dom/TestDomainEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/test/dom/TestDomainEntity.java @@ -27,7 +27,7 @@ import static net.hostsharing.hsadminng.rbac.rbacdef.RbacView.rbacViewFor; @Setter @NoArgsConstructor @AllArgsConstructor -public class TestDomainEntity implements RbacObject { +public class TestDomainEntity implements RbacObject { @Id @GeneratedValue diff --git a/src/main/java/net/hostsharing/hsadminng/rbac/test/pac/TestPackageEntity.java b/src/main/java/net/hostsharing/hsadminng/rbac/test/pac/TestPackageEntity.java index c7161064..5de98a64 100644 --- a/src/main/java/net/hostsharing/hsadminng/rbac/test/pac/TestPackageEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/rbac/test/pac/TestPackageEntity.java @@ -27,7 +27,7 @@ import static net.hostsharing.hsadminng.rbac.rbacdef.RbacView.rbacViewFor; @Setter @NoArgsConstructor @AllArgsConstructor -public class TestPackageEntity implements RbacObject { +public class TestPackageEntity implements RbacObject { @Id @GeneratedValue diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java index c24a88d3..540fd2c7 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java @@ -287,7 +287,10 @@ class HsOfficeBankAccountControllerAcceptanceTest extends ContextBasedTestWithCl .statusCode(204); // @formatter:on // then the given bankaccount is still there - assertThat(bankAccountRepo.findByUuid(givenBankAccount.getUuid())).isEmpty(); + jpaAttempt.transacted(() -> { + context("superuser-alex@hostsharing.net", null); + assertThat(bankAccountRepo.findByUuid(givenBankAccount.getUuid())).isEmpty(); + }).assertSuccessful(); } @Test diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java index 425d39ab..2d171fcc 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactControllerAcceptanceTest.java @@ -309,7 +309,10 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .statusCode(204); // @formatter:on // then the given contact is gone - assertThat(contactRepo.findByUuid(givenContact.getUuid())).isEmpty(); + jpaAttempt.transacted(() -> { + context("superuser-alex@hostsharing.net", null); + assertThat(contactRepo.findByUuid(givenContact.getUuid())).isEmpty(); + }).assertSuccessful(); } @Test @@ -326,7 +329,10 @@ class HsOfficeContactControllerAcceptanceTest extends ContextBasedTestWithCleanu .statusCode(204); // @formatter:on // then the given contact is still there - assertThat(contactRepo.findByUuid(givenContact.getUuid())).isEmpty(); + jpaAttempt.transacted(() -> { + context("superuser-alex@hostsharing.net", null); + assertThat(contactRepo.findByUuid(givenContact.getUuid())).isEmpty(); + }).assertSuccessful(); } @Test diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java index 0c9215f9..8306ac20 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java @@ -119,7 +119,7 @@ class HsOfficeCoopAssetsTransactionRepositoryIntegrationTest extends ContextBase private void assertThatCoopAssetsTransactionIsPersisted(final HsOfficeCoopAssetsTransactionEntity saved) { final var found = coopAssetsTransactionRepo.findByUuid(saved.getUuid()); - assertThat(found).isNotEmpty().get().usingRecursiveComparison().isEqualTo(saved); + assertThat(found).isNotEmpty().get().extracting(HsOfficeCoopAssetsTransactionEntity::toString).isEqualTo(saved.toString()); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionRepositoryIntegrationTest.java index e6163cd4..afe48f0e 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionRepositoryIntegrationTest.java @@ -118,7 +118,7 @@ class HsOfficeCoopSharesTransactionRepositoryIntegrationTest extends ContextBase private void assertThatCoopSharesTransactionIsPersisted(final HsOfficeCoopSharesTransactionEntity saved) { final var found = coopSharesTransactionRepo.findByUuid(saved.getUuid()); - assertThat(found).isNotEmpty().get().usingRecursiveComparison().isEqualTo(saved); + assertThat(found).isNotEmpty().get().extracting(HsOfficeCoopSharesTransactionEntity::toString).isEqualTo(saved.toString()); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java index 1408a87d..ff315651 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java @@ -113,38 +113,6 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "debitorNumber": 1000111, "debitorNumberSuffix": 11, - "partner": { - "partnerNumber": 10001, - "partnerRel": { - "anchor": { - "personType": "LEGAL_PERSON", - "tradeName": "Hostsharing eG", - "givenName": null, - "familyName": null - }, - "holder": { - "personType": "LEGAL_PERSON", - "tradeName": "First GmbH", - "givenName": null, - "familyName": null - }, - "type": "PARTNER", - "mark": null, - "contact": { - "caption": "first contact", - "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": { "phone_office": "+49 123 1234567" } - } - }, - "details": { - "registrationOffice": "Hamburg", - "registrationNumber": "RegNo123456789", - "birthName": null, - "birthPlace": null, - "birthday": null, - "dateOfDeath": null - } - }, "billable": true, "vatId": null, "vatCountryCode": null, @@ -168,21 +136,6 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "debitorNumber": 1000212, "debitorNumberSuffix": 12, - "partner": { - "partnerNumber": 10002, - "partnerRel": { - "anchor": {"tradeName": "Hostsharing eG"}, - "holder": {"tradeName": "Second e.K."}, - "type": "PARTNER", - "contact": { - "emailAddresses": { "main": "contact-admin@secondcontact.example.com" } - } - }, - "details": { - "registrationOffice": "Hamburg", - "registrationNumber": "RegNo123456789" - } - }, "billable": true, "vatId": null, "vatCountryCode": null, @@ -202,21 +155,6 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "debitorNumber": 1000313, "debitorNumberSuffix": 13, - "partner": { - "partnerNumber": 10003, - "partnerRel": { - "anchor": {"tradeName": "Hostsharing eG"}, - "holder": {"tradeName": "Third OHG"}, - "type": "PARTNER", - "contact": { - "emailAddresses": { "main": "contact-admin@thirdcontact.example.com" } - } - }, - "details": { - "registrationOffice": "Hamburg", - "registrationNumber": "RegNo123456789" - } - }, "billable": true, "vatId": null, "vatCountryCode": null, @@ -357,7 +295,6 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu .contentType(ContentType.JSON) .body("uuid", isUuidValid()) .body("debitorRel.contact.caption", is(givenContact.getCaption())) - .body("partner.partnerRel.holder.tradeName", is(givenPartner.getPartnerRel().getHolder().getTradeName())) .body("vatId", equalTo(null)) .body("vatCountryCode", equalTo(null)) .body("vatBusiness", equalTo(false)) @@ -471,25 +408,6 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "debitorNumber": 1000111, "debitorNumberSuffix": 11, - "partner": { - "partnerNumber": 10001, - "partnerRel": { - "anchor": { "personType": "LEGAL_PERSON", "tradeName": "Hostsharing eG"}, - "holder": { "personType": "LEGAL_PERSON", "tradeName": "First GmbH"}, - "type": "PARTNER", - "mark": null, - "contact": { - "caption": "first contact", - "postalAddress": "Vorname Nachname\\nStraße Hnr\\nPLZ Stadt", - "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, - "phoneNumbers": { "phone_office": "+49 123 1234567" } - } - }, - "details": { - "registrationOffice": "Hamburg", - "registrationNumber": "RegNo123456789" - } - }, "billable": true, "vatBusiness": true, "vatReverseCharge": false, @@ -583,24 +501,6 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "debitorNumber": 10004${debitorNumberSuffix}, "debitorNumberSuffix": ${debitorNumberSuffix}, - "partner": { - "partnerNumber": 10004, - "partnerRel": { - "anchor": { "tradeName": "Hostsharing eG" }, - "holder": { "tradeName": "Fourth eG" }, - "type": "PARTNER", - "mark": null, - "contact": { "caption": "fourth contact" } - }, - "details": { - "registrationOffice": "Hamburg", - "registrationNumber": "RegNo123456789", - "birthName": null, - "birthPlace": null, - "birthday": null, - "dateOfDeath": null - } - }, "billable": true, "vatId": "VAT222222", "vatCountryCode": "AA", @@ -609,22 +509,24 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu "defaultPrefix": "for" } """ - .replace("${debitorNumberSuffix}", givenDebitor.getDebitorNumberSuffix().toString())) + .replace("${debitorNumberSuffix}", givenDebitor.getDebitorNumberSuffix())) ); // @formatter:on // finally, the debitor is actually updated - context.define("superuser-alex@hostsharing.net"); - assertThat(debitorRepo.findByUuid(givenDebitor.getUuid())).isPresent().get() - .matches(debitor -> { - assertThat(debitor.getDebitorRel().getHolder().getTradeName()) - .isEqualTo(givenDebitor.getDebitorRel().getHolder().getTradeName()); - assertThat(debitor.getDebitorRel().getContact().getCaption()).isEqualTo("fourth contact"); - assertThat(debitor.getVatId()).isEqualTo("VAT222222"); - assertThat(debitor.getVatCountryCode()).isEqualTo("AA"); - assertThat(debitor.isVatBusiness()).isEqualTo(true); - return true; - }); + jpaAttempt.transacted(() -> { + context.define("superuser-alex@hostsharing.net"); + assertThat(debitorRepo.findByUuid(givenDebitor.getUuid())).isPresent().get() + .matches(debitor -> { + assertThat(debitor.getDebitorRel().getHolder().getTradeName()) + .isEqualTo(givenDebitor.getDebitorRel().getHolder().getTradeName()); + assertThat(debitor.getDebitorRel().getContact().getCaption()).isEqualTo("fourth contact"); + assertThat(debitor.getVatId()).isEqualTo("VAT222222"); + assertThat(debitor.getVatCountryCode()).isEqualTo("AA"); + assertThat(debitor.isVatBusiness()).isEqualTo(true); + return true; + }); + }).assertSuccessful(); } @Test @@ -721,6 +623,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu final var givenPartner = partnerRepo.findPartnerByOptionalNameLike("Fourth").get(0); final var givenContact = contactRepo.findContactByOptionalCaptionLike("fourth contact").get(0); final var newDebitor = HsOfficeDebitorEntity.builder() + .partnerNumber(givenPartner.getPartnerNumber()) .debitorNumberSuffix(nextDebitorSuffix()) .billable(true) .debitorRel( diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java index e79644cf..4de44aa3 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java @@ -89,6 +89,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean // when final var result = attempt(em, () -> { final var newDebitor = HsOfficeDebitorEntity.builder() + .partnerNumber(10001) .debitorNumberSuffix("21") .debitorRel(HsOfficeRelationEntity.builder() .type(HsOfficeRelationType.DEBITOR) @@ -99,7 +100,8 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean .defaultPrefix("abc") .billable(false) .build(); - return toCleanup(debitorRepo.save(newDebitor)); + final HsOfficeDebitorEntity entity = debitorRepo.save(newDebitor); + return toCleanup(entity.load()); }); // then @@ -156,6 +158,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenDebitorPerson = one(personRepo.findPersonByOptionalNameLike("Fourth eG")); final var givenContact = one(contactRepo.findContactByOptionalCaptionLike("fourth contact")); final var newDebitor = HsOfficeDebitorEntity.builder() + .partnerNumber(10001) .debitorNumberSuffix("22") .debitorRel(HsOfficeRelationEntity.builder() .type(HsOfficeRelationType.DEBITOR) @@ -317,7 +320,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean assertThatDebitorIsVisibleForUserWithRole( givenDebitor, - "hs_office_relation#FourtheG-with-DEBITOR-FourtheG:ADMIN", true); + "hs_office_relation#FourtheG-with-DEBITOR-FourtheG:ADMIN"); final var givenNewPartnerPerson = one(personRepo.findPersonByOptionalNameLike("First")); final var givenNewBillingPerson = one(personRepo.findPersonByOptionalNameLike("Firby")); final var givenNewContact = one(contactRepo.findContactByOptionalCaptionLike("sixth contact")); @@ -339,14 +342,15 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean givenDebitor.setVatId(givenNewVatId); givenDebitor.setVatCountryCode(givenNewVatCountryCode); givenDebitor.setVatBusiness(givenNewVatBusiness); - return toCleanup(debitorRepo.save(givenDebitor)); + final HsOfficeDebitorEntity entity = debitorRepo.save(givenDebitor); + return toCleanup(entity.load()); }); // then result.assertSuccessful(); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "global#global:ADMIN", true); + "global#global:ADMIN"); // ... partner role was reassigned: assertThatDebitorIsNotVisibleForUserWithRole( @@ -354,7 +358,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean "hs_office_relation#FourtheG-with-DEBITOR-FourtheG:ADMIN"); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "hs_office_relation#FirstGmbH-with-DEBITOR-FirbySusan:AGENT", true); + "hs_office_relation#FirstGmbH-with-DEBITOR-FirbySusan:AGENT"); // ... contact role was reassigned: assertThatDebitorIsNotVisibleForUserWithRole( @@ -362,7 +366,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean "hs_office_contact#fifthcontact:ADMIN"); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "hs_office_contact#sixthcontact:ADMIN", false); + "hs_office_contact#sixthcontact:ADMIN"); // ... bank-account role was reassigned: assertThatDebitorIsNotVisibleForUserWithRole( @@ -370,7 +374,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean "hs_office_bankaccount#DE02200505501015871393:ADMIN"); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "hs_office_bankaccount#DE02120300000000202051:ADMIN", true); + "hs_office_bankaccount#DE02120300000000202051:ADMIN"); } @Test @@ -380,27 +384,27 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "fifth contact", null, "fig"); assertThatDebitorIsVisibleForUserWithRole( givenDebitor, - "hs_office_relation#FourtheG-with-DEBITOR-FourtheG:ADMIN", true); - assertThatDebitorActuallyInDatabase(givenDebitor, true); + "hs_office_relation#FourtheG-with-DEBITOR-FourtheG:ADMIN"); + assertThatDebitorActuallyInDatabase(givenDebitor); final var givenNewBankAccount = one(bankAccountRepo.findByOptionalHolderLike("first")); // when final var result = jpaAttempt.transacted(() -> { context("superuser-alex@hostsharing.net"); givenDebitor.setRefundBankAccount(givenNewBankAccount); - return toCleanup(debitorRepo.save(givenDebitor)); + return toCleanup(debitorRepo.save(givenDebitor).load()); }); // then result.assertSuccessful(); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "global#global:ADMIN", true); + "global#global:ADMIN"); // ... bank-account role was assigned: assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "hs_office_bankaccount#DE02120300000000202051:ADMIN", true); + "hs_office_bankaccount#DE02120300000000202051:ADMIN"); } @Test @@ -410,21 +414,21 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "fifth contact", "Fourth", "fih"); assertThatDebitorIsVisibleForUserWithRole( givenDebitor, - "hs_office_relation#HostsharingeG-with-PARTNER-FourtheG:AGENT", true); - assertThatDebitorActuallyInDatabase(givenDebitor, true); + "hs_office_relation#HostsharingeG-with-PARTNER-FourtheG:AGENT"); + assertThatDebitorActuallyInDatabase(givenDebitor); // when final var result = jpaAttempt.transacted(() -> { context("superuser-alex@hostsharing.net"); givenDebitor.setRefundBankAccount(null); - return toCleanup(debitorRepo.save(givenDebitor)); + return toCleanup(debitorRepo.save(givenDebitor).load()); }); // then result.assertSuccessful(); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "global#global:ADMIN", true); + "global#global:ADMIN"); // ... bank-account role was removed from previous bank-account admin: assertThatDebitorIsNotVisibleForUserWithRole( @@ -439,8 +443,8 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "eighth", "Fourth", "eig"); assertThatDebitorIsVisibleForUserWithRole( givenDebitor, - "hs_office_relation#HostsharingeG-with-PARTNER-FourtheG:AGENT", true); - assertThatDebitorActuallyInDatabase(givenDebitor, true); + "hs_office_relation#HostsharingeG-with-PARTNER-FourtheG:AGENT"); + assertThatDebitorActuallyInDatabase(givenDebitor); // when final var result = jpaAttempt.transacted(() -> { @@ -459,16 +463,17 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean // given context("superuser-alex@hostsharing.net"); final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "ninth", "Fourth", "nin"); - assertThatDebitorActuallyInDatabase(givenDebitor, true); + assertThatDebitorActuallyInDatabase(givenDebitor); assertThatDebitorIsVisibleForUserWithRole( givenDebitor, - "hs_office_contact#ninthcontact:ADMIN", false); + "hs_office_contact#ninthcontact:ADMIN"); // when final var result = jpaAttempt.transacted(() -> { context("superuser-alex@hostsharing.net", "hs_office_contact#ninthcontact:ADMIN"); givenDebitor.setVatId("NEW-VAT-ID"); - return toCleanup(debitorRepo.save(givenDebitor)); + final HsOfficeDebitorEntity entity = debitorRepo.save(givenDebitor); + return toCleanup(entity.load()); }); // then @@ -478,7 +483,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean "Unable to find net.hostsharing.hsadminng.hs.office.bankaccount.HsOfficeBankAccountEntity with id "); } - private void assertThatDebitorActuallyInDatabase(final HsOfficeDebitorEntity saved, final boolean withPartner) { + private void assertThatDebitorActuallyInDatabase(final HsOfficeDebitorEntity saved) { final var found = debitorRepo.findByUuid(saved.getUuid()); assertThat(found).isNotEmpty(); found.ifPresent(foundEntity -> { @@ -492,11 +497,10 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean private void assertThatDebitorIsVisibleForUserWithRole( final HsOfficeDebitorEntity entity, - final String assumedRoles, - final boolean withPartner) { + final String assumedRoles) { jpaAttempt.transacted(() -> { context("superuser-alex@hostsharing.net", assumedRoles); - assertThatDebitorActuallyInDatabase(entity, withPartner); + assertThatDebitorActuallyInDatabase(entity); }).assertSuccessful(); } @@ -605,11 +609,13 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final String defaultPrefix) { return jpaAttempt.transacted(() -> { context("superuser-alex@hostsharing.net"); - final var givenPartnerPerson = one(personRepo.findPersonByOptionalNameLike(partnerName)); + final var givenPartner = one(partnerRepo.findPartnerByOptionalNameLike(partnerName)); + final var givenPartnerPerson = givenPartner.getPartnerRel().getHolder(); final var givenContact = one(contactRepo.findContactByOptionalCaptionLike(contactCaption)); final var givenBankAccount = bankAccountHolder != null ? one(bankAccountRepo.findByOptionalHolderLike(bankAccountHolder)) : null; final var newDebitor = HsOfficeDebitorEntity.builder() + .partnerNumber(givenPartner.getPartnerNumber()) .debitorNumberSuffix("20") .debitorRel(HsOfficeRelationEntity.builder() .type(HsOfficeRelationType.DEBITOR) @@ -622,7 +628,8 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean .billable(true) .build(); - return toCleanup(debitorRepo.save(newDebitor)); + final HsOfficeDebitorEntity entity = debitorRepo.save(newDebitor); + return toCleanup(entity.load()); }).assertSuccessful().returnedValue(); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipRepositoryIntegrationTest.java index 701e6651..b1d64b44 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipRepositoryIntegrationTest.java @@ -4,6 +4,7 @@ import io.hypersistence.utils.hibernate.type.range.Range; import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.hs.office.debitor.HsOfficeDebitorRepository; import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerRepository; +import net.hostsharing.hsadminng.rbac.rbacobject.RbacObject; import net.hostsharing.hsadminng.rbac.test.ContextBasedTestWithCleanup; import net.hostsharing.hsadminng.rbac.rbacgrant.RawRbacGrantRepository; import net.hostsharing.hsadminng.rbac.rbacrole.RawRbacRoleRepository; @@ -75,7 +76,7 @@ class HsOfficeMembershipRepositoryIntegrationTest extends ContextBasedTestWithCl .validity(Range.closedInfinite(LocalDate.parse("2020-01-01"))) .membershipFeeBillable(true) .build(); - return toCleanup(membershipRepo.save(newMembership)); + return toCleanup(membershipRepo.save(newMembership).load()); }); // then @@ -202,9 +203,12 @@ class HsOfficeMembershipRepositoryIntegrationTest extends ContextBasedTestWithCl @Test public void globalAdmin_canUpdateValidityOfArbitraryMembership() { // given - context("superuser-alex@hostsharing.net"); - final var givenMembership = givenSomeTemporaryMembership("First", "11"); - assertThatMembershipExistsAndIsAccessibleToCurrentContext(givenMembership); + final var givenMembership = jpaAttempt.transacted(() -> { + context("superuser-alex@hostsharing.net"); + final var tempMembership = givenSomeTemporaryMembership("First", "11"); + assertThatMembershipExistsAndIsAccessibleToCurrentContext(tempMembership); + return tempMembership; + }).assertSuccessful().returnedValue(); final var newValidityEnd = LocalDate.now(); // when @@ -214,13 +218,12 @@ class HsOfficeMembershipRepositoryIntegrationTest extends ContextBasedTestWithCl givenMembership.setValidity(Range.closedOpen( givenMembership.getValidity().lower(), newValidityEnd)); givenMembership.setStatus(HsOfficeMembershipStatus.CANCELLED); - return toCleanup(membershipRepo.save(givenMembership)); + final HsOfficeMembershipEntity entity = membershipRepo.save(givenMembership); + return toCleanup(entity.load()); }); // then result.assertSuccessful(); - - membershipRepo.deleteByUuid(givenMembership.getUuid()); } @Test @@ -363,7 +366,8 @@ class HsOfficeMembershipRepositoryIntegrationTest extends ContextBasedTestWithCl .membershipFeeBillable(true) .build(); - return toCleanup(membershipRepo.save(newMembership)); + final HsOfficeMembershipEntity entity = membershipRepo.save(newMembership); + return toCleanup(entity.load()); }).assertSuccessful().returnedValue(); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java index fa8680f0..1bf30d14 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerControllerAcceptanceTest.java @@ -548,7 +548,7 @@ class HsOfficePartnerControllerAcceptanceTest extends ContextBasedTestWithCleanu .build()) .build(); - return partnerRepo.save(newPartner); + return partnerRepo.save(newPartner).load(); }).assertSuccessful().returnedValue(); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java index 5daf0f8f..2afe3baa 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java @@ -473,7 +473,7 @@ class HsOfficePartnerRepositoryIntegrationTest extends ContextBasedTestWithClean .anchor(givenMandantorPerson) .contact(givenContact) .build(); - relationRepo.save(partnerRel); + relationRepo.save(partnerRel).load(); return partnerRel; } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonControllerAcceptanceTest.java index b193e97c..995df064 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonControllerAcceptanceTest.java @@ -332,7 +332,7 @@ class HsOfficePersonControllerAcceptanceTest extends ContextBasedTestWithCleanup .givenName("Temp Given Name " + RandomStringUtils.randomAlphabetic(10)) .build(); - return personRepo.save(newPerson); + return personRepo.save(newPerson).load(); }).assertSuccessful().returnedValue(); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java index 0792d656..c8304e2f 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java @@ -295,7 +295,8 @@ class HsOfficeRelationRepositoryIntegrationTest extends ContextBasedTestWithClea final var found = relationRepo.findByUuid(saved.getUuid()); assertThat(found).isNotEmpty().get() .isNotSameAs(saved) - .usingRecursiveComparison().ignoringFields("version").isEqualTo(saved); + .extracting(HsOfficeRelationEntity::toString) + .isEqualTo(saved.toString()); } private void assertThatRelationIsVisibleForUserWithRole( diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java index 40d9cee8..77fe2f94 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java @@ -497,7 +497,7 @@ class HsOfficeSepaMandateControllerAcceptanceTest extends ContextBasedTestWithCl return jpaAttempt.transacted(() -> { context.define("superuser-alex@hostsharing.net"); final var givenDebitor = debitorRepo.findDebitorByDebitorNumber(debitorNumber).get(0); - final var givenPartner = partnerRepo.findPartnerByDebitorUuid(givenDebitor.getUuid()); + final var givenPartner = partnerRepo.findPartnerByPartnerNumber(debitorNumber/100); final var bankAccountHolder = ofNullable(givenPartner.getPartnerRel().getHolder().getTradeName()) .orElse(givenPartner.getPartnerRel().getHolder().getFamilyName()); final var givenBankAccount = bankAccountRepo.findByOptionalHolderLike(bankAccountHolder).get(0); -- 2.39.5 From aa9696cfe19e431b49acf82fc8d5552346783882 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 24 Jul 2024 09:57:51 +0200 Subject: [PATCH 05/14] amend tests to lazy-loading references by introducing RbacObject.load() --- .../hs/office/debitor/HsOfficeDebitorEntity.java | 1 - .../hs/office/partner/HsOfficePartnerEntity.java | 7 +++++++ .../office/relation/HsOfficeRelationEntity.java | 2 +- .../db/changelog/1-rbac/1050-rbac-base.sql | 1 + ...fficeBankAccountRepositoryIntegrationTest.java | 2 +- .../HsOfficeContactRepositoryIntegrationTest.java | 2 +- .../HsOfficeDebitorControllerAcceptanceTest.java | 2 +- .../debitor/HsOfficeDebitorEntityUnitTest.java | 1 - ...OfficeMembershipRepositoryIntegrationTest.java | 15 +++++---------- .../HsOfficePartnerRepositoryIntegrationTest.java | 2 +- .../HsOfficePersonControllerAcceptanceTest.java | 5 ++++- .../HsOfficePersonRepositoryIntegrationTest.java | 2 +- ...HsOfficeRelationRepositoryIntegrationTest.java | 4 ++-- ...fficeSepaMandateRepositoryIntegrationTest.java | 4 ++-- .../TestCustomerRepositoryIntegrationTest.java | 2 +- src/test/resources/application.yml | 1 + 16 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java index a759a975..d52a1410 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java @@ -13,7 +13,6 @@ import net.hostsharing.hsadminng.rbac.rbacdef.RbacView; import net.hostsharing.hsadminng.rbac.rbacdef.RbacView.SQL; import net.hostsharing.hsadminng.stringify.Stringify; import net.hostsharing.hsadminng.stringify.Stringifyable; -import org.hibernate.Hibernate; import org.hibernate.annotations.GenericGenerator; import jakarta.persistence.Column; diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java index b3c35880..6f9589f5 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java @@ -75,6 +75,13 @@ public class HsOfficePartnerEntity implements Stringifyable, RbacObject load() { + public HsOfficeRelationEntity load() { RbacObject.super.load(); anchor.load(); holder.load(); diff --git a/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql b/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql index 5722c26e..86a4527d 100644 --- a/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql +++ b/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql @@ -372,6 +372,7 @@ create table RbacPermission op RbacOp not null, opTableName varchar(60) ); +-- TODO.perf: check if these indexes are really useful create index on RbacPermission (objectUuid, op); create index on RbacPermission (opTableName, op); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountRepositoryIntegrationTest.java index d7d07f69..291b8863 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountRepositoryIntegrationTest.java @@ -123,7 +123,7 @@ class HsOfficeBankAccountRepositoryIntegrationTest extends ContextBasedTestWithC private void assertThatBankAccountIsPersisted(final HsOfficeBankAccountEntity saved) { final var found = bankAccountRepo.findByUuid(saved.getUuid()); - assertThat(found).isNotEmpty().get().usingRecursiveComparison().isEqualTo(saved); + assertThat(found).isNotEmpty().get().extracting(Object::toString).isEqualTo(saved.toString()); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactRepositoryIntegrationTest.java index 4e591973..89a03f67 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/contact/HsOfficeContactRepositoryIntegrationTest.java @@ -122,7 +122,7 @@ class HsOfficeContactRepositoryIntegrationTest extends ContextBasedTestWithClean private void assertThatContactIsPersisted(final HsOfficeContactEntity saved) { final var found = contactRepo.findByUuid(saved.getUuid()); - assertThat(found).isNotEmpty().get().usingRecursiveComparison().isEqualTo(saved); + assertThat(found).isNotEmpty().get().extracting(Object::toString).isEqualTo(saved.toString()); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java index ff315651..05f72887 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java @@ -638,7 +638,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu .vatReverseCharge(false) .build(); - return debitorRepo.save(newDebitor); + return debitorRepo.save(newDebitor).load(); }).assertSuccessful().returnedValue(); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java index 193a0d59..5a0d3e03 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java @@ -1,7 +1,6 @@ package net.hostsharing.hsadminng.hs.office.debitor; import net.hostsharing.hsadminng.hs.office.contact.HsOfficeContactEntity; -import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerEntity; import net.hostsharing.hsadminng.hs.office.person.HsOfficePersonEntity; import net.hostsharing.hsadminng.hs.office.person.HsOfficePersonType; import net.hostsharing.hsadminng.hs.office.relation.HsOfficeRelationEntity; diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipRepositoryIntegrationTest.java index b1d64b44..581febd8 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipRepositoryIntegrationTest.java @@ -4,7 +4,6 @@ import io.hypersistence.utils.hibernate.type.range.Range; import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.hs.office.debitor.HsOfficeDebitorRepository; import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerRepository; -import net.hostsharing.hsadminng.rbac.rbacobject.RbacObject; import net.hostsharing.hsadminng.rbac.test.ContextBasedTestWithCleanup; import net.hostsharing.hsadminng.rbac.rbacgrant.RawRbacGrantRepository; import net.hostsharing.hsadminng.rbac.rbacrole.RawRbacRoleRepository; @@ -144,7 +143,7 @@ class HsOfficeMembershipRepositoryIntegrationTest extends ContextBasedTestWithCl private void assertThatMembershipIsPersisted(final HsOfficeMembershipEntity saved) { final var found = membershipRepo.findByUuid(saved.getUuid()); - assertThat(found).isNotEmpty().get().usingRecursiveComparison().isEqualTo(saved); + assertThat(found).isNotEmpty().get().extracting(Object::toString).isEqualTo(saved.toString()) ; } } @@ -203,12 +202,9 @@ class HsOfficeMembershipRepositoryIntegrationTest extends ContextBasedTestWithCl @Test public void globalAdmin_canUpdateValidityOfArbitraryMembership() { // given - final var givenMembership = jpaAttempt.transacted(() -> { - context("superuser-alex@hostsharing.net"); - final var tempMembership = givenSomeTemporaryMembership("First", "11"); - assertThatMembershipExistsAndIsAccessibleToCurrentContext(tempMembership); - return tempMembership; - }).assertSuccessful().returnedValue(); + context("superuser-alex@hostsharing.net"); + final var givenMembership = givenSomeTemporaryMembership("First", "11"); + assertThatMembershipExistsAndIsAccessibleToCurrentContext(givenMembership); final var newValidityEnd = LocalDate.now(); // when @@ -366,8 +362,7 @@ class HsOfficeMembershipRepositoryIntegrationTest extends ContextBasedTestWithCl .membershipFeeBillable(true) .build(); - final HsOfficeMembershipEntity entity = membershipRepo.save(newMembership); - return toCleanup(entity.load()); + return toCleanup(membershipRepo.save(newMembership).load()); }).assertSuccessful().returnedValue(); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java index 2afe3baa..ecf645d7 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerRepositoryIntegrationTest.java @@ -180,7 +180,7 @@ class HsOfficePartnerRepositoryIntegrationTest extends ContextBasedTestWithClean private void assertThatPartnerIsPersisted(final HsOfficePartnerEntity saved) { final var found = partnerRepo.findByUuid(saved.getUuid()); - assertThat(found).isNotEmpty().get().usingRecursiveComparison().isEqualTo(saved); + assertThat(found).isNotEmpty().get().extracting(Object::toString).isEqualTo(saved.toString()); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonControllerAcceptanceTest.java index 995df064..4a136331 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonControllerAcceptanceTest.java @@ -298,7 +298,10 @@ class HsOfficePersonControllerAcceptanceTest extends ContextBasedTestWithCleanup .statusCode(204); // @formatter:on // then the given person is still there - assertThat(personRepo.findByUuid(givenPerson.getUuid())).isEmpty(); + jpaAttempt.transacted(() -> { + context.define("superuser-alex@hostsharing.net"); + assertThat(personRepo.findByUuid(givenPerson.getUuid())).isEmpty(); + }).assertSuccessful(); } @Test diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonRepositoryIntegrationTest.java index efd7064f..b0e1c893 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/person/HsOfficePersonRepositoryIntegrationTest.java @@ -124,7 +124,7 @@ class HsOfficePersonRepositoryIntegrationTest extends ContextBasedTestWithCleanu private void assertThatPersonIsPersisted(final HsOfficePersonEntity saved) { final var found = personRepo.findByUuid(saved.getUuid()); - assertThat(found).isNotEmpty().get().usingRecursiveComparison().isEqualTo(saved); + assertThat(found).isNotEmpty().get().extracting(Object::toString).isEqualTo(saved.toString()); } } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java index c8304e2f..f6807b34 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/relation/HsOfficeRelationRepositoryIntegrationTest.java @@ -158,7 +158,7 @@ class HsOfficeRelationRepositoryIntegrationTest extends ContextBasedTestWithClea private void assertThatRelationIsPersisted(final HsOfficeRelationEntity saved) { final var found = relationRepo.findByUuid(saved.getUuid()); - assertThat(found).isNotEmpty().get().usingRecursiveComparison().isEqualTo(saved); + assertThat(found).isNotEmpty().get().extracting(Object::toString).isEqualTo(saved.toString()); } } @@ -225,7 +225,7 @@ class HsOfficeRelationRepositoryIntegrationTest extends ContextBasedTestWithClea final var result = jpaAttempt.transacted(() -> { context("superuser-alex@hostsharing.net"); givenRelation.setContact(givenContact); - return toCleanup(relationRepo.save(givenRelation)); + return toCleanup(relationRepo.save(givenRelation).load()); }); // then diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateRepositoryIntegrationTest.java index ad7ee76e..d5fdb87d 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateRepositoryIntegrationTest.java @@ -152,7 +152,7 @@ class HsOfficeSepaMandateRepositoryIntegrationTest extends ContextBasedTestWithC private void assertThatSepaMandateIsPersisted(final HsOfficeSepaMandateEntity saved) { final var found = sepaMandateRepo.findByUuid(saved.getUuid()); - assertThat(found).isNotEmpty().get().usingRecursiveComparison().isEqualTo(saved); + assertThat(found).isNotEmpty().get().extracting(Object::toString).isEqualTo(saved.toString()); } } @@ -250,7 +250,7 @@ class HsOfficeSepaMandateRepositoryIntegrationTest extends ContextBasedTestWithC jpaAttempt.transacted(() -> { context("superuser-alex@hostsharing.net"); assertThat(sepaMandateRepo.findByUuid(givenSepaMandate.getUuid())).isNotEmpty().get() - .usingRecursiveComparison().isEqualTo(givenSepaMandate); + .extracting(Object::toString).isEqualTo(givenSepaMandate.toString()); }).assertSuccessful(); } diff --git a/src/test/java/net/hostsharing/hsadminng/rbac/test/cust/TestCustomerRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/rbac/test/cust/TestCustomerRepositoryIntegrationTest.java index ae878a61..04175d04 100644 --- a/src/test/java/net/hostsharing/hsadminng/rbac/test/cust/TestCustomerRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/rbac/test/cust/TestCustomerRepositoryIntegrationTest.java @@ -90,7 +90,7 @@ class TestCustomerRepositoryIntegrationTest extends ContextBasedTest { private void assertThatCustomerIsPersisted(final TestCustomerEntity saved) { final var found = testCustomerRepository.findByUuid(saved.getUuid()); - assertThat(found).isNotEmpty().get().usingRecursiveComparison().isEqualTo(saved); + assertThat(found).isNotEmpty().get().extracting(Object::toString).isEqualTo(saved.toString()); } } diff --git a/src/test/resources/application.yml b/src/test/resources/application.yml index 696dfbd9..293e6f72 100644 --- a/src/test/resources/application.yml +++ b/src/test/resources/application.yml @@ -28,6 +28,7 @@ spring: change-log: classpath:/db/changelog/db.changelog-master.yaml contexts: tc,test,dev +# FIXME: re-activate? #logging: # level: # liquibase: INFO -- 2.39.5 From c1881bb1e935c0ea2bb2e29279caea94cb98094f Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 24 Jul 2024 11:23:39 +0200 Subject: [PATCH 06/14] some more documentation --- .aliases | 3 ++- doc/rbac-performance-analysis.md | 41 +++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/.aliases b/.aliases index cabdb7d4..b5d3cd0e 100644 --- a/.aliases +++ b/.aliases @@ -71,7 +71,8 @@ alias podman-stop='systemctl --user disable --now podman.socket && systemctl --u alias podman-use='export DOCKER_HOST="unix:///run/user/$UID/podman/podman.sock"; export TESTCONTAINERS_RYUK_DISABLED=true' alias gw=gradleWrapper -alias pg-sql-run='docker run --name hsadmin-ng-postgres -e POSTGRES_PASSWORD=password -v COPY etc/postgresql-log-slow-queries.conf:/etc/postgresql/postgresql.conf -p 5432:5432 -d postgres:15.5-bookworm' +alias pg-sql-run='docker run --name hsadmin-ng-postgres -e POSTGRES_PASSWORD=password -p 5432:5432 -d postgres:15.5-bookworm' +alias pg-sql-run-with-query-stats='docker run --name hsadmin-ng-postgres -e POSTGRES_PASSWORD=password -v COPY etc/postgresql-log-slow-queries.conf:/etc/postgresql/postgresql.conf -p 5432:5432 -d postgres:15.5-bookworm' alias pg-sql-stop='docker stop hsadmin-ng-postgres' alias pg-sql-start='docker container start hsadmin-ng-postgres' alias pg-sql-remove='docker rm hsadmin-ng-postgres' diff --git a/doc/rbac-performance-analysis.md b/doc/rbac-performance-analysis.md index ad1db594..c306bd28 100644 --- a/doc/rbac-performance-analysis.md +++ b/doc/rbac-performance-analysis.md @@ -213,14 +213,7 @@ VACUUM ANALYZE; This did not improve the performance. -### Improving Indexes - -The sequentiap - -create index on RbacPermission (objectUuid, op); -create index on RbacPermission (opTableName, op); - -### +### Improving Joins + Indexes We were suspicious about the sequential scan over all `rbacpermission` rows which was done by PostgreSQL to execute a HashJoin strategy. Turning off that strategy by @@ -243,7 +236,14 @@ did not improve the performance though. The HashJoin was actually still applied, The HashJoin strategy could be great if the hash-map could be kept for multiple invocations. But during an import process, of course, there are always new rows in the underlying table and the hash-map would be outdated immediately. +Also creating indexes which should suppor the RBAC query, like the following, did not improve performance: +```SQL +create index on RbacPermission (objectUuid, op); +create index on RbacPermission (opTableName, op); +``` + +### ### LAZY loading for Relation.anchorPerson/.holderPerson/ @@ -251,11 +251,24 @@ The slowest query now was fetching Relations joined with Contact, Anchor-Person We changed these mappings from `EAGER` (default) to `LAZY` to `@ManyToOne(fetch = FetchType.LAZY)` and got this result: -| query | calls | total (min) | mean (ms) | -|-------|-------|-------------|-----------| -| select hore1_0.uuid,hore1_0.anchoruuid,hore1_0.contactuuid,hore1_0.holderuuid,hore1_0.mark,hore1_0.type,hore1_0.version from hs_office_relation_rv hore1_0 where hore1_0.uuid=$1 | 517 | 5 | 565 | -| select hope1_0.uuid,hope1_0.familyname,hope1_0.givenname,hope1_0.persontype,hope1_0.salutation,hope1_0.title,hope1_0.tradename,hope1_0.version from hs_office_person_rv hope1_0 where hope1_0.uuid=$1 | 1015 | 4 | 240 | -| select hoce1_0.uuid,hoce1_0.caption,hoce1_0.emailaddresses,hoce1_0.phonenumbers,hoce1_0.postaladdress,hoce1_0.version from hs_office_contact_rv hoce1_0 where hoce1_0.uuid=$1 | 497 | 2 | 235 -select * from isGranted(array[granteeId], grantedId) | 44613 | 1 | 1 | +| query | calls | total (min) | mean (ms) | +|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-------|-------------|----------| +| select hope1_0.uuid,hope1_0.familyname,hope1_0.givenname,hope1_0.persontype,hope1_0.salutation,hope1_0.title,hope1_0.tradename,hope1_0.version from public.hs_office_person_rv hope1_0 where hope1_0.uuid=$1 | 1015 | 4 | 238 | +| select hore1_0.uuid,hore1_0.anchoruuid,hore1_0.contactuuid,hore1_0.holderuuid,hore1_0.mark,hore1_0.type,hore1_0.version from public.hs_office_relation_rv hore1_0 where hore1_0.uuid=$1 | 517 | 4 | 439 | +| select hoce1_0.uuid,hoce1_0.caption,hoce1_0.emailaddresses,hoce1_0.phonenumbers,hoce1_0.postaladdress,hoce1_0.version from public.hs_office_contact_rv hoce1_0 where hoce1_0.uuid=$1 | 497 | 2 | 213 | +| call grantRoleToRole(roleUuid, superRoleUuid, superRoleDesc.assumed) | 31316 | 0 | 1 | +| select * from isGranted(array[granteeId], grantedId) | 44613 | 0 | 0 | +| call buildRbacSystemForHsHostingAsset(NEW) | 2258 | 0 | 7 | +| insert into public.hs_hosting_asset_rv (alarmcontactuuid,assignedtoassetuuid,bookingitemuuid,caption,config,identifier,parentassetuuid,type,version,uuid) values ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10) | 2207 | 0 | 7 | +| insert into hs_hosting_asset (alarmcontactuuid, version, bookingitemuuid, type, parentassetuuid, assignedtoassetuuid, config, uuid, identifier, caption) values (new.alarmcontactuuid, new. version, new. bookingitemuuid, new. type, new. parentassetuuid, new. assignedtoassetuuid, new. config, new. uuid, new. identifier, new. caption) returning * | 2207 | 0 | 7 | +| with recursive grants as ( select descendantUuid, ascendantUuid from RbacGrants where descendantUuid = grantedId union all select ""grant"".descendantUuid, ""grant"".ascendantUuid from RbacGrants ""grant"" inner join grants recur on recur.ascendantUuid = ""grant"".descendantUuid ) select exists ( select $3 from grants where ascendantUuid = any(granteeIds) ) or grantedId = any(granteeIds) | 47538 | 0 | 0 | + insert into public.hs_office_relation_rv (anchoruuid,contactuuid,holderuuid,mark,type,version,uuid) values ($1,$2,$3,$4,$5,$6,$7) | 1261 | 0 | 8 | +| insert into hs_office_relation (uuid, version, anchoruuid, holderuuid, contactuuid, type, mark) values (new.uuid, new. version, new. anchoruuid, new. holderuuid, new. contactuuid, new. type, new. mark) returning * | 1261 | 0 | 8 | +| call buildRbacSystemForHsOfficeRelation(NEW) | 1276 | 0 | 7 | +| insert into public.hs_booking_item_rv (caption,parentitemuuid,projectuuid,resources,type,validity,version,uuid) values ($1,$2,$3,$4,$5,$6,$7,$8) | 926 | 0 | 7 | +| insert into hs_booking_item (resources, version, projectuuid, type, parentitemuuid, validity, uuid, caption) values (new.resources, new. version, new. projectuuid, new. type, new. parentitemuuid, new. validity, new. uuid, new. caption) returning * | 926 | 0 | 7 | + insert into RbacGrants (grantedByTriggerOf, ascendantuuid, descendantUuid, assumed) values (currentTriggerObjectUuid(), superRoleId, subRoleId, doAssume) on conflict do nothing | 40472 | 0 | 0 | + + Now, finally, the total runtime of the import was down to 12 minutes. -- 2.39.5 From 7207accea5cfb4fcbdb301bc4a2700f121fffa85 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 24 Jul 2024 12:07:34 +0200 Subject: [PATCH 07/14] improve documentation --- doc/rbac-performance-analysis.md | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/doc/rbac-performance-analysis.md b/doc/rbac-performance-analysis.md index c306bd28..3747eb29 100644 --- a/doc/rbac-performance-analysis.md +++ b/doc/rbac-performance-analysis.md @@ -183,7 +183,7 @@ Limit (cost=6549.08..6549.35 rows=54 width=16) Index Cond: ((objecttable = 'hs_hosting_asset'::text) AND (uuid = perm.objectuuid)) ``` -### Office-Relations-Query +### Office-Relation-Query ```SQL SELECT hore1_0.uuid,a1_0.uuid,a1_0.familyname,a1_0.givenname,a1_0.persontype,a1_0.salutation,a1_0.title,a1_0.tradename,a1_0.version,c1_0.uuid,c1_0.caption,c1_0.emailaddresses,c1_0.phonenumbers,c1_0.postaladdress,c1_0.version,h1_0.uuid,h1_0.familyname,h1_0.givenname,h1_0.persontype,h1_0.salutation,h1_0.title,h1_0.tradename,h1_0.version,hore1_0.mark,hore1_0.type,hore1_0.version @@ -194,12 +194,14 @@ SELECT hore1_0.uuid,a1_0.uuid,a1_0.familyname,a1_0.givenname,a1_0.persontype,a1_ WHERE hore1_0.uuid=$1 ``` -That query into the `hs_office_relation_rv`-table is presumably the query to determine the partner of a debitor, which requires two of such queries each (Debitor -> Debitor-Relation -> Debitor-Anchor == Partner-Holder -> Partner-Relation -> Partner). +That query on the `hs_office_relation_rv`-table joins the three references anchor-person, holder-person and contact. + ### Total-Query-Time > Total-Import-Runtime That both queries total up to more than the runtime of the import-process is most likely due to internal parallel query processing. + ## Attempts to Mitigate the Problem ### VACUUM ANALYZE @@ -245,6 +247,27 @@ create index on RbacPermission (opTableName, op); ### +The import took 21mins with these statistics: + +| query | calls | total_m | mean_ms | +|-------|-------|---------|---------| +| select hore1_0.uuid,a1_0.uuid,a1_0.familyname,a1_0.givenname,a1_0.persontype,a1_0.salutation,a1_0.title,a1_0.tradename,a1_0.version,c1_0.uuid,c1_0.caption,c1_0.emailaddresses,c1_0.phonenumbers,c1_0.postaladdress, c1_0.version,h1_0.uuid,h1_0.familyname,h1_0.givenname,h1_0.persontype,h1_0.salutation,h1_0.title,h1_0.tradename,h1_0.version,hore1_0.mark,hore1_0.type,hore1_0.version from public.hs_office_relation_rv hore1_0 left join public.hs_office_person_rv a1_0 on a1_0.uuid=hore1_0.anchoruuid left join public.hs_office_contact_rv c1_0 on c1_0.uuid=hore1_0.contactuuid left join public.hs_office_person_rv h1_0 on h1_0.uuid=hore1_0.holderuuid where hore1_0.uuid=$1 | 517 | 11 | 1282 | +| select hope1_0.uuid,hope1_0.familyname,hope1_0.givenname,hope1_0.persontype,hope1_0.salutation,hope1_0.title,hope1_0.tradename,hope1_0.version from public.hs_office_person_rv hope1_0 where hope1_0.uuid=$1 | 973 | 4 | 254 | +| select hoce1_0.uuid,hoce1_0.caption,hoce1_0.emailaddresses,hoce1_0.phonenumbers,hoce1_0.postaladdress,hoce1_0.version from public.hs_office_contact_rv hoce1_0 where hoce1_0.uuid=$1 | 973 | 4 | 253 | +| call grantRoleToRole(roleUuid, superRoleUuid, superRoleDesc.assumed) | 31316 | 0 | 1 | +| call buildRbacSystemForHsHostingAsset(NEW) | 2258 | 0 | 7 | +| select * from isGranted(array[granteeId], grantedId) | 44613 | 0 | 0 | +| insert into public.hs_hosting_asset_rv (alarmcontactuuid,assignedtoassetuuid,bookingitemuuid,caption,config,identifier,parentassetuuid,type,version,uuid) values ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10) | 2207 | 0 | 7 | +| insert into hs_hosting_asset (alarmcontactuuid, version, bookingitemuuid, type, parentassetuuid, assignedtoassetuuid, config, uuid, identifier, caption) values (new.alarmcontactuuid, new. version, new. bookingitemuuid, new. type, new. parentassetuuid, new. assignedtoassetuuid, new. config, new. uuid, new. identifier, new. caption) returning * | 2207 | 0 | 7 | +| insert into public.hs_office_relation_rv (anchoruuid,contactuuid,holderuuid,mark,type,version,uuid) values ($1,$2,$3,$4,$5,$6,$7) | 1261 | 0 | 9 | +| insert into hs_office_relation (uuid, version, anchoruuid, holderuuid, contactuuid, type, mark) values (new.uuid, new. version, new. anchoruuid, new. holderuuid, new. contactuuid, new. type, new. mark) returning * | 1261 | 0 | 9 | +| call buildRbacSystemForHsOfficeRelation(NEW) | 1276 | 0 | 8 | +| with recursive grants as ( select descendantUuid, ascendantUuid from RbacGrants where descendantUuid = grantedId union all select ""grant"".descendantUuid, ""grant"".ascendantUuid from RbacGrants ""grant"" inner join grants recur on recur.ascendantUuid = ""grant"".descendantUuid ) select exists ( select $3 from grants where ascendantUuid = any(granteeIds) ) or grantedId = any(granteeIds) | 47540 | 0 | 0 | +| insert into RbacGrants (grantedByTriggerOf, ascendantuuid, descendantUuid, assumed) values (currentTriggerObjectUuid(), superRoleId, subRoleId, doAssume) on conflict do nothing" | 40472 | 0 | 0 | +| insert into public.hs_booking_item_rv (caption,parentitemuuid,projectuuid,resources,type,validity,version,uuid) values ($1,$2,$3,$4,$5,$6,$7,$8) | 926 | 0 | 7 | +| insert into hs_booking_item (resources, version, projectuuid, type, parentitemuuid, validity, uuid, caption) values (new.resources, new. version, new. projectuuid, new. type, new. parentitemuuid, new. validity, new. uuid, new. caption) returning * | 926 | 0 | 7 | + + ### LAZY loading for Relation.anchorPerson/.holderPerson/ The slowest query now was fetching Relations joined with Contact, Anchor-Person and Holder-Person, for all tables using the restricted (RBAC) views (_rv). -- 2.39.5 From 94ef33c775f4ccf34a314ff8a92fd7f9858424e4 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 24 Jul 2024 13:37:54 +0200 Subject: [PATCH 08/14] revert redundant partner-number in debitor --- .../debitor/HsOfficeDebitorController.java | 2 +- .../office/debitor/HsOfficeDebitorEntity.java | 25 ++++++++-- .../506-debitor/5060-hs-office-debitor.sql | 1 - .../5068-hs-office-debitor-test-data.sql | 13 +++-- .../6100-hs-booking-debitor.sql | 10 ++-- ...HostingAssetRepositoryIntegrationTest.java | 2 +- .../hs/migration/ImportOfficeData.java | 2 +- ...OfficeDebitorControllerAcceptanceTest.java | 2 +- .../HsOfficeDebitorEntityUnitTest.java | 32 ++++++++++-- ...fficeDebitorRepositoryIntegrationTest.java | 50 +++++++++---------- .../office/debitor/TestHsOfficeDebitor.java | 2 +- ...ceSepaMandateControllerAcceptanceTest.java | 9 +--- 12 files changed, 93 insertions(+), 57 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java index 9aa0745a..10409c20 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java @@ -96,7 +96,7 @@ public class HsOfficeDebitorController implements HsOfficeDebitorsApi { } final var relatedPartner = partnerRepo.findPartnerByPartnerPersonUuid(entityToSave.getDebitorRel().getHolder().getUuid()); - entityToSave.setPartnerNumber(relatedPartner.getPartnerNumber()); + entityToSave.setPartner(relatedPartner); final var savedEntity = debitorRepo.save(entityToSave); em.flush(); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java index d52a1410..75fa9b6d 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java @@ -7,6 +7,7 @@ import lombok.NoArgsConstructor; import lombok.Setter; import net.hostsharing.hsadminng.errors.DisplayName; import net.hostsharing.hsadminng.hs.office.bankaccount.HsOfficeBankAccountEntity; +import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerEntity; import net.hostsharing.hsadminng.hs.office.relation.HsOfficeRelationEntity; import net.hostsharing.hsadminng.rbac.rbacobject.RbacObject; import net.hostsharing.hsadminng.rbac.rbacdef.RbacView; @@ -14,6 +15,9 @@ import net.hostsharing.hsadminng.rbac.rbacdef.RbacView.SQL; import net.hostsharing.hsadminng.stringify.Stringify; import net.hostsharing.hsadminng.stringify.Stringifyable; import org.hibernate.annotations.GenericGenerator; +import org.hibernate.annotations.JoinFormula; +import org.hibernate.annotations.NotFound; +import org.hibernate.annotations.NotFoundAction; import jakarta.persistence.Column; import jakarta.persistence.Entity; @@ -73,8 +77,22 @@ public class HsOfficeDebitorEntity implements RbacObject, @Version private int version; - @Column(name = "partnernumber", columnDefinition = "numeric(5) not null") - private Integer partnerNumber; // redundant to HsOfficePartnerEntity.partnerNumber for performance reasons + @ManyToOne + @JoinFormula( + referencedColumnName = "uuid", + value = """ + ( + SELECT DISTINCT partner.uuid + FROM hs_office_partner_rv partner + JOIN hs_office_relation_rv dRel + ON dRel.uuid = debitorreluuid AND dRel.type = 'DEBITOR' + JOIN hs_office_relation_rv pRel + ON pRel.uuid = partner.partnerRelUuid AND pRel.type = 'PARTNER' + WHERE pRel.holderUuid = dRel.anchorUuid + ) + """) + @NotFound(action = NotFoundAction.IGNORE) + private HsOfficePartnerEntity partner; @Column(name = "debitornumbersuffix", length = 2) @Pattern(regexp = TWO_DECIMAL_DIGITS) @@ -117,8 +135,9 @@ public class HsOfficeDebitorEntity implements RbacObject, } private String getDebitorNumberString() { - return ofNullable(partnerNumber) + return ofNullable(partner) .filter(partner -> debitorNumberSuffix != null) + .map(HsOfficePartnerEntity::getPartnerNumber) .map(Object::toString) .map(partnerNumber -> partnerNumber + debitorNumberSuffix) .orElse(null); diff --git a/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql b/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql index 0a64321a..bbf72543 100644 --- a/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql +++ b/src/main/resources/db/changelog/5-hs-office/506-debitor/5060-hs-office-debitor.sql @@ -8,7 +8,6 @@ create table hs_office_debitor ( uuid uuid unique references RbacObject (uuid) initially deferred, version int not null default 0, - partnerNumber numeric(5) not null, -- redundant to hs_office_partner.partnerNumber for performance reasons debitorNumberSuffix char(2) not null check (debitorNumberSuffix::text ~ '^[0-9][0-9]$'), debitorRelUuid uuid not null references hs_office_relation(uuid), billable boolean not null default true, diff --git a/src/main/resources/db/changelog/5-hs-office/506-debitor/5068-hs-office-debitor-test-data.sql b/src/main/resources/db/changelog/5-hs-office/506-debitor/5068-hs-office-debitor-test-data.sql index 6c5cde10..2e888e29 100644 --- a/src/main/resources/db/changelog/5-hs-office/506-debitor/5068-hs-office-debitor-test-data.sql +++ b/src/main/resources/db/changelog/5-hs-office/506-debitor/5068-hs-office-debitor-test-data.sql @@ -9,8 +9,7 @@ Creates a single debitor test record. */ create or replace procedure createHsOfficeDebitorTestData( - forPartnerNumber numeric(5), - withDebitorNumberSuffix numeric(2), + withDebitorNumberSuffix numeric(5), forPartnerPersonName varchar, forBillingContactCaption varchar, withDefaultPrefix varchar @@ -43,8 +42,8 @@ begin -- raise exception 'creating test debitor: (uuid=%, debitorRelUuid=%, debitornumbersuffix=%, billable=%, vatbusiness=%, vatreversecharge=%, refundbankaccountuuid=%, defaultprefix=%)', -- uuid_generate_v4(), relatedDebitorRelUuid, withDebitorNumberSuffix, true, true, false, relatedBankAccountUuid, withDefaultPrefix; insert - into hs_office_debitor (uuid, debitorRelUuid, partnerNumber, debitornumbersuffix, billable, vatbusiness, vatreversecharge, refundbankaccountuuid, defaultprefix) - values (uuid_generate_v4(), relatedDebitorRelUuid, forPartnerNumber, withDebitorNumberSuffix, true, true, false, relatedBankAccountUuid, withDefaultPrefix); + into hs_office_debitor (uuid, debitorRelUuid, debitornumbersuffix, billable, vatbusiness, vatreversecharge, refundbankaccountuuid, defaultprefix) + values (uuid_generate_v4(), relatedDebitorRelUuid, withDebitorNumberSuffix, true, true, false, relatedBankAccountUuid, withDefaultPrefix); end; $$; --// @@ -55,9 +54,9 @@ end; $$; do language plpgsql $$ begin - call createHsOfficeDebitorTestData( 10001, 11, 'First GmbH', 'first contact', 'fir'); - call createHsOfficeDebitorTestData( 10002, 12, 'Second e.K.', 'second contact', 'sec'); - call createHsOfficeDebitorTestData( 10003, 13, 'Third OHG', 'third contact', 'thi'); + call createHsOfficeDebitorTestData(11, 'First GmbH', 'first contact', 'fir'); + call createHsOfficeDebitorTestData(12, 'Second e.K.', 'second contact', 'sec'); + call createHsOfficeDebitorTestData(13, 'Third OHG', 'third contact', 'thi'); end; $$; --// diff --git a/src/main/resources/db/changelog/6-hs-booking/610-booking-debitor/6100-hs-booking-debitor.sql b/src/main/resources/db/changelog/6-hs-booking/610-booking-debitor/6100-hs-booking-debitor.sql index d38640fc..72d9563f 100644 --- a/src/main/resources/db/changelog/6-hs-booking/610-booking-debitor/6100-hs-booking-debitor.sql +++ b/src/main/resources/db/changelog/6-hs-booking/610-booking-debitor/6100-hs-booking-debitor.sql @@ -1,13 +1,17 @@ --liquibase formatted sql -- ============================================================================ ---changeset hs-booking-debitor-EXTRACTED-VIEW:1 endDelimiter:--// +--changeset hs-booking-debitor-RESTRICTED-VIEW:1 endDelimiter:--// -- ---------------------------------------------------------------------------- create view hs_booking_debitor_xv as select debitor.uuid, debitor.version, - (debitor.partnerNumber::varchar || debitor.debitorNumberSuffix)::numeric as debitorNumber, + (partner.partnerNumber::varchar || debitor.debitorNumberSuffix)::numeric as debitorNumber, debitor.defaultPrefix - from hs_office_debitor debitor -- not from _rv for performance, nothing really secret here + from hs_office_debitor debitor + -- RBAC for debitor is sufficient, for faster access we are bypassing RBAC for the join tables + join hs_office_relation debitorRel on debitor.debitorReluUid=debitorRel.uuid + join hs_office_relation partnerRel on partnerRel.holderUuid=debitorRel.anchorUuid + join hs_office_partner partner on partner.partnerReluUid=partnerRel.uuid; --// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java index 0025d8d5..99f0efd6 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/hosting/asset/HsHostingAssetRepositoryIntegrationTest.java @@ -208,8 +208,8 @@ class HsHostingAssetRepositoryIntegrationTest extends ContextBasedTestWithCleanu // then exactlyTheseAssetsAreReturned( result, - "HsHostingAssetEntity(MANAGED_WEBSPACE, fir01, some Webspace, MANAGED_SERVER:vm1011, D-1000111:D-1000111 default project:separate ManagedWebspace)", "HsHostingAssetEntity(MANAGED_WEBSPACE, sec01, some Webspace, MANAGED_SERVER:vm1012, D-1000212:D-1000212 default project:separate ManagedWebspace)", + "HsHostingAssetEntity(MANAGED_WEBSPACE, fir01, some Webspace, MANAGED_SERVER:vm1011, D-1000111:D-1000111 default project:separate ManagedWebspace)", "HsHostingAssetEntity(MANAGED_WEBSPACE, thi01, some Webspace, MANAGED_SERVER:vm1013, D-1000313:D-1000313 default project:separate ManagedWebspace)"); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java b/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java index ec179f33..dd1f7d2b 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/migration/ImportOfficeData.java @@ -759,7 +759,7 @@ public class ImportOfficeData extends CsvDataImport { final var debitor = HsOfficeDebitorEntity.builder() .debitorNumberSuffix("00") - .partnerNumber(partner.getPartnerNumber()) + .partner(partner) .debitorRel(debitorRel) .defaultPrefix(rec.getString("member_code").replace("hsh00-", "")) .billable(rec.isEmpty("free") || rec.getString("free").equals("f")) diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java index 05f72887..ef8b4598 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java @@ -623,7 +623,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu final var givenPartner = partnerRepo.findPartnerByOptionalNameLike("Fourth").get(0); final var givenContact = contactRepo.findContactByOptionalCaptionLike("fourth contact").get(0); final var newDebitor = HsOfficeDebitorEntity.builder() - .partnerNumber(givenPartner.getPartnerNumber()) + .partner(givenPartner) .debitorNumberSuffix(nextDebitorSuffix()) .billable(true) .debitorRel( diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java index 5a0d3e03..e1250775 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntityUnitTest.java @@ -1,6 +1,7 @@ package net.hostsharing.hsadminng.hs.office.debitor; import net.hostsharing.hsadminng.hs.office.contact.HsOfficeContactEntity; +import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerEntity; import net.hostsharing.hsadminng.hs.office.person.HsOfficePersonEntity; import net.hostsharing.hsadminng.hs.office.person.HsOfficePersonType; import net.hostsharing.hsadminng.hs.office.relation.HsOfficeRelationEntity; @@ -28,7 +29,9 @@ class HsOfficeDebitorEntityUnitTest { .debitorNumberSuffix("67") .debitorRel(givenDebitorRel) .defaultPrefix("som") - .partnerNumber(12345) + .partner(HsOfficePartnerEntity.builder() + .partnerNumber(12345) + .build()) .build(); final var result = given.toString(); @@ -41,7 +44,9 @@ class HsOfficeDebitorEntityUnitTest { final var given = HsOfficeDebitorEntity.builder() .debitorRel(givenDebitorRel) .debitorNumberSuffix("67") - .partnerNumber(12345) + .partner(HsOfficePartnerEntity.builder() + .partnerNumber(12345) + .build()) .build(); final var result = given.toShortString(); @@ -54,7 +59,9 @@ class HsOfficeDebitorEntityUnitTest { final var given = HsOfficeDebitorEntity.builder() .debitorRel(givenDebitorRel) .debitorNumberSuffix("67") - .partnerNumber(12345) + .partner(HsOfficePartnerEntity.builder() + .partnerNumber(12345) + .build()) .build(); final var result = given.getDebitorNumber(); @@ -62,12 +69,25 @@ class HsOfficeDebitorEntityUnitTest { assertThat(result).isEqualTo(1234567); } + @Test + void getDebitorNumberWithoutPartnerReturnsNull() { + final var given = HsOfficeDebitorEntity.builder() + .debitorRel(givenDebitorRel) + .debitorNumberSuffix("67") + .partner(null) + .build(); + + final var result = given.getDebitorNumber(); + + assertThat(result).isNull(); + } + @Test void getDebitorNumberWithoutPartnerNumberReturnsNull() { final var given = HsOfficeDebitorEntity.builder() .debitorRel(givenDebitorRel) .debitorNumberSuffix("67") - .partnerNumber(null) + .partner(HsOfficePartnerEntity.builder().build()) .build(); final var result = given.getDebitorNumber(); @@ -80,7 +100,9 @@ class HsOfficeDebitorEntityUnitTest { final var given = HsOfficeDebitorEntity.builder() .debitorRel(givenDebitorRel) .debitorNumberSuffix(null) - .partnerNumber(12345) + .partner(HsOfficePartnerEntity.builder() + .partnerNumber(12345) + .build()) .build(); final var result = given.getDebitorNumber(); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java index 4de44aa3..d9c9f72c 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java @@ -89,7 +89,6 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean // when final var result = attempt(em, () -> { final var newDebitor = HsOfficeDebitorEntity.builder() - .partnerNumber(10001) .debitorNumberSuffix("21") .debitorRel(HsOfficeRelationEntity.builder() .type(HsOfficeRelationType.DEBITOR) @@ -158,7 +157,6 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenDebitorPerson = one(personRepo.findPersonByOptionalNameLike("Fourth eG")); final var givenContact = one(contactRepo.findContactByOptionalCaptionLike("fourth contact")); final var newDebitor = HsOfficeDebitorEntity.builder() - .partnerNumber(10001) .debitorNumberSuffix("22") .debitorRel(HsOfficeRelationEntity.builder() .type(HsOfficeRelationType.DEBITOR) @@ -320,7 +318,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean assertThatDebitorIsVisibleForUserWithRole( givenDebitor, - "hs_office_relation#FourtheG-with-DEBITOR-FourtheG:ADMIN"); + "hs_office_relation#FourtheG-with-DEBITOR-FourtheG:ADMIN", true); final var givenNewPartnerPerson = one(personRepo.findPersonByOptionalNameLike("First")); final var givenNewBillingPerson = one(personRepo.findPersonByOptionalNameLike("Firby")); final var givenNewContact = one(contactRepo.findContactByOptionalCaptionLike("sixth contact")); @@ -348,9 +346,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean // then result.assertSuccessful(); - assertThatDebitorIsVisibleForUserWithRole( - result.returnedValue(), - "global#global:ADMIN"); + assertThatDebitorIsVisibleForUserWithRole(result.returnedValue(), "global#global:ADMIN", true); // ... partner role was reassigned: assertThatDebitorIsNotVisibleForUserWithRole( @@ -358,7 +354,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean "hs_office_relation#FourtheG-with-DEBITOR-FourtheG:ADMIN"); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "hs_office_relation#FirstGmbH-with-DEBITOR-FirbySusan:AGENT"); + "hs_office_relation#FirstGmbH-with-DEBITOR-FirbySusan:AGENT", true); // ... contact role was reassigned: assertThatDebitorIsNotVisibleForUserWithRole( @@ -366,7 +362,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean "hs_office_contact#fifthcontact:ADMIN"); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "hs_office_contact#sixthcontact:ADMIN"); + "hs_office_contact#sixthcontact:ADMIN", false); // ... bank-account role was reassigned: assertThatDebitorIsNotVisibleForUserWithRole( @@ -374,7 +370,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean "hs_office_bankaccount#DE02200505501015871393:ADMIN"); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "hs_office_bankaccount#DE02120300000000202051:ADMIN"); + "hs_office_bankaccount#DE02120300000000202051:ADMIN", true); } @Test @@ -384,8 +380,8 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "fifth contact", null, "fig"); assertThatDebitorIsVisibleForUserWithRole( givenDebitor, - "hs_office_relation#FourtheG-with-DEBITOR-FourtheG:ADMIN"); - assertThatDebitorActuallyInDatabase(givenDebitor); + "hs_office_relation#FourtheG-with-DEBITOR-FourtheG:ADMIN", true); + assertThatDebitorActuallyInDatabase(givenDebitor, true); final var givenNewBankAccount = one(bankAccountRepo.findByOptionalHolderLike("first")); // when @@ -399,12 +395,12 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean result.assertSuccessful(); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "global#global:ADMIN"); + "global#global:ADMIN", true); // ... bank-account role was assigned: assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "hs_office_bankaccount#DE02120300000000202051:ADMIN"); + "hs_office_bankaccount#DE02120300000000202051:ADMIN", true); } @Test @@ -414,8 +410,8 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "fifth contact", "Fourth", "fih"); assertThatDebitorIsVisibleForUserWithRole( givenDebitor, - "hs_office_relation#HostsharingeG-with-PARTNER-FourtheG:AGENT"); - assertThatDebitorActuallyInDatabase(givenDebitor); + "hs_office_relation#HostsharingeG-with-PARTNER-FourtheG:AGENT", true); + assertThatDebitorActuallyInDatabase(givenDebitor, true); // when final var result = jpaAttempt.transacted(() -> { @@ -428,7 +424,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean result.assertSuccessful(); assertThatDebitorIsVisibleForUserWithRole( result.returnedValue(), - "global#global:ADMIN"); + "global#global:ADMIN", true); // ... bank-account role was removed from previous bank-account admin: assertThatDebitorIsNotVisibleForUserWithRole( @@ -443,8 +439,8 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "eighth", "Fourth", "eig"); assertThatDebitorIsVisibleForUserWithRole( givenDebitor, - "hs_office_relation#HostsharingeG-with-PARTNER-FourtheG:AGENT"); - assertThatDebitorActuallyInDatabase(givenDebitor); + "hs_office_relation#HostsharingeG-with-PARTNER-FourtheG:AGENT", true); + assertThatDebitorActuallyInDatabase(givenDebitor, true); // when final var result = jpaAttempt.transacted(() -> { @@ -463,10 +459,8 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean // given context("superuser-alex@hostsharing.net"); final var givenDebitor = givenSomeTemporaryDebitor("Fourth", "ninth", "Fourth", "nin"); - assertThatDebitorActuallyInDatabase(givenDebitor); - assertThatDebitorIsVisibleForUserWithRole( - givenDebitor, - "hs_office_contact#ninthcontact:ADMIN"); + assertThatDebitorActuallyInDatabase(givenDebitor, true); + assertThatDebitorIsVisibleForUserWithRole(givenDebitor, "hs_office_contact#ninthcontact:ADMIN", false); // when final var result = jpaAttempt.transacted(() -> { @@ -483,13 +477,16 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean "Unable to find net.hostsharing.hsadminng.hs.office.bankaccount.HsOfficeBankAccountEntity with id "); } - private void assertThatDebitorActuallyInDatabase(final HsOfficeDebitorEntity saved) { + private void assertThatDebitorActuallyInDatabase(final HsOfficeDebitorEntity saved, final boolean withPartner) { final var found = debitorRepo.findByUuid(saved.getUuid()); assertThat(found).isNotEmpty(); found.ifPresent(foundEntity -> { em.refresh(foundEntity); Hibernate.initialize(foundEntity); assertThat(foundEntity).isNotSameAs(saved); + if (withPartner) { + assertThat(foundEntity.getPartner()).isNotNull(); + } assertThat(foundEntity.getDebitorRel()).extracting(HsOfficeRelationEntity::toString) .isEqualTo(saved.getDebitorRel().toString()); }); @@ -497,10 +494,11 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean private void assertThatDebitorIsVisibleForUserWithRole( final HsOfficeDebitorEntity entity, - final String assumedRoles) { + final String assumedRoles, + final boolean withPartner) { jpaAttempt.transacted(() -> { context("superuser-alex@hostsharing.net", assumedRoles); - assertThatDebitorActuallyInDatabase(entity); + assertThatDebitorActuallyInDatabase(entity, withPartner); }).assertSuccessful(); } @@ -615,7 +613,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenBankAccount = bankAccountHolder != null ? one(bankAccountRepo.findByOptionalHolderLike(bankAccountHolder)) : null; final var newDebitor = HsOfficeDebitorEntity.builder() - .partnerNumber(givenPartner.getPartnerNumber()) + .partner(givenPartner) .debitorNumberSuffix("20") .debitorRel(HsOfficeRelationEntity.builder() .type(HsOfficeRelationType.DEBITOR) diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/TestHsOfficeDebitor.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/TestHsOfficeDebitor.java index 57b65678..b8ddf8b5 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/TestHsOfficeDebitor.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/TestHsOfficeDebitor.java @@ -19,7 +19,7 @@ public class TestHsOfficeDebitor { .anchor(HsOfficePersonEntity.builder().build()) .contact(TEST_CONTACT) .build()) - .partnerNumber(TEST_PARTNER.getPartnerNumber()) + .partner(TEST_PARTNER) .defaultPrefix("abc") .build(); } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java index 77fe2f94..c0f68451 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/sepamandate/HsOfficeSepaMandateControllerAcceptanceTest.java @@ -6,7 +6,6 @@ import io.restassured.http.ContentType; import net.hostsharing.hsadminng.HsadminNgApplication; import net.hostsharing.hsadminng.hs.office.bankaccount.HsOfficeBankAccountRepository; import net.hostsharing.hsadminng.hs.office.debitor.HsOfficeDebitorRepository; -import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerRepository; import net.hostsharing.hsadminng.rbac.test.ContextBasedTestWithCleanup; import net.hostsharing.hsadminng.rbac.test.JpaAttempt; import org.json.JSONException; @@ -43,9 +42,6 @@ class HsOfficeSepaMandateControllerAcceptanceTest extends ContextBasedTestWithCl @Autowired HsOfficeSepaMandateRepository sepaMandateRepo; - @Autowired - HsOfficePartnerRepository partnerRepo; - @Autowired HsOfficeDebitorRepository debitorRepo; @@ -497,9 +493,8 @@ class HsOfficeSepaMandateControllerAcceptanceTest extends ContextBasedTestWithCl return jpaAttempt.transacted(() -> { context.define("superuser-alex@hostsharing.net"); final var givenDebitor = debitorRepo.findDebitorByDebitorNumber(debitorNumber).get(0); - final var givenPartner = partnerRepo.findPartnerByPartnerNumber(debitorNumber/100); - final var bankAccountHolder = ofNullable(givenPartner.getPartnerRel().getHolder().getTradeName()) - .orElse(givenPartner.getPartnerRel().getHolder().getFamilyName()); + final var bankAccountHolder = ofNullable(givenDebitor.getPartner().getPartnerRel().getHolder().getTradeName()) + .orElse(givenDebitor.getPartner().getPartnerRel().getHolder().getFamilyName()); final var givenBankAccount = bankAccountRepo.findByOptionalHolderLike(bankAccountHolder).get(0); final var newSepaMandate = HsOfficeSepaMandateEntity.builder() .uuid(UUID.randomUUID()) -- 2.39.5 From 9ab0ce02f29141218d99a93407243acf602266fb Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Thu, 25 Jul 2024 09:32:49 +0200 Subject: [PATCH 09/14] remove alias pg-sql-run-with-query-stats, does not work --- .aliases | 1 - 1 file changed, 1 deletion(-) diff --git a/.aliases b/.aliases index b5d3cd0e..f215442d 100644 --- a/.aliases +++ b/.aliases @@ -72,7 +72,6 @@ alias podman-use='export DOCKER_HOST="unix:///run/user/$UID/podman/podman.sock"; alias gw=gradleWrapper alias pg-sql-run='docker run --name hsadmin-ng-postgres -e POSTGRES_PASSWORD=password -p 5432:5432 -d postgres:15.5-bookworm' -alias pg-sql-run-with-query-stats='docker run --name hsadmin-ng-postgres -e POSTGRES_PASSWORD=password -v COPY etc/postgresql-log-slow-queries.conf:/etc/postgresql/postgresql.conf -p 5432:5432 -d postgres:15.5-bookworm' alias pg-sql-stop='docker stop hsadmin-ng-postgres' alias pg-sql-start='docker container start hsadmin-ng-postgres' alias pg-sql-remove='docker rm hsadmin-ng-postgres' -- 2.39.5 From 604013848c64cb16e9d06ab28eca5995ab77042d Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Thu, 25 Jul 2024 09:42:18 +0200 Subject: [PATCH 10/14] improve documentation --- doc/rbac-performance-analysis.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/doc/rbac-performance-analysis.md b/doc/rbac-performance-analysis.md index 3747eb29..b3a578b7 100644 --- a/doc/rbac-performance-analysis.md +++ b/doc/rbac-performance-analysis.md @@ -245,9 +245,9 @@ create index on RbacPermission (objectUuid, op); create index on RbacPermission (opTableName, op); ``` -### +### LAZY loading for Relation.anchorPerson/.holderPerson/ -The import took 21mins with these statistics: +At this point, the import took 21mins with these statistics: | query | calls | total_m | mean_ms | |-------|-------|---------|---------| @@ -268,8 +268,6 @@ The import took 21mins with these statistics: | insert into hs_booking_item (resources, version, projectuuid, type, parentitemuuid, validity, uuid, caption) values (new.resources, new. version, new. projectuuid, new. type, new. parentitemuuid, new. validity, new. uuid, new. caption) returning * | 926 | 0 | 7 | -### LAZY loading for Relation.anchorPerson/.holderPerson/ - The slowest query now was fetching Relations joined with Contact, Anchor-Person and Holder-Person, for all tables using the restricted (RBAC) views (_rv). We changed these mappings from `EAGER` (default) to `LAZY` to `@ManyToOne(fetch = FetchType.LAZY)` and got this result: @@ -292,6 +290,12 @@ We changed these mappings from `EAGER` (default) to `LAZY` to `@ManyToOne(fetch | insert into hs_booking_item (resources, version, projectuuid, type, parentitemuuid, validity, uuid, caption) values (new.resources, new. version, new. projectuuid, new. type, new. parentitemuuid, new. validity, new. uuid, new. caption) returning * | 926 | 0 | 7 | insert into RbacGrants (grantedByTriggerOf, ascendantuuid, descendantUuid, assumed) values (currentTriggerObjectUuid(), superRoleId, subRoleId, doAssume) on conflict do nothing | 40472 | 0 | 0 | +Now, finally, the total runtime of the import was down to 12 minutes. This is repeatable, where originally, the import took about 25mins in most cases and just rarely - and for unknown reasons - 10min. +## Summary -Now, finally, the total runtime of the import was down to 12 minutes. +That the import runtime is down to about 12min is repeatable, where originally, the import took about 25mins in most cases and just rarely - and for unknown reasons - just 10min. + +Merging the recursive CTE query to determine the RBAC SELECT-permission, made it more clear which business-queries take the time. + +Avoiding EAGER-loading where not neccessary, reduced the total runtime of the import to about the half. -- 2.39.5 From 3dc1e3b4b248db19965669b567ed52a488fd0075 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Thu, 25 Jul 2024 10:00:25 +0200 Subject: [PATCH 11/14] amendmends after self-review --- .../hs/office/debitor/HsOfficeDebitorController.java | 7 ------- .../hsadminng/hs/office/partner/HsOfficePartnerEntity.java | 5 +++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java index 10409c20..5455b99b 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorController.java @@ -5,7 +5,6 @@ import net.hostsharing.hsadminng.hs.office.generated.api.v1.api.HsOfficeDebitors import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeDebitorInsertResource; import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeDebitorPatchResource; import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeDebitorResource; -import net.hostsharing.hsadminng.hs.office.partner.HsOfficePartnerRepository; import net.hostsharing.hsadminng.hs.office.relation.HsOfficeRelationEntity; import net.hostsharing.hsadminng.hs.office.relation.HsOfficeRelationRepository; import net.hostsharing.hsadminng.mapper.Mapper; @@ -38,9 +37,6 @@ public class HsOfficeDebitorController implements HsOfficeDebitorsApi { @Autowired private HsOfficeDebitorRepository debitorRepo; - @Autowired - private HsOfficePartnerRepository partnerRepo; - @Autowired private HsOfficeRelationRepository relRepo; @@ -95,9 +91,6 @@ public class HsOfficeDebitorController implements HsOfficeDebitorsApi { () -> { throw new EntityNotFoundException("ERROR: [400] debitorRelUuid not found: " + body.getDebitorRelUuid());}); } - final var relatedPartner = partnerRepo.findPartnerByPartnerPersonUuid(entityToSave.getDebitorRel().getHolder().getUuid()); - entityToSave.setPartner(relatedPartner); - final var savedEntity = debitorRepo.save(entityToSave); em.flush(); em.refresh(savedEntity); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java index 6f9589f5..2ec637be 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/partner/HsOfficePartnerEntity.java @@ -66,11 +66,11 @@ public class HsOfficePartnerEntity implements Stringifyable, RbacObject Date: Thu, 25 Jul 2024 11:15:13 +0200 Subject: [PATCH 12/14] amendmends after code-review --- .../office/debitor/HsOfficeDebitorEntity.java | 8 +- .../membership/HsOfficeMembershipEntity.java | 3 +- .../partner/HsOfficePartnerRepository.java | 9 -- .../090-log-slow-queries-extensions.sql | 2 +- .../db/changelog/1-rbac/1050-rbac-base.sql | 1 - .../7010-hs-hosting-asset.sql | 2 - ...OfficeDebitorControllerAcceptanceTest.java | 101 +++++++++++++++++- ...fficeDebitorRepositoryIntegrationTest.java | 1 - src/test/resources/application.yml | 11 +- 9 files changed, 113 insertions(+), 25 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java index 75fa9b6d..a9e0ca98 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java @@ -21,6 +21,7 @@ import org.hibernate.annotations.NotFoundAction; import jakarta.persistence.Column; import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; import jakarta.persistence.JoinColumn; @@ -77,7 +78,7 @@ public class HsOfficeDebitorEntity implements RbacObject, @Version private int version; - @ManyToOne + @ManyToOne(fetch = FetchType.LAZY) @JoinFormula( referencedColumnName = "uuid", value = """ @@ -98,7 +99,7 @@ public class HsOfficeDebitorEntity implements RbacObject, @Pattern(regexp = TWO_DECIMAL_DIGITS) private String debitorNumberSuffix; - @ManyToOne(cascade = { PERSIST, MERGE, REFRESH, DETACH }, optional = false) + @ManyToOne(cascade = { PERSIST, MERGE, REFRESH, DETACH }, optional = false, fetch = FetchType.LAZY) @JoinColumn(name = "debitorreluuid", nullable = false) private HsOfficeRelationEntity debitorRel; @@ -117,7 +118,7 @@ public class HsOfficeDebitorEntity implements RbacObject, @Column(name = "vatreversecharge") private boolean vatReverseCharge; - @ManyToOne + @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "refundbankaccountuuid") private HsOfficeBankAccountEntity refundBankAccount; @@ -127,6 +128,7 @@ public class HsOfficeDebitorEntity implements RbacObject, @Override public HsOfficeDebitorEntity load() { RbacObject.super.load(); + partner.load(); debitorRel.load(); if (refundBankAccount != null) { refundBankAccount.load(); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipEntity.java index fe55f653..20dac5c7 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/membership/HsOfficeMembershipEntity.java @@ -21,6 +21,7 @@ import jakarta.persistence.Column; 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; @@ -80,7 +81,7 @@ public class HsOfficeMembershipEntity implements RbacObject findPartnerByOptionalNameLike(String name); HsOfficePartnerEntity findPartnerByPartnerNumber(Integer partnerNumber); - @Query(""" - SELECT DISTINCT partner - FROM HsOfficePartnerEntity partner - JOIN HsOfficeRelationEntity pRel - ON pRel.uuid = partner.partnerRel.uuid AND pRel.type = 'PARTNER' - WHERE pRel.holder.uuid = :partnerPersonUuid - """) - HsOfficePartnerEntity findPartnerByPartnerPersonUuid(UUID partnerPersonUuid); - HsOfficePartnerEntity save(final HsOfficePartnerEntity entity); long count(); diff --git a/src/main/resources/db/changelog/0-basis/090-log-slow-queries-extensions.sql b/src/main/resources/db/changelog/0-basis/090-log-slow-queries-extensions.sql index dd8aa74f..953004db 100644 --- a/src/main/resources/db/changelog/0-basis/090-log-slow-queries-extensions.sql +++ b/src/main/resources/db/changelog/0-basis/090-log-slow-queries-extensions.sql @@ -3,7 +3,7 @@ -- ============================================================================ -- PG-STAT-STATEMENTS-EXTENSION ---changeset pg-stat-statements-extension:1 runOnChange=true endDelimiter:--// +--changeset pg-stat-statements-extension:1 context:pg_stat_statements endDelimiter:--// -- ---------------------------------------------------------------------------- /* Makes improved uuid generation available. diff --git a/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql b/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql index 86a4527d..6199abcd 100644 --- a/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql +++ b/src/main/resources/db/changelog/1-rbac/1050-rbac-base.sql @@ -750,7 +750,6 @@ begin end if; end; $$; -ALTER FUNCTION queryAccessibleObjectUuidsOfSubjectIds SET enable_hashjoin = off; --// -- ============================================================================ diff --git a/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7010-hs-hosting-asset.sql b/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7010-hs-hosting-asset.sql index c1c127f0..3b1b54d1 100644 --- a/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7010-hs-hosting-asset.sql +++ b/src/main/resources/db/changelog/7-hs-hosting/701-hosting-asset/7010-hs-hosting-asset.sql @@ -44,8 +44,6 @@ create table if not exists hs_hosting_asset constraint chk_hs_hosting_asset_has_booking_item_or_parent_asset check (bookingItemUuid is not null or parentAssetUuid is not null or type in ('DOMAIN_SETUP', 'IPV4_NUMBER', 'IPV6_NUMBER')) ); --- create index on hs_hosting_asset (identifier); --- create index on hs_hosting_asset (type); --// diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java index ef8b4598..737809b7 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java @@ -113,6 +113,38 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "debitorNumber": 1000111, "debitorNumberSuffix": 11, + "partner": { + "partnerNumber": 10001, + "partnerRel": { + "anchor": { + "personType": "LEGAL_PERSON", + "tradeName": "Hostsharing eG", + "givenName": null, + "familyName": null + }, + "holder": { + "personType": "LEGAL_PERSON", + "tradeName": "First GmbH", + "givenName": null, + "familyName": null + }, + "type": "PARTNER", + "mark": null, + "contact": { + "caption": "first contact", + "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, + "phoneNumbers": { "phone_office": "+49 123 1234567" } + } + }, + "details": { + "registrationOffice": "Hamburg", + "registrationNumber": "RegNo123456789", + "birthName": null, + "birthPlace": null, + "birthday": null, + "dateOfDeath": null + } + }, "billable": true, "vatId": null, "vatCountryCode": null, @@ -136,6 +168,21 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "debitorNumber": 1000212, "debitorNumberSuffix": 12, + "partner": { + "partnerNumber": 10002, + "partnerRel": { + "anchor": {"tradeName": "Hostsharing eG"}, + "holder": {"tradeName": "Second e.K."}, + "type": "PARTNER", + "contact": { + "emailAddresses": { "main": "contact-admin@secondcontact.example.com" } + } + }, + "details": { + "registrationOffice": "Hamburg", + "registrationNumber": "RegNo123456789" + } + }, "billable": true, "vatId": null, "vatCountryCode": null, @@ -155,6 +202,21 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "debitorNumber": 1000313, "debitorNumberSuffix": 13, + "partner": { + "partnerNumber": 10003, + "partnerRel": { + "anchor": {"tradeName": "Hostsharing eG"}, + "holder": {"tradeName": "Third OHG"}, + "type": "PARTNER", + "contact": { + "emailAddresses": { "main": "contact-admin@thirdcontact.example.com" } + } + }, + "details": { + "registrationOffice": "Hamburg", + "registrationNumber": "RegNo123456789" + } + }, "billable": true, "vatId": null, "vatCountryCode": null, @@ -295,6 +357,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu .contentType(ContentType.JSON) .body("uuid", isUuidValid()) .body("debitorRel.contact.caption", is(givenContact.getCaption())) + .body("partner.partnerRel.holder.tradeName", is(givenPartner.getPartnerRel().getHolder().getTradeName())) .body("vatId", equalTo(null)) .body("vatCountryCode", equalTo(null)) .body("vatBusiness", equalTo(false)) @@ -408,6 +471,25 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "debitorNumber": 1000111, "debitorNumberSuffix": 11, + "partner": { + "partnerNumber": 10001, + "partnerRel": { + "anchor": { "personType": "LEGAL_PERSON", "tradeName": "Hostsharing eG"}, + "holder": { "personType": "LEGAL_PERSON", "tradeName": "First GmbH"}, + "type": "PARTNER", + "mark": null, + "contact": { + "caption": "first contact", + "postalAddress": "Vorname Nachname\\nStraße Hnr\\nPLZ Stadt", + "emailAddresses": { "main": "contact-admin@firstcontact.example.com" }, + "phoneNumbers": { "phone_office": "+49 123 1234567" } + } + }, + "details": { + "registrationOffice": "Hamburg", + "registrationNumber": "RegNo123456789" + } + }, "billable": true, "vatBusiness": true, "vatReverseCharge": false, @@ -501,6 +583,24 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu }, "debitorNumber": 10004${debitorNumberSuffix}, "debitorNumberSuffix": ${debitorNumberSuffix}, + "partner": { + "partnerNumber": 10004, + "partnerRel": { + "anchor": { "tradeName": "Hostsharing eG" }, + "holder": { "tradeName": "Fourth eG" }, + "type": "PARTNER", + "mark": null, + "contact": { "caption": "fourth contact" } + }, + "details": { + "registrationOffice": "Hamburg", + "registrationNumber": "RegNo123456789", + "birthName": null, + "birthPlace": null, + "birthday": null, + "dateOfDeath": null + } + }, "billable": true, "vatId": "VAT222222", "vatCountryCode": "AA", @@ -623,7 +723,6 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu final var givenPartner = partnerRepo.findPartnerByOptionalNameLike("Fourth").get(0); final var givenContact = contactRepo.findContactByOptionalCaptionLike("fourth contact").get(0); final var newDebitor = HsOfficeDebitorEntity.builder() - .partner(givenPartner) .debitorNumberSuffix(nextDebitorSuffix()) .billable(true) .debitorRel( diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java index d9c9f72c..f3986f4f 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java @@ -613,7 +613,6 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenBankAccount = bankAccountHolder != null ? one(bankAccountRepo.findByOptionalHolderLike(bankAccountHolder)) : null; final var newDebitor = HsOfficeDebitorEntity.builder() - .partner(givenPartner) .debitorNumberSuffix("20") .debitorRel(HsOfficeRelationEntity.builder() .type(HsOfficeRelationType.DEBITOR) diff --git a/src/test/resources/application.yml b/src/test/resources/application.yml index 293e6f72..7ae587f3 100644 --- a/src/test/resources/application.yml +++ b/src/test/resources/application.yml @@ -26,10 +26,9 @@ spring: liquibase: change-log: classpath:/db/changelog/db.changelog-master.yaml - contexts: tc,test,dev + contexts: tc,test,dev,pg_stat_statements -# FIXME: re-activate? -#logging: -# level: -# liquibase: INFO -# net.ttddyy.dsproxy.listener: DEBUG +logging: + level: + liquibase: INFO + net.ttddyy.dsproxy.listener: DEBUG -- 2.39.5 From 8c26ee24e8fab5139b087543f84db6ea27a39ecf Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Fri, 26 Jul 2024 11:19:13 +0200 Subject: [PATCH 13/14] lazy-loading related test-fixes --- .../coopassets/HsOfficeCoopAssetsTransactionEntity.java | 7 +++++++ .../coopshares/HsOfficeCoopSharesTransactionEntity.java | 7 +++++++ .../hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java | 7 +++++-- ...ficeCoopAssetsTransactionRepositoryIntegrationTest.java | 2 +- ...ficeCoopSharesTransactionRepositoryIntegrationTest.java | 2 +- .../debitor/HsOfficeDebitorControllerAcceptanceTest.java | 2 +- .../debitor/HsOfficeDebitorRepositoryIntegrationTest.java | 7 +++++-- 7 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java index 6a366417..35e0bda9 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionEntity.java @@ -105,6 +105,13 @@ public class HsOfficeCoopAssetsTransactionEntity implements Stringifyable, RbacO @OneToOne(mappedBy = "adjustedAssetTx") private HsOfficeCoopAssetsTransactionEntity adjustmentAssetTx; + @Override + public HsOfficeCoopAssetsTransactionEntity load() { + RbacObject.super.load(); + membership.load(); + return this; + } + public String getTaggedMemberNumber() { return ofNullable(membership).map(HsOfficeMembershipEntity::toShortString).orElse("M-???????"); } diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java index b1193945..cbab7e4f 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionEntity.java @@ -102,6 +102,13 @@ public class HsOfficeCoopSharesTransactionEntity implements Stringifyable, RbacO @OneToOne(mappedBy = "adjustedShareTx") private HsOfficeCoopSharesTransactionEntity adjustmentShareTx; + @Override + public HsOfficeCoopSharesTransactionEntity load() { + RbacObject.super.load(); + membership.load(); + return this; + } + @Override public String toString() { return stringify.apply(this); diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java index a9e0ca98..04ebd03b 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorEntity.java @@ -92,7 +92,7 @@ public class HsOfficeDebitorEntity implements RbacObject, WHERE pRel.holderUuid = dRel.anchorUuid ) """) - @NotFound(action = NotFoundAction.IGNORE) + @NotFound(action = NotFoundAction.IGNORE) // TODO.impl: map a simplified raw-PartnerEntity, just for the partner-number private HsOfficePartnerEntity partner; @Column(name = "debitornumbersuffix", length = 2) @@ -120,6 +120,7 @@ public class HsOfficeDebitorEntity implements RbacObject, @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "refundbankaccountuuid") + @NotFound(action = NotFoundAction.IGNORE) private HsOfficeBankAccountEntity refundBankAccount; @Column(name = "defaultprefix", columnDefinition = "char(3) not null") @@ -128,7 +129,9 @@ public class HsOfficeDebitorEntity implements RbacObject, @Override public HsOfficeDebitorEntity load() { RbacObject.super.load(); - partner.load(); + if (partner != null) { + partner.load(); + } debitorRel.load(); if (refundBankAccount != null) { refundBankAccount.load(); diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java index 8306ac20..376da64d 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/coopassets/HsOfficeCoopAssetsTransactionRepositoryIntegrationTest.java @@ -62,7 +62,7 @@ class HsOfficeCoopAssetsTransactionRepositoryIntegrationTest extends ContextBase // given context("superuser-alex@hostsharing.net"); final var count = coopAssetsTransactionRepo.count(); - final var givenMembership = membershipRepo.findMembershipByMemberNumber(1000101); + final var givenMembership = membershipRepo.findMembershipByMemberNumber(1000101).load(); // when final var result = attempt(em, () -> { diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionRepositoryIntegrationTest.java index afe48f0e..cc81f352 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/coopshares/HsOfficeCoopSharesTransactionRepositoryIntegrationTest.java @@ -61,7 +61,7 @@ class HsOfficeCoopSharesTransactionRepositoryIntegrationTest extends ContextBase // given context("superuser-alex@hostsharing.net"); final var count = coopSharesTransactionRepo.count(); - final var givenMembership = membershipRepo.findMembershipByMemberNumber(1000101); + final var givenMembership = membershipRepo.findMembershipByMemberNumber(1000101).load(); // when final var result = attempt(em, () -> { diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java index 737809b7..2fee9a31 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorControllerAcceptanceTest.java @@ -720,7 +720,7 @@ class HsOfficeDebitorControllerAcceptanceTest extends ContextBasedTestWithCleanu private HsOfficeDebitorEntity givenSomeTemporaryDebitor() { return jpaAttempt.transacted(() -> { context.define("superuser-alex@hostsharing.net"); - final var givenPartner = partnerRepo.findPartnerByOptionalNameLike("Fourth").get(0); + final var givenPartner = partnerRepo.findPartnerByOptionalNameLike("Fourth").get(0).load(); final var givenContact = contactRepo.findContactByOptionalCaptionLike("fourth contact").get(0); final var newDebitor = HsOfficeDebitorEntity.builder() .debitorNumberSuffix(nextDebitorSuffix()) diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java index f3986f4f..383945b4 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java @@ -14,6 +14,7 @@ import net.hostsharing.hsadminng.rbac.rbacrole.RawRbacRoleRepository; import net.hostsharing.hsadminng.mapper.Array; import net.hostsharing.hsadminng.rbac.test.JpaAttempt; import org.hibernate.Hibernate; +import org.hibernate.exception.GenericJDBCException; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -23,7 +24,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.Import; -import org.springframework.orm.jpa.JpaObjectRetrievalFailureException; import org.springframework.orm.jpa.JpaSystemException; import org.springframework.transaction.annotation.Transactional; @@ -83,12 +83,14 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean // given context("superuser-alex@hostsharing.net"); final var count = debitorRepo.count(); + final var givenPartner = partnerRepo.findPartnerByPartnerNumber(10001); final var givenPartnerPerson = one(personRepo.findPersonByOptionalNameLike("First GmbH")); final var givenContact = one(contactRepo.findContactByOptionalCaptionLike("first contact")); // when final var result = attempt(em, () -> { final var newDebitor = HsOfficeDebitorEntity.builder() + .partner(givenPartner) .debitorNumberSuffix("21") .debitorRel(HsOfficeRelationEntity.builder() .type(HsOfficeRelationType.DEBITOR) @@ -472,7 +474,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean // then result.assertExceptionWithRootCauseMessage( - JpaObjectRetrievalFailureException.class, + GenericJDBCException.class, // this technical error message gets translated to a [403] error at the controller level "Unable to find net.hostsharing.hsadminng.hs.office.bankaccount.HsOfficeBankAccountEntity with id "); } @@ -613,6 +615,7 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean final var givenBankAccount = bankAccountHolder != null ? one(bankAccountRepo.findByOptionalHolderLike(bankAccountHolder)) : null; final var newDebitor = HsOfficeDebitorEntity.builder() + .partner(givenPartner) .debitorNumberSuffix("20") .debitorRel(HsOfficeRelationEntity.builder() .type(HsOfficeRelationType.DEBITOR) -- 2.39.5 From 606c5e2f953a520a8054813e5e8a2507422a4af1 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Sat, 27 Jul 2024 10:17:33 +0200 Subject: [PATCH 14/14] amend changed error message due to explicit lazy loading --- .../debitor/HsOfficeDebitorRepositoryIntegrationTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java index 383945b4..fabc93e7 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/debitor/HsOfficeDebitorRepositoryIntegrationTest.java @@ -14,7 +14,6 @@ import net.hostsharing.hsadminng.rbac.rbacrole.RawRbacRoleRepository; import net.hostsharing.hsadminng.mapper.Array; import net.hostsharing.hsadminng.rbac.test.JpaAttempt; import org.hibernate.Hibernate; -import org.hibernate.exception.GenericJDBCException; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -474,9 +473,9 @@ class HsOfficeDebitorRepositoryIntegrationTest extends ContextBasedTestWithClean // then result.assertExceptionWithRootCauseMessage( - GenericJDBCException.class, - // this technical error message gets translated to a [403] error at the controller level - "Unable to find net.hostsharing.hsadminng.hs.office.bankaccount.HsOfficeBankAccountEntity with id "); + JpaSystemException.class, + "ERROR: [403]", + "is not allowed to update hs_office_debitor uuid"); } private void assertThatDebitorActuallyInDatabase(final HsOfficeDebitorEntity saved, final boolean withPartner) { -- 2.39.5