diff --git a/NEWS.md b/NEWS.md index 3978b9f95..7245d3176 100644 --- a/NEWS.md +++ b/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 diff --git a/service/gradle.properties b/service/gradle.properties index fd41ba6af..eaa8369ec 100644 --- a/service/gradle.properties +++ b/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 diff --git a/service/grails-app/controllers/org/olf/ResourceController.groovy b/service/grails-app/controllers/org/olf/ResourceController.groovy index 73591abf9..5b8b1acb0 100644 --- a/service/grails-app/controllers/org/olf/ResourceController.groovy +++ b/service/grails-app/controllers/org/olf/ResourceController.groovy @@ -155,7 +155,72 @@ class ResourceController extends OkapiTenantAwareController { }) 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 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 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 resClass, final ErmResource res -> switch (resClass) { case TitleInstance: diff --git a/service/grails-app/controllers/org/olf/SubscriptionAgreementController.groovy b/service/grails-app/controllers/org/olf/SubscriptionAgreementController.groovy index 9e32bfb46..6b6037415 100644 --- a/service/grails-app/controllers/org/olf/SubscriptionAgreementController.groovy +++ b/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 :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 queryParams = [subscriptionAgreementId: subscriptionAgreementId] + + if (subset in ['current', 'dropped', 'future']) { + queryParams.put('today', today) + } + + final List resIds = PackageContentItem.executeQuery( + buildStaticResourceIdsHQL(subset), + queryParams + ); + final List 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 staticResources () { + respond doStaticResourcesFetch(); + } + + List staticCurrentResources () { + respond doStaticResourcesFetch('current'); + } + + List staticDroppedResources () { + respond doStaticResourcesFetch('dropped'); + } + + List staticFutureResources () { + respond doStaticResourcesFetch('future'); + } private static final Map> CLONE_GROUPING = [ 'agreementInfo': ['name', 'description', 'renewalPriority' , 'isPerpetual'], diff --git a/service/grails-app/controllers/org/olf/UrlMappings.groovy b/service/grails-app/controllers/org/olf/UrlMappings.groovy index 052bb1c3a..3356b29a7 100644 --- a/service/grails-app/controllers/org/olf/UrlMappings.groovy +++ b/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' ) diff --git a/service/grails-app/views/ermResource/_ermResource.gson b/service/grails-app/views/ermResource/_ermResource.gson index 4c335fc1e..8e8e0212c 100644 --- a/service/grails-app/views/ermResource/_ermResource.gson +++ b/service/grails-app/views/ermResource/_ermResource.gson @@ -14,50 +14,54 @@ Map customCoverageMap = request?.getAttribute("${controllerName}.${actionName}.c // Check for custom coverage on this resource. List customCoverageList = customCoverageMap?.get("${ermResource.id}") -json { - - 'id' ermResource.id - 'class' GrailsHibernateUtil.unwrapIfProxy(ermResource).class.name - 'name' ermResource.getName() - 'suppressFromDiscovery' ermResource.suppressFromDiscovery - 'tags' g.render(ermResource.tags) - 'alternateResourceNames' g.render(ermResource.alternateResourceNames) - - RefdataValue rdv = ermResource.getType() - - if (rdv) { - 'type' g.render (rdv) - } - - if (ermResource.publicationType) { - 'publicationType' g.render (ermResource.publicationType) - } +def minimalView = params.minimalView ?: false +if (minimalView && ermResource.class.name == 'org.olf.kb.PackageContentItem') { + json g.render (template: '/packageContentItem/minimalPackageContentItem', model: (binding.variables + [customCoverageList: customCoverageList])) +} else { + json { + 'id' ermResource.id + 'class' ermResource.class.name + 'name' ermResource.getName() - if (ermResource.subType) { - 'subType' g.render (ermResource.subType) - } - - if (customCoverageList) { + 'suppressFromDiscovery' ermResource.suppressFromDiscovery + 'tags' g.render(ermResource.tags) + 'alternateResourceNames' g.render(ermResource.alternateResourceNames) - 'coverage' g.render (customCoverageList) - 'customCoverage' true + RefdataValue rdv = ermResource.getType() - } else if (ermResource.coverage) { + if (rdv) { + 'type' g.render (rdv) + } - 'coverage' g.render (ermResource.coverage) - 'customCoverage' false - - } else { - 'coverage' [] - 'customCoverage' false - } - - if (params.controller == 'export' ) { - // add extra fields for export - + if (ermResource.publicationType) { + 'publicationType' g.render (ermResource.publicationType) + } + + if (ermResource.subType) { + 'subType' g.render (ermResource.subType) + } + + if (customCoverageList) { + + 'coverage' g.render (customCoverageList) + 'customCoverage' true + + } else if (ermResource.coverage) { + + 'coverage' g.render (ermResource.coverage) + 'customCoverage' false + + } else { + 'coverage' [] + 'customCoverage' false + } + + if (params.controller == 'export' ) { + // add extra fields for export + + } + + //Render the full representation of whatever this object is. + '_object' g.render(ermResource) } - - // Render the full representation of whatever this object is. - '_object' g.render(GrailsHibernateUtil.unwrapIfProxy(ermResource)) - } diff --git a/service/grails-app/views/packageContentItem/_minimalPackageContentItem.gson b/service/grails-app/views/packageContentItem/_minimalPackageContentItem.gson new file mode 100644 index 000000000..05ac959f7 --- /dev/null +++ b/service/grails-app/views/packageContentItem/_minimalPackageContentItem.gson @@ -0,0 +1,36 @@ +import org.olf.kb.PackageContentItem + +import groovy.transform.* + +@Field +PackageContentItem packageContentItem + +@Field +List customCoverageList + +json { + 'tiName' packageContentItem.pti.titleInstance.name + 'platformUrl' packageContentItem.pti.url + 'platformName' packageContentItem.pti.platform.name + + identifiers (packageContentItem.pti.titleInstance.approvedIdentifierOccurrences) {IdentifierOccurrence occurrence -> + identifier g.render(occurrence.identifier) + } + 'pkg' packageContentItem.pkg.name + 'accessStart' packageContentItem.accessStart + 'accessEnd' packageContentItem.accessEnd + + if (customCoverageList) { + 'coverage' g.render (customCoverageList) + 'customCoverage' true + + } else if (packageContentItem.coverage) { + + 'coverage' g.render (packageContentItem.coverage) + 'customCoverage' false + + } else { + 'coverage' [] + 'customCoverage' false + } +} \ No newline at end of file diff --git a/service/grails-app/views/pkg/_pkg.gson b/service/grails-app/views/pkg/_pkg.gson index 0b4fe6e12..63eb392db 100644 --- a/service/grails-app/views/pkg/_pkg.gson +++ b/service/grails-app/views/pkg/_pkg.gson @@ -5,7 +5,25 @@ import groovy.transform.* @Field Pkg pkg -json g.render(pkg, [expand: ['remoteKb','vendor', 'type', 'subType', 'lifecycleStatus', 'availabilityScope', 'packageDescriptionUrls', 'contentTypes', 'alternateResourceNames', 'availabilityConstraints'], excludes: ['contentItems', 'identifiers']]) { +final def should_expand = [ + 'remoteKb', + 'type', + 'subType', + 'lifecycleStatus', + 'availabilityScope', + 'packageDescriptionUrls', + 'contentTypes', + 'alternateResourceNames', + 'availabilityConstraints', + 'vendor' +]; + +final def should_exclude = [ + 'contentItems', + 'identifiers' +]; + +json g.render(pkg, [expand: should_expand, excludes: should_exclude]) { resourceCount pkg.getResourceCount() 'class' Pkg.name diff --git a/service/grails-app/views/resource/staticEntitlementOptions.gson b/service/grails-app/views/resource/staticEntitlementOptions.gson new file mode 100644 index 000000000..2cbd1c6e8 --- /dev/null +++ b/service/grails-app/views/resource/staticEntitlementOptions.gson @@ -0,0 +1 @@ +json g.render (template: 'resourceList', model: (binding.variables)) \ No newline at end of file diff --git a/service/grails-app/views/subscriptionAgreement/staticCurrentResources.gson b/service/grails-app/views/subscriptionAgreement/staticCurrentResources.gson new file mode 100644 index 000000000..6d3e3a479 --- /dev/null +++ b/service/grails-app/views/subscriptionAgreement/staticCurrentResources.gson @@ -0,0 +1 @@ +json g.render (template: '/resource/resourceList', model: (binding.variables)) \ No newline at end of file diff --git a/service/grails-app/views/subscriptionAgreement/staticDroppedResources.gson b/service/grails-app/views/subscriptionAgreement/staticDroppedResources.gson new file mode 100644 index 000000000..6d3e3a479 --- /dev/null +++ b/service/grails-app/views/subscriptionAgreement/staticDroppedResources.gson @@ -0,0 +1 @@ +json g.render (template: '/resource/resourceList', model: (binding.variables)) \ No newline at end of file diff --git a/service/grails-app/views/subscriptionAgreement/staticFutureResources.gson b/service/grails-app/views/subscriptionAgreement/staticFutureResources.gson new file mode 100644 index 000000000..6d3e3a479 --- /dev/null +++ b/service/grails-app/views/subscriptionAgreement/staticFutureResources.gson @@ -0,0 +1 @@ +json g.render (template: '/resource/resourceList', model: (binding.variables)) \ No newline at end of file diff --git a/service/grails-app/views/subscriptionAgreement/staticResources.gson b/service/grails-app/views/subscriptionAgreement/staticResources.gson new file mode 100644 index 000000000..6d3e3a479 --- /dev/null +++ b/service/grails-app/views/subscriptionAgreement/staticResources.gson @@ -0,0 +1 @@ +json g.render (template: '/resource/resourceList', model: (binding.variables)) \ No newline at end of file diff --git a/service/src/main/groovy/org/olf/dataimport/internal/titleInstanceResolvers/BaseTIRS.groovy b/service/src/main/groovy/org/olf/dataimport/internal/titleInstanceResolvers/BaseTIRS.groovy index 772fe3412..e3250f679 100644 --- a/service/src/main/groovy/org/olf/dataimport/internal/titleInstanceResolvers/BaseTIRS.groovy +++ b/service/src/main/groovy/org/olf/dataimport/internal/titleInstanceResolvers/BaseTIRS.groovy @@ -1,5 +1,7 @@ package org.olf.dataimport.internal.titleInstanceResolvers +import com.k_int.web.toolkit.refdata.RefdataValue + import org.olf.IdentifierService import org.olf.dataimport.internal.PackageContentImpl @@ -120,7 +122,7 @@ abstract class BaseTIRS implements TitleInstanceResolverService { * If the "Authoritative" publication type is not equal to whatever mad value a remote site has sent then * replace the authortiative value with the one sent? */ - if (title.publicationType?.value != citation.instancePublicationMedia) { + if (title.publicationType?.value != RefdataValue.normValue(citation.instancePublicationMedia)) { if (citation.instancePublicationMedia) { title.publicationTypeFromString = citation.instancePublicationMedia } else { diff --git a/service/src/main/okapi/ModuleDescriptor-template.json b/service/src/main/okapi/ModuleDescriptor-template.json index 172def207..6b48fea4b 100644 --- a/service/src/main/okapi/ModuleDescriptor-template.json +++ b/service/src/main/okapi/ModuleDescriptor-template.json @@ -300,6 +300,11 @@ "pathPattern": "/erm/resource/{id}/entitlementOptions", "permissionsRequired": [ "erm.resources.item.entitlementOptions.get" ] }, + { + "methods": ["GET"], + "pathPattern": "/erm/resource/{id}/static/entitlementOptions", + "permissionsRequired": [ "erm.resources.item.entitlementOptions.get" ] + }, { "methods": ["GET"], "pathPattern": "/erm/titles",