Merge pull request #772 from folio-org/release_6.1.4

Release 6.1.4

NEWS.md

@@ -1,3 +1,8 @@
+## 6.1.4 2024-05-26
+  * ERM-3246 Improve performance of entitlementOptions endpoint
+  * ERM-3187 Re-write query to show "list of resources in an agreement" for improved performance
+  * Titles previously _always_ enriched, sometimes multiple times, on ingest. Fixed this behaviour where no changes occur
+
 ## 6.1.3 2024-05-08
   * ERM-3185 Remove PTI/PCI filter from the /titles/electronic endpoint in Agreements
   * ERM-3166 On encountering a GOKb title with the same ISSN assigned as both print and electronic ISSN, ingest stops

service/gradle.properties

@@ -5,7 +5,7 @@ gorm.version=7.3.3
 
 # Application
 appName=mod-agreements
-appVersion=6.1.3
+appVersion=6.1.4
 dockerTagSuffix=
 dockerRepo=folioci
 

service/grails-app/controllers/org/olf/ResourceController.groovy

@@ -155,7 +155,72 @@ class ResourceController extends OkapiTenantAwareController<ErmResource> {
     })
     log.debug("completed in ${Duration.between(start, Instant.now()).toSeconds()} seconds")
   }
-
+
+  private String buildStaticEntitlementOptionsHQL(Boolean isCount = false) {
+    return """
+      SELECT ${isCount ? 'COUNT(res.id)' : 'res'} FROM ErmResource as res WHERE
+      res.id IN :flattenedIds
+    """.toString();
+  }
+
+  // I'd like to move this "static fetch" code into a shared space if we get a chance before some kind of OS/ES implementation
+  @Transactional(readOnly=true)
+  def staticEntitlementOptions (final String resourceId) {
+    //final String resourceId = params.get("resourceId")
+    final Integer perPage = (params.get("perPage") ?: "10").toInteger();
+
+    // Funky things will happen if you pass 0 or negative numbers...
+    final Integer page = (params.get("page") ?: "1").toInteger();
+
+    if (resourceId) {
+      // Splitting this into two queries to avoid unnecessary joins. Wish we could do this in one but there's
+      // seemingly no way to flatten results like this
+      List<String> flattenedIds = PackageContentItem.executeQuery("""
+        SELECT pci.id, pci.pkg.id FROM PackageContentItem as pci WHERE
+          pci.removedTimestamp IS NULL AND
+          pci.pti.titleInstance.id = :resourceId
+      """.toString(), [resourceId: resourceId]).flatten()
+
+      final List<PackageContentItem> results = PackageContentItem.executeQuery(
+        buildStaticEntitlementOptionsHQL(),
+        [
+          flattenedIds: flattenedIds,
+        ],
+        [
+          max: perPage,
+          offset: (page - 1) * perPage
+          //readOnly: true -- handled in the transaction, no?
+        ]
+      );
+
+      if (params.boolean('stats')) {
+        final Integer count = PackageContentItem.executeQuery(
+          buildStaticEntitlementOptionsHQL(true),
+          [
+            flattenedIds: flattenedIds,
+          ]
+        )[0].toInteger();
+
+        final def resultsMap = [
+          pageSize: perPage,
+          page: page,
+          totalPages: ((int)(count / perPage) + (count % perPage == 0 ? 0 : 1)),
+          meta: [:], // Idk what this is for
+          totalRecords: count,
+          total: count,
+          results: results
+        ];
+
+        // respond with full result set
+        respond resultsMap;
+      } else {
+
+        // Respond the list of items
+        respond results
+      }
+    }
+  }
+
   private final Closure entitlementCriteria = { final Class<? extends ErmResource> resClass, final ErmResource res ->
     switch (resClass) {
       case TitleInstance:

service/grails-app/controllers/org/olf/SubscriptionAgreementController.groovy

@@ -8,6 +8,7 @@ import org.olf.kb.ErmResource
 import org.olf.kb.PackageContentItem
 import org.olf.kb.Pkg
 import org.olf.kb.PlatformTitleInstance
+import org.olf.erm.Entitlement
 
 import com.k_int.okapi.OkapiTenantAwareController
 
@@ -18,6 +19,8 @@ import groovy.util.logging.Slf4j
 
 import static org.springframework.http.HttpStatus.*
 
+import java.time.Duration
+import java.time.Instant
 
 /**
  * Control access to subscription agreements.
@@ -533,7 +536,210 @@ class SubscriptionAgreementController extends OkapiTenantAwareController<Subscri
       return
     }
   }
+
+  private String buildStaticResourceIdsHQL(String subset) {
+    String topLine = """SELECT res.id FROM PackageContentItem as res""";
+
+    switch (subset) {
+      case 'current':
+        return """${topLine}
+          LEFT OUTER JOIN res.entitlements AS direct_ent
+          LEFT OUTER JOIN res.pkg.entitlements AS pkg_ent
+        WHERE
+          (
+            direct_ent.owner.id = :subscriptionAgreementId AND
+            (
+                direct_ent.activeFrom IS NULL OR
+                direct_ent.activeFrom < :today
+              ) AND
+              (
+                direct_ent.activeTo IS NULL OR
+                direct_ent.activeTo > :today
+              )
+          ) OR
+          (
+            res.removedTimestamp IS NULL AND
+            pkg_ent.owner.id = :subscriptionAgreementId AND
+            (
+              pkg_ent.activeFrom IS NULL OR
+              pkg_ent.activeFrom < :today
+            ) AND
+            (
+              pkg_ent.activeTo IS NULL OR
+              pkg_ent.activeTo > :today
+            ) AND
+            (
+              res.accessStart IS NULL OR
+              res.accessStart < :today
+            ) AND
+            (
+              res.accessEnd IS NULL OR
+              res.accessEnd > :today
+            )
+          )
+        """.toString();
+      case 'dropped':
+        return """${topLine}
+          LEFT OUTER JOIN res.entitlements AS direct_ent
+          LEFT OUTER JOIN res.pkg.entitlements AS pkg_ent
+        WHERE
+          (
+            direct_ent.owner.id = :subscriptionAgreementId AND
+            direct_ent.activeTo < :today
+          ) OR
+          (
+            res.removedTimestamp IS NULL AND
+            pkg_ent.owner.id = :subscriptionAgreementId AND
+            (
+              res.accessStart IS NULL OR
+              pkg_ent.activeTo IS NULL OR
+              res.accessStart < pkg_ent.activeTo
+            ) AND
+            (
+              res.accessEnd IS NULL OR
+              pkg_ent.activeFrom IS NULL OR
+              res.accessEnd > pkg_ent.activeFrom
+            ) AND
+            (
+              pkg_ent.activeTo < :today OR
+              res.accessEnd < :today
+            )
+          )
+        """.toString();
+      case 'future':
+        return """${topLine}
+          LEFT OUTER JOIN res.entitlements AS direct_ent
+          LEFT OUTER JOIN res.pkg.entitlements AS pkg_ent
+        WHERE
+          (
+            direct_ent.owner.id = :subscriptionAgreementId AND
+            direct_ent.activeFrom > :today
+          ) OR
+          (
+            res.removedTimestamp IS NULL AND
+            pkg_ent.owner.id = :subscriptionAgreementId AND
+            (
+              res.accessStart IS NULL OR
+              pkg_ent.activeTo IS NULL OR
+              res.accessStart < pkg_ent.activeTo
+            ) AND
+            (
+              res.accessEnd IS NULL OR
+              pkg_ent.activeFrom IS NULL OR
+              res.accessEnd > pkg_ent.activeFrom
+            ) AND
+            (
+              pkg_ent.activeFrom > :today OR
+              res.accessStart > :today
+            )
+          )
+        """.toString();
+      case 'all':
+      default:
+        return """${topLine}
+          LEFT OUTER JOIN res.entitlements AS direct_ent
+          LEFT OUTER JOIN res.pkg.entitlements AS pkg_ent
+        WHERE
+          (
+            direct_ent.owner.id = :subscriptionAgreementId
+          ) OR (
+            res.removedTimestamp IS NULL AND
+            pkg_ent.owner.id = :subscriptionAgreementId
+          )
+        """.toString()
+    }
+  }
+
+  // Subset can be "all", "current", "dropped" or "future"
+  // ASSUMES there is a subscription agreement
+  private String buildStaticResourceHQL(Boolean isCount = false) {
+    String topLine = """
+      SELECT ${isCount ? 'COUNT(res.id)' : 'res'} FROM PackageContentItem as res
+      WHERE res.id IN :resIds
+      ${isCount ? '' : 'ORDER BY res.pti.titleInstance.name'}
+    """.toString();
+  }
+
+  // I'd like to move this "static fetch" code into a shared space if we get a chance before some kind of OS/ES implementation
+  @Transactional(readOnly=true)
+  private def doStaticResourcesFetch (final String subset = 'all') {
+    final String subscriptionAgreementId = params.get("subscriptionAgreementId")
+    final Integer perPage = (params.get("perPage") ?: "10").toInteger();
+
+    // Funky things will happen if you pass 0 or negative numbers...
+    final Integer page = (params.get("page") ?: "1").toInteger();
+
+    if (subscriptionAgreementId) {
+      // Now
+      final LocalDate today = LocalDate.now()
+      Map<String, Object> queryParams = [subscriptionAgreementId: subscriptionAgreementId]
+
+      if (subset in ['current', 'dropped', 'future']) {
+        queryParams.put('today', today)
+      }
+
+      final List<String> resIds = PackageContentItem.executeQuery(
+        buildStaticResourceIdsHQL(subset),
+        queryParams
+      );
 
+      final List<PackageContentItem> results = PackageContentItem.executeQuery(
+        buildStaticResourceHQL(),
+        [resIds: resIds],
+        [
+          max: perPage,
+          offset: (page - 1) * perPage
+          //readOnly: true -- handled in the transaction, no?
+        ]
+      );
+
+      if (params.boolean('stats')) {
+        final Integer count = PackageContentItem.executeQuery(
+          buildStaticResourceHQL(true),
+          [resIds: resIds],
+        )[0].toInteger();
+
+        final def resultsMap = [
+          pageSize: perPage,
+          page: page,
+          totalPages: ((int)(count / perPage) + (count % perPage == 0 ? 0 : 1)),
+          meta: [:], // Idk what this is for
+          totalRecords: count,
+          total: count,
+          results: results
+        ];
+        // This method writes to the web request if there is one (which of course there should be as we are in a controller method)
+        coverageService.lookupCoverageOverrides(resultsMap, "${subscriptionAgreementId}");
+
+        // respond with full result set
+        return resultsMap;
+      } else {
+
+        final def resultsMap = [ results: results ];
+        // This method writes to the web request if there is one (which of course there should be as we are in a controller method)
+        coverageService.lookupCoverageOverrides(resultsMap, "${subscriptionAgreementId}")
+
+        // Respond the list of items
+        return results
+      }
+    }
+  }
+
+  List<ErmResource> staticResources () {
+    respond doStaticResourcesFetch();
+  }
+
+  List<ErmResource> staticCurrentResources () {
+    respond doStaticResourcesFetch('current');
+  }
+
+  List<ErmResource> staticDroppedResources () {
+    respond doStaticResourcesFetch('dropped');
+  }
+
+  List<ErmResource> staticFutureResources () {
+    respond doStaticResourcesFetch('future');
+  }
 
   private static final Map<String, List<String>> CLONE_GROUPING = [
     'agreementInfo': ['name', 'description', 'renewalPriority' , 'isPerpetual'],

service/grails-app/controllers/org/olf/UrlMappings.groovy

@@ -23,6 +23,13 @@ class UrlMappings {
       "/resources/future"     (action: 'futureResources', method: 'GET')
       "/resources/dropped"    (action: 'droppedResources', method: 'GET')
 
+      // The "static" versions will ONLY return PCIs, not PTIs linked directly (for now)
+      "/resources/static"    (action: 'staticResources', method: 'GET')
+      "/resources/static/all"    (action: 'staticResources', method: 'GET')
+      "/resources/static/current"    (action: 'staticCurrentResources', method: 'GET')
+      "/resources/static/dropped"    (action: 'staticDroppedResources', method: 'GET')
+      "/resources/static/future"    (action: 'staticFutureResources', method: 'GET')
+
       "/resources/export/$format?"          (controller: 'export', method: 'GET')
       "/resources/export/current/$format?"  (controller: 'export', action: 'current', method: 'GET')
       "/resources/export/all/$format?"  (controller: 'export', action: 'index', method: 'GET')
@@ -155,6 +162,7 @@ class UrlMappings {
       collection {
         "/electronic" ( action:'electronic', method: 'GET')
       }
+      "/static/entitlementOptions" ( action:'staticEntitlementOptions', method: 'GET')
       "/entitlementOptions" ( action:'entitlementOptions', method: 'GET')
       "/entitlements" ( action:'entitlements', method: 'GET' )
       "/entitlements/related" ( action:'relatedEntitlements', method: 'GET' )