From 5c60935d2137c068233d31d6167b7a46ad0b1818 Mon Sep 17 00:00:00 2001
From: Lars Johansson <lars.johansson@ess.eu>
Date: Thu, 4 Jan 2024 08:22:35 +0100
Subject: [PATCH] Refactor predicate handling for structure repository classes

Refactor handling of where statements to aid understanding and better allow maintenance.
---
 .../repository/DeviceGroupRepository.java     | 41 ++++++++-----------
 .../repository/DeviceTypeRepository.java      | 41 ++++++++-----------
 .../repository/DisciplineRepository.java      | 41 ++++++++-----------
 .../names/repository/SubsystemRepository.java | 41 ++++++++-----------
 .../repository/SystemGroupRepository.java     | 41 ++++++++-----------
 .../names/repository/SystemRepository.java    | 41 ++++++++-----------
 6 files changed, 108 insertions(+), 138 deletions(-)

diff --git a/src/main/java/org/openepics/names/repository/DeviceGroupRepository.java b/src/main/java/org/openepics/names/repository/DeviceGroupRepository.java
index df7e6f14..e5524fdd 100644
--- a/src/main/java/org/openepics/names/repository/DeviceGroupRepository.java
+++ b/src/main/java/org/openepics/names/repository/DeviceGroupRepository.java
@@ -206,25 +206,20 @@ public class DeviceGroupRepository {
             // purpose of Naming to show valid entries
             //     therefore
             //         exclude some values unless history requested
-            //         make sure to not exclude present and future values
+            //         make sure to not exclude present and future values (latest approved and pending)
             //
-            // not only
-            //     exclude APPROVED and not latest
-            //          select * from Structure s where
-            //              not(s.status = 'APPROVED' and s.latest = false)
-            // not only
-            //     exclude earlier than APPROVED and latest (considered history)
-            //     include PENDING and not latest           (considered future)
-            //          select * from Structure s where
-            //                 (not s.id < (select id from structure s2 where s2.uuid = s.uuid and s2.latest = true))
-            //              or (s.status = 'PENDING' and s.latest = false)
-            // instead
-            //     exclude earlier than APPROVED and latest (considered history)
-            //   ( include if latest )
-            //     include if latest not exists             (considered present + future)
-            //         select * from Structure s where
-            //        	      (not s.id < (select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
-            //             or (not  exists(select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
+            // condition(s)
+            //     exclude content (with latest) before latest - considered history
+            //         select * from structure s
+            //         where (
+            //                 not (exists (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true) and s.id < (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true))
+            //         )
+            //         <-->
+            //                 not (exists (subquery id) and s.id < (subquery id))
+            //
+            // note
+            //     persistence libraries may optimize query
+            //         e.g. change (!a and !b) to !(a or b)
 
             Root<DeviceGroup> fromSub = sub.from(DeviceGroup.class);
             sub.where(cb.and(
@@ -233,11 +228,11 @@ public class DeviceGroupRepository {
                     ));
             sub.select(fromSub.get(Persistable.FIELD_ID));
 
-            Predicate predicateExcludeInclude =
-                    cb.or(
-                            cb.not(cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub)),
-                            cb.not(cb.exists(sub)));
-            predicates.add(predicateExcludeInclude);
+            Predicate predicateExclude =
+                    cb.and(cb.not(cb.and(
+                            cb.exists(sub),
+                            cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub))));
+            predicates.add(predicateExclude);
         }
 
         if (statuses != null) {
diff --git a/src/main/java/org/openepics/names/repository/DeviceTypeRepository.java b/src/main/java/org/openepics/names/repository/DeviceTypeRepository.java
index 43553ac1..b1543e1a 100644
--- a/src/main/java/org/openepics/names/repository/DeviceTypeRepository.java
+++ b/src/main/java/org/openepics/names/repository/DeviceTypeRepository.java
@@ -206,25 +206,20 @@ public class DeviceTypeRepository {
             // purpose of Naming to show valid entries
             //     therefore
             //         exclude some values unless history requested
-            //         make sure to not exclude present and future values
+            //         make sure to not exclude present and future values (latest approved and pending)
             //
-            // not only
-            //     exclude APPROVED and not latest
-            //          select * from Structure s where
-            //              not(s.status = 'APPROVED' and s.latest = false)
-            // not only
-            //     exclude earlier than APPROVED and latest (considered history)
-            //     include PENDING and not latest           (considered future)
-            //          select * from Structure s where
-            //                 (not s.id < (select id from structure s2 where s2.uuid = s.uuid and s2.latest = true))
-            //              or (s.status = 'PENDING' and s.latest = false)
-            // instead
-            //     exclude earlier than APPROVED and latest (considered history)
-            //   ( include if latest )
-            //     include if latest not exists             (considered present + future)
-            //         select * from Structure s where
-            //        	      (not s.id < (select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
-            //             or (not  exists(select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
+            // condition(s)
+            //     exclude content (with latest) before latest - considered history
+            //         select * from structure s
+            //         where (
+            //                 not (exists (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true) and s.id < (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true))
+            //         )
+            //         <-->
+            //                 not (exists (subquery id) and s.id < (subquery id))
+            //
+            // note
+            //     persistence libraries may optimize query
+            //         e.g. change (!a and !b) to !(a or b)
 
             Root<DeviceType> fromSub = sub.from(DeviceType.class);
             sub.where(cb.and(
@@ -233,11 +228,11 @@ public class DeviceTypeRepository {
                     ));
             sub.select(fromSub.get(Persistable.FIELD_ID));
 
-            Predicate predicateExcludeInclude =
-                    cb.or(
-                            cb.not(cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub)),
-                            cb.not(cb.exists(sub)));
-            predicates.add(predicateExcludeInclude);
+            Predicate predicateExclude =
+                    cb.and(cb.not(cb.and(
+                            cb.exists(sub),
+                            cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub))));
+            predicates.add(predicateExclude);
         }
 
         if (statuses != null) {
diff --git a/src/main/java/org/openepics/names/repository/DisciplineRepository.java b/src/main/java/org/openepics/names/repository/DisciplineRepository.java
index f168ad7c..ad07c283 100644
--- a/src/main/java/org/openepics/names/repository/DisciplineRepository.java
+++ b/src/main/java/org/openepics/names/repository/DisciplineRepository.java
@@ -202,25 +202,20 @@ public class DisciplineRepository {
             // purpose of Naming to show valid entries
             //     therefore
             //         exclude some values unless history requested
-            //         make sure to not exclude present and future values
+            //         make sure to not exclude present and future values (latest approved and pending)
             //
-            // not only
-            //     exclude APPROVED and not latest
-            //          select * from Structure s where
-            //              not(s.status = 'APPROVED' and s.latest = false)
-            // not only
-            //     exclude earlier than APPROVED and latest (considered history)
-            //     include PENDING and not latest           (considered future)
-            //          select * from Structure s where
-            //                 (not s.id < (select id from structure s2 where s2.uuid = s.uuid and s2.latest = true))
-            //              or (s.status = 'PENDING' and s.latest = false)
-            // instead
-            //     exclude earlier than APPROVED and latest (considered history)
-            //   ( include if latest )
-            //     include if latest not exists             (considered present + future)
-            //         select * from Structure s where
-            //        	      (not s.id < (select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
-            //             or (not  exists(select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
+            // condition(s)
+            //     exclude content (with latest) before latest - considered history
+            //         select * from structure s
+            //         where (
+            //                 not (exists (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true) and s.id < (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true))
+            //         )
+            //         <-->
+            //                 not (exists (subquery id) and s.id < (subquery id))
+            //
+            // note
+            //     persistence libraries may optimize query
+            //         e.g. change (!a and !b) to !(a or b)
 
             Root<Discipline> fromSub = sub.from(Discipline.class);
             sub.where(cb.and(
@@ -229,11 +224,11 @@ public class DisciplineRepository {
                     ));
             sub.select(fromSub.get(Persistable.FIELD_ID));
 
-            Predicate predicateExcludeInclude =
-                    cb.or(
-                            cb.not(cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub)),
-                            cb.not(cb.exists(sub)));
-            predicates.add(predicateExcludeInclude);
+            Predicate predicateExclude =
+                    cb.and(cb.not(cb.and(
+                            cb.exists(sub),
+                            cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub))));
+            predicates.add(predicateExclude);
         }
 
         if (statuses != null) {
diff --git a/src/main/java/org/openepics/names/repository/SubsystemRepository.java b/src/main/java/org/openepics/names/repository/SubsystemRepository.java
index 911ae3ea..9fc3d7d1 100644
--- a/src/main/java/org/openepics/names/repository/SubsystemRepository.java
+++ b/src/main/java/org/openepics/names/repository/SubsystemRepository.java
@@ -206,25 +206,20 @@ public class SubsystemRepository {
             // purpose of Naming to show valid entries
             //     therefore
             //         exclude some values unless history requested
-            //         make sure to not exclude present and future values
+            //         make sure to not exclude present and future values (latest approved and pending)
             //
-            // not only
-            //     exclude APPROVED and not latest
-            //          select * from Structure s where
-            //              not(s.status = 'APPROVED' and s.latest = false)
-            // not only
-            //     exclude earlier than APPROVED and latest (considered history)
-            //     include PENDING and not latest           (considered future)
-            //          select * from Structure s where
-            //                 (not s.id < (select id from structure s2 where s2.uuid = s.uuid and s2.latest = true))
-            //              or (s.status = 'PENDING' and s.latest = false)
-            // instead
-            //     exclude earlier than APPROVED and latest (considered history)
-            //   ( include if latest )
-            //     include if latest not exists             (considered present + future)
-            //         select * from Structure s where
-            //        	      (not s.id < (select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
-            //             or (not  exists(select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
+            // condition(s)
+            //     exclude content (with latest) before latest - considered history
+            //         select * from structure s
+            //         where (
+            //                 not (exists (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true) and s.id < (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true))
+            //         )
+            //         <-->
+            //                 not (exists (subquery id) and s.id < (subquery id))
+            //
+            // note
+            //     persistence libraries may optimize query
+            //         e.g. change (!a and !b) to !(a or b)
 
             Root<Subsystem> fromSub = sub.from(Subsystem.class);
             sub.where(cb.and(
@@ -233,11 +228,11 @@ public class SubsystemRepository {
                     ));
             sub.select(fromSub.get(Persistable.FIELD_ID));
 
-            Predicate predicateExcludeInclude =
-                    cb.or(
-                            cb.not(cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub)),
-                            cb.not(cb.exists(sub)));
-            predicates.add(predicateExcludeInclude);
+            Predicate predicateExclude =
+                    cb.and(cb.not(cb.and(
+                            cb.exists(sub),
+                            cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub))));
+            predicates.add(predicateExclude);
         }
 
         if (statuses != null) {
diff --git a/src/main/java/org/openepics/names/repository/SystemGroupRepository.java b/src/main/java/org/openepics/names/repository/SystemGroupRepository.java
index 0663f83f..f9f3b06c 100644
--- a/src/main/java/org/openepics/names/repository/SystemGroupRepository.java
+++ b/src/main/java/org/openepics/names/repository/SystemGroupRepository.java
@@ -198,25 +198,20 @@ public class SystemGroupRepository {
             // purpose of Naming to show valid entries
             //     therefore
             //         exclude some values unless history requested
-            //         make sure to not exclude present and future values
+            //         make sure to not exclude present and future values (latest approved and pending)
             //
-            // not only
-            //     exclude APPROVED and not latest
-            //          select * from Structure s where
-            //              not(s.status = 'APPROVED' and s.latest = false)
-            // not only
-            //     exclude earlier than APPROVED and latest (considered history)
-            //     include PENDING and not latest           (considered future)
-            //          select * from Structure s where
-            //                 (not s.id < (select id from structure s2 where s2.uuid = s.uuid and s2.latest = true))
-            //              or (s.status = 'PENDING' and s.latest = false)
-            // instead
-            //     exclude earlier than APPROVED and latest (considered history)
-            //   ( include if latest )
-            //     include if latest not exists             (considered present + future)
-            //         select * from Structure s where
-            //        	      (not s.id < (select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
-            //             or (not  exists(select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
+            // condition(s)
+            //     exclude content (with latest) before latest - considered history
+            //         select * from structure s
+            //         where (
+            //                 not (exists (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true) and s.id < (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true))
+            //         )
+            //         <-->
+            //                 not (exists (subquery id) and s.id < (subquery id))
+            //
+            // note
+            //     persistence libraries may optimize query
+            //         e.g. change (!a and !b) to !(a or b)
 
             Root<SystemGroup> fromSub = sub.from(SystemGroup.class);
             sub.where(cb.and(
@@ -225,11 +220,11 @@ public class SystemGroupRepository {
                     ));
             sub.select(fromSub.get(Persistable.FIELD_ID));
 
-            Predicate predicateExcludeInclude =
-                    cb.or(
-                            cb.not(cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub)),
-                            cb.not(cb.exists(sub)));
-            predicates.add(predicateExcludeInclude);
+            Predicate predicateExclude =
+                    cb.and(cb.not(cb.and(
+                            cb.exists(sub),
+                            cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub))));
+            predicates.add(predicateExclude);
         }
 
         if (statuses != null) {
diff --git a/src/main/java/org/openepics/names/repository/SystemRepository.java b/src/main/java/org/openepics/names/repository/SystemRepository.java
index b9edeaf7..75c8107b 100644
--- a/src/main/java/org/openepics/names/repository/SystemRepository.java
+++ b/src/main/java/org/openepics/names/repository/SystemRepository.java
@@ -206,25 +206,20 @@ public class SystemRepository {
             // purpose of Naming to show valid entries
             //     therefore
             //         exclude some values unless history requested
-            //         make sure to not exclude present and future values
+            //         make sure to not exclude present and future values (latest approved and pending)
             //
-            // not only
-            //     exclude APPROVED and not latest
-            //          select * from Structure s where
-            //              not(s.status = 'APPROVED' and s.latest = false)
-            // not only
-            //     exclude earlier than APPROVED and latest (considered history)
-            //     include PENDING and not latest           (considered future)
-            //          select * from Structure s where
-            //                 (not s.id < (select id from structure s2 where s2.uuid = s.uuid and s2.latest = true))
-            //              or (s.status = 'PENDING' and s.latest = false)
-            // instead
-            //     exclude earlier than APPROVED and latest (considered history)
-            //   ( include if latest )
-            //     include if latest not exists             (considered present + future)
-            //         select * from Structure s where
-            //        	      (not s.id < (select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
-            //             or (not  exists(select id from system s2 where s2.uuid = s.uuid and s2.latest = true))
+            // condition(s)
+            //     exclude content (with latest) before latest - considered history
+            //         select * from structure s
+            //         where (
+            //                 not (exists (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true) and s.id < (select s2.id from structure s2 where s2."uuid" = s."uuid" and s2.latest=true))
+            //         )
+            //         <-->
+            //                 not (exists (subquery id) and s.id < (subquery id))
+            //
+            // note
+            //     persistence libraries may optimize query
+            //         e.g. change (!a and !b) to !(a or b)
 
             Root<System> fromSub = sub.from(System.class);
             sub.where(cb.and(
@@ -233,11 +228,11 @@ public class SystemRepository {
                     ));
             sub.select(fromSub.get(Persistable.FIELD_ID));
 
-            Predicate predicateExcludeInclude =
-                    cb.or(
-                            cb.not(cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub)),
-                            cb.not(cb.exists(sub)));
-            predicates.add(predicateExcludeInclude);
+            Predicate predicateExclude =
+                    cb.and(cb.not(cb.and(
+                            cb.exists(sub),
+                            cb.lessThan(from.get(Persistable.FIELD_ID).as(Long.class), sub))));
+            predicates.add(predicateExclude);
         }
 
         if (statuses != null) {
-- 
GitLab