From 95853bd6428ec7c310f0546762ba973d612edbec Mon Sep 17 00:00:00 2001 From: EthanFreestone <54310740+EthanFreestone@users.noreply.github.com> Date: Wed, 27 Mar 2024 15:14:55 +0000 Subject: [PATCH 01/12] fix: ERM-3177: Check on `noSICount` in WorkSourceIdentifierTIRSImpl is not correct (#737) Previously WorkSourceIdentifierTIRS used check `work.sourceIdentifier = null` in the HQL statement, assuming that the Hibernate query language and GORM would play nicely for null check. This was incorrect however, so it has been swapped for a more direct count, of the number of Works LEFT JOIN the IdentifierOccurrences, where the IdentifierOccurrence is null. NOTE: This will only work so long as Work only has a single IdentifierOccurrence attached to it in this manner. Reworks will be needed if this changes. ERM-3177 --- .../WorkSourceIdentifierTIRSImpl.groovy | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/service/src/main/groovy/org/olf/dataimport/internal/titleInstanceResolvers/WorkSourceIdentifierTIRSImpl.groovy b/service/src/main/groovy/org/olf/dataimport/internal/titleInstanceResolvers/WorkSourceIdentifierTIRSImpl.groovy index 369724d8..ffd29eec 100644 --- a/service/src/main/groovy/org/olf/dataimport/internal/titleInstanceResolvers/WorkSourceIdentifierTIRSImpl.groovy +++ b/service/src/main/groovy/org/olf/dataimport/internal/titleInstanceResolvers/WorkSourceIdentifierTIRSImpl.groovy @@ -45,13 +45,19 @@ class WorkSourceIdentifierTIRSImpl extends IdFirstTIRSImpl implements DataBinder return false; } + // This assumes that IdentiferOccurrences can only be attached to Works as the sourceIdentifier. + // If this changes then rework will be needed. long noSICount = Work.executeQuery(""" - SELECT COUNT(w) FROM Work w WHERE w.sourceIdentifier = null - """.toString())?.get(0); + SELECT COUNT(w) FROM Work w + LEFT JOIN IdentifierOccurrence io ON io.resource = w.id + WHERE io.id = null + """.toString())?.get(0); +/* long workCount = Work.executeQuery(""" SELECT COUNT(w) FROM Work w """.toString())?.get(0); +*/ // For now keep the fallback unless there are NO Works in the system without SI if (noSICount == 0) { @@ -74,7 +80,6 @@ class WorkSourceIdentifierTIRSImpl extends IdFirstTIRSImpl implements DataBinder // Error out if sourceIdentifier or sourceIdentifierNamespace do not exist ensureSourceIdentifierFields(citation); - // TODO Check this works with the switch to grabbing only id List candidate_works = Work.executeQuery(""" SELECT w.id FROM Work as w WHERE From 75a7bebb84d1caf4dc9db66f17f0550ce1de11b5 Mon Sep 17 00:00:00 2001 From: Jack-Golding <94838251+Jack-Golding@users.noreply.github.com> Date: Tue, 2 Apr 2024 10:49:41 +0100 Subject: [PATCH 02/12] fix: Source title count default (#738) Changed the behaviour for when a package doesnt have a current title count it will be instanciated at 0 as opposed to null --- service/grails-app/services/org/olf/PackageIngestService.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/grails-app/services/org/olf/PackageIngestService.groovy b/service/grails-app/services/org/olf/PackageIngestService.groovy index 0bca7947..bbd974b3 100644 --- a/service/grails-app/services/org/olf/PackageIngestService.groovy +++ b/service/grails-app/services/org/olf/PackageIngestService.groovy @@ -278,7 +278,7 @@ class PackageIngestService implements DataBinder { description: package_data.header.description, sourceDataCreated: package_data.header.sourceDataCreated, sourceDataUpdated: package_data.header.sourceDataUpdated, - sourceTitleCount: (package_data.header.sourceTitleCount ?: null), + sourceTitleCount: package_data.header.sourceTitleCount, availabilityScope: ( package_data.header.availabilityScope != null ? Pkg.lookupOrCreateAvailabilityScope(package_data.header.availabilityScope) : null ), lifecycleStatus: Pkg.lookupOrCreateLifecycleStatus(package_data.header.lifecycleStatus != null ? package_data.header.lifecycleStatus : 'Unknown'), vendor: vendor, From ca8a5eb4f272418976e4dce1278eb510104bbbd8 Mon Sep 17 00:00:00 2001 From: Jack-Golding <94838251+Jack-Golding@users.noreply.github.com> Date: Tue, 2 Apr 2024 11:06:48 +0100 Subject: [PATCH 03/12] refactor: Additional info.log messages for jobs/processes (#739) Changed log.warn for the interrupted job exception to log.info ERM-3139 Co-authored-by: EthanFreestone <54310740+EthanFreestone@users.noreply.github.com> --- service/grails-app/services/org/olf/KbHarvestService.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/grails-app/services/org/olf/KbHarvestService.groovy b/service/grails-app/services/org/olf/KbHarvestService.groovy index 26a4ce61..8cc21a0d 100644 --- a/service/grails-app/services/org/olf/KbHarvestService.groovy +++ b/service/grails-app/services/org/olf/KbHarvestService.groovy @@ -260,7 +260,7 @@ where rkb.type is not null // This might ought to be in debug to bring in line with other methods in here, // although it might be more useful to know it happened since the call log above is "debug" level // Default logback config has this service at "DEBUG" level anyway so it might not matter. - log.warn("KBHarvestService::handleInterruptedJob() completed"); + log.info("KBHarvestService::handleInterruptedJob() completed"); } } } From 46fdda52815924cad7c633a7376b477a9000e482 Mon Sep 17 00:00:00 2001 From: EthanFreestone <54310740+EthanFreestone@users.noreply.github.com> Date: Tue, 2 Apr 2024 12:56:19 +0100 Subject: [PATCH 04/12] build: Dependency upgrades (#740) Updated dependencies as listed in this comment: https://folio-org.atlassian.net/browse/ERM-3174?focusedCommentId=206688 This prevents several security issues ERM-3174 --- service/build.gradle | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/service/build.gradle b/service/build.gradle index 1c7fecc9..09346dc6 100644 --- a/service/build.gradle +++ b/service/build.gradle @@ -99,7 +99,12 @@ dependencies { implementation("org.grails.plugins:async") implementation("org.grails.plugins:events") implementation("org.grails.plugins:hibernate5") - implementation("org.grails.plugins:spring-security-core:6.1.1") // NOT IN LINE WITH GRAILS PATCH VERSION + implementation("org.grails.plugins:spring-security-core:6.1.1") /* { // NOT IN LINE WITH GRAILS PATCH VERSION + exclude group: 'org.springframework.security', module: 'spring-security-core' + } */ + // 5.8.9 affected by https://security.snyk.io/vuln/SNYK-JAVA-ORGSPRINGFRAMEWORKSECURITY-6457293 + implementation("org.springframework.security:spring-security-core:5.8.11") + implementation("org.grails.plugins:views-json") implementation("org.grails.plugins:views-json-templates") implementation("org.hibernate:hibernate-core:5.6.15.Final") @@ -129,7 +134,7 @@ dependencies { implementation "org.springframework.boot:spring-boot-starter-undertow" // Replaces spring-boot-starter-tomcat implementation "org.hibernate:hibernate-java8" runtimeOnly "com.zaxxer:HikariCP:5.1.0" // Replaces Tomcat JDBC pool - runtimeOnly "org.postgresql:postgresql:42.5.3" + runtimeOnly "org.postgresql:postgresql:42.7.3" implementation ('org.grails.plugins:database-migration:4.2.1') { exclude group: 'org.liquibase', module: 'liquibase-core' } @@ -137,9 +142,12 @@ dependencies { implementation 'com.opencsv:opencsv:5.7.1' - implementation 'commons-io:commons-io:2.6' - implementation 'io.github.virtualdogbert:logback-groovy-config:1.14.1' // Grails 5 and up no longer supports groovy files for logback config + implementation 'commons-io:commons-io:2.7' + // Grails 5 and up no longer supports groovy files for logback config + implementation('io.github.virtualdogbert:logback-groovy-config:1.14.1') compileOnly 'ch.qos.logback:logback-classic:1.4.7' + // Is on runtime classpath via spring-boot-starter + runtimeOnly 'ch.qos.logback:logback-classic:1.2.13' implementation 'uk.co.cacoethes:groovy-handlebars-engine:0.2' implementation 'com.github.jknack:handlebars-helpers:4.3.1' @@ -164,8 +172,8 @@ dependencies { implementation "org.seleniumhq.selenium:selenium-firefox-driver:3.14.0" /* ---- Custom non profile deps ---- */ - implementation 'org.apache.kafka:kafka-clients:2.3.0' - implementation 'com.github.everit-org.json-schema:org.everit.json.schema:1.14.3' + implementation 'org.apache.kafka:kafka-clients:3.7.0' + implementation 'com.github.everit-org.json-schema:org.everit.json.schema:1.14.4' // Better test reports. testImplementation( 'com.athaydes:spock-reports:2.3.2-groovy-3.0' ) { transitive = false // this avoids affecting your version of Groovy/Spock From 38937062b7277c9e8255db10238035ab735d3586 Mon Sep 17 00:00:00 2001 From: Jack-Golding <94838251+Jack-Golding@users.noreply.github.com> Date: Mon, 8 Apr 2024 11:36:58 +0100 Subject: [PATCH 05/12] ERM-3157: Missed declaration of backend permissions fix: Removed unused codex endpoints (#742) Codex permissions were previously removed but codex endpoints were still included in the module descriptor, these have now been removed ERM-3157 --- .../main/okapi/ModuleDescriptor-template.json | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/service/src/main/okapi/ModuleDescriptor-template.json b/service/src/main/okapi/ModuleDescriptor-template.json index 172def20..1c476354 100644 --- a/service/src/main/okapi/ModuleDescriptor-template.json +++ b/service/src/main/okapi/ModuleDescriptor-template.json @@ -513,22 +513,6 @@ } ] }, - { - "id" : "codex", - "version" : "3.0", - "interfaceType": "multiple", - "handlers" : [ - { - "methods" : [ "GET" ], - "pathPattern" : "/codex-instances", - "permissionsRequired" : [ "codex.collection.get" ] - }, { - "methods" : [ "GET" ], - "pathPattern" : "/codex-instances/{id}", - "permissionsRequired" : [ "codex.item.get" ] - } - ] - }, { "id": "_timer", "version": "1.0", From 28c15f3d989c19b914a39a2b16a7ffdedc365bc3 Mon Sep 17 00:00:00 2001 From: Jack-Golding <94838251+Jack-Golding@users.noreply.github.com> Date: Mon, 8 Apr 2024 13:28:39 +0100 Subject: [PATCH 06/12] ERM-3191: Add logging to support issue triage for HibernateOptimisticLockingFailureException (#743) * refactor: Remotekb id warning Expanded upon the runsync warning to include the remotekb id * fix: Added error logging to warn message --------- Co-authored-by: EthanFreestone <54310740+EthanFreestone@users.noreply.github.com> --- service/grails-app/services/org/olf/KbHarvestService.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/grails-app/services/org/olf/KbHarvestService.groovy b/service/grails-app/services/org/olf/KbHarvestService.groovy index 8cc21a0d..043355b7 100644 --- a/service/grails-app/services/org/olf/KbHarvestService.groovy +++ b/service/grails-app/services/org/olf/KbHarvestService.groovy @@ -163,7 +163,7 @@ where rkb.type is not null knowledgeBaseCacheService.runSync((String)remotekb_id) } catch ( Exception e ) { - log.warn("problem processing remote KB link",e) + log.warn("Unexpected exception encountered during runSync for ${remotekb_id}", e) } finally { log.info("KbHarvestService.closure completed - ${System.currentTimeMillis()-gokb_sync_start_time}ms elapsed. Release sync status"); From b799b897edb451d186820c82890c365a843a43dc Mon Sep 17 00:00:00 2001 From: EthanFreestone <54310740+EthanFreestone@users.noreply.github.com> Date: Mon, 8 Apr 2024 15:45:49 +0100 Subject: [PATCH 07/12] ERM-3181: sourceTitleCount should not be required on uploading packages via JSON using mod-agreements-package schema (#744) * fix: ERM-3181: sourceTitleCount should not be required on uploading packages via JSON using mod-agreements-package schema Tweaked packageSchema to allow packages imported via ERM schema to _not_ provide a sourceTitleCount. Also made tweak to import logging to better reflect the number of packages imported, as it previously defaulted to "imported: true" whether the package made it past validation or not ERM-3181 * chore: Spacing Small tweak to spacing of code --- service/grails-app/services/org/olf/ImportService.groovy | 4 ++-- .../groovy/org/olf/dataimport/erm/ErmPackageImpl.groovy | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/service/grails-app/services/org/olf/ImportService.groovy b/service/grails-app/services/org/olf/ImportService.groovy index dc71ad32..a46fbe84 100644 --- a/service/grails-app/services/org/olf/ImportService.groovy +++ b/service/grails-app/services/org/olf/ImportService.groovy @@ -97,7 +97,7 @@ class ImportService implements DataBinder { envelope.records?.each { Map record -> // Ingest 1 package at a time. Map importResult = importPackage (record, ErmPackageImplWithContentItems) - + if (importResult.packageImported) { packageCount ++ String packageId = importResult.packageId @@ -115,7 +115,7 @@ class ImportService implements DataBinder { } private Map importPackage (final Map record, final Class schemaClass) { - boolean packageImported = true + boolean packageImported = false String packageId = "" final PackageSchema pkg = schemaClass.newInstance() diff --git a/service/src/main/groovy/org/olf/dataimport/erm/ErmPackageImpl.groovy b/service/src/main/groovy/org/olf/dataimport/erm/ErmPackageImpl.groovy index cbaf749f..fbe9775b 100644 --- a/service/src/main/groovy/org/olf/dataimport/erm/ErmPackageImpl.groovy +++ b/service/src/main/groovy/org/olf/dataimport/erm/ErmPackageImpl.groovy @@ -56,13 +56,14 @@ class ErmPackageImpl implements PackageHeaderSchema, PackageSchema, Validateable trustedSourceTI nullable: true description nullable: true - source nullable: false, blank: false - reference nullable: false, blank: false - name nullable: false, blank: false - packageProvider nullable: true + source nullable: false, blank: false + reference nullable: false, blank: false + name nullable: false, blank: false + packageProvider nullable: true availabilityScope nullable: true, blank: false sourceDataCreated nullable: true, blank: false sourceDataUpdated nullable: true, blank: false + sourceTitleCount nullable: true, blank: false lifecycleStatus nullable: true, blank: false } From c87097add13f6add84b022dab57d64c723e53334 Mon Sep 17 00:00:00 2001 From: EthanFreestone <54310740+EthanFreestone@users.noreply.github.com> Date: Thu, 11 Apr 2024 16:30:40 +0100 Subject: [PATCH 08/12] ERM-3193 (#747) * fix: WIP -- fix for saving String Templates Explicitly separate events for pre-insert/update/delete, and for insert events _only_ send special instruction to avoid fetching "current" idScopes from session. Need to check what behaviour was in previous versions of grails, but gut instinct says those "previous" scopes should be an empty set, yet in Grails 6 they seem to fire another pre-insert event, causing an infinite loop ERM-3193 * refactor: Cleanup code for previous solution Cleaned up dangling log debugs and slightly tweaked the way StringTemplatingService checks if an event is a pre-insert ERM-3193 * chore: Log debug removed Extra missed log debug statement --- .../general/StringTemplatingService.groovy | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/service/grails-app/services/org/olf/general/StringTemplatingService.groovy b/service/grails-app/services/org/olf/general/StringTemplatingService.groovy index 490647e3..2de08f0b 100644 --- a/service/grails-app/services/org/olf/general/StringTemplatingService.groovy +++ b/service/grails-app/services/org/olf/general/StringTemplatingService.groovy @@ -323,7 +323,6 @@ public class StringTemplatingService implements ApplicationListener theScopes = (event.eventType != PreDelete ? template.idScopes : Collections.emptySet()) as Set - // Context final String theContext = template.context.value // Fetch the scopes from the Session to get "Previous" values - final Set previousScopes = getScopesForStringTemplate(template.id) + /* For some reason pre insert events making a query end up re-raising + * the pre insert event in Grails 6, causing an infinite loop. + * However in this case a pre-insert event means that no + * "previous scopes" exist for the string template in question, so + * assume an empty set and move on with the logic + */ + Set previousScopes = [] as Set; + if (event.eventType != PreInsert) { + previousScopes = getScopesForStringTemplate(template.id) + } // Explicitly re-process the differences between the scope now and the previous final Set reprocess = ( theScopes + previousScopes ) - theScopes.intersect(previousScopes) @@ -587,9 +594,9 @@ public class StringTemplatingService implements ApplicationListener) { final String tenantId, final Set scopes, final String context, final Set overrides, StringTemplate tmpl -> - Tenants.withId(tenantId) { GormStaticApi ptiApi = GormUtils.gormStaticApi(Platform) List platformIds = getAllPlatformIdsForTemplateParams(scopes, context, overrides) @@ -625,6 +632,7 @@ public class StringTemplatingService implements ApplicationListener inEx -> Tenants.withId(tid) { @@ -635,12 +643,11 @@ public class StringTemplatingService implements ApplicationListener getScopesForStringTemplate ( final String templateId ) { - final String hql = ''' SELECT scope FROM StringTemplate tmp JOIN tmp.idScopes AS scope WHERE tmp.id = :templateId ''' - GormUtils.gormStaticApi(StringTemplate).executeQuery(hql, ['templateId': templateId]) as Set + return GormUtils.gormStaticApi(StringTemplate).executeQuery(hql.toString(), ['templateId': templateId]) as Set } } From 7e3dc067d61fa73e49af97608583a65f5a813189 Mon Sep 17 00:00:00 2001 From: Claudia Malzer <42971992+CalamityC@users.noreply.github.com> Date: Fri, 12 Apr 2024 11:17:42 +0200 Subject: [PATCH 09/12] ERM-3185 Remove PTI/PCI filter from the /titles/electronic endpoint in Agreements (#745) * remove pci and pti sub queries from TitleController Co-authored-by: EthanFreestone <54310740+EthanFreestone@users.noreply.github.com> --- .../org/olf/TitleController.groovy | 28 +------------------ 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/service/grails-app/controllers/org/olf/TitleController.groovy b/service/grails-app/controllers/org/olf/TitleController.groovy index f2f3fe81..3db65de1 100644 --- a/service/grails-app/controllers/org/olf/TitleController.groovy +++ b/service/grails-app/controllers/org/olf/TitleController.groovy @@ -22,38 +22,12 @@ class TitleController extends OkapiTenantAwareController { super(TitleInstance) } - DetachedCriteria pciSubQuery = PackageContentItem.where({ - eqProperty('pti.id', 'platformTitleInstance.id') - setAlias 'packageContentItem' - isNull 'removedTimestamp' - - projections { - property 'id' - } - }) - - DetachedCriteria ptiSubQuery = PlatformTitleInstance.where({ - eqProperty('titleInstance.id', "${DEFAULT_ROOT_ALIAS}.id") //here "this" refers to the root alias of criteria - setAlias 'platformTitleInstance' - - exists(pciSubQuery) - - projections { - property 'id' - } - }) - - // LEFT JOIN query def electronic () { respond doTheLookup { - and { eq 'subType', TitleInstance.lookupOrCreateSubType('electronic') - - exists(ptiSubQuery) - } } } - + def entitled() { respond doTheLookup (TitleInstance.entitled) } From 2aeccc3092abfd4d5715a1fcc105938540fd1d9b Mon Sep 17 00:00:00 2001 From: sosguthorpe Date: Mon, 15 Apr 2024 16:27:13 +0100 Subject: [PATCH 10/12] fix: Connection closed error when changing schema (#749) --- service/build.gradle | 8 +++++--- service/grails-app/conf/logback-development.xml | 6 ++++++ service/grails-app/conf/logback.xml | 5 +++++ .../org/olf/general/jobs/JobRunnerService.groovy | 16 ++++++++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/service/build.gradle b/service/build.gradle index 09346dc6..3db27e65 100644 --- a/service/build.gradle +++ b/service/build.gradle @@ -1,7 +1,9 @@ /* configurations.all { + // Check for updates every build resolutionStrategy.cacheChangingModulesFor 0, 'seconds' -} */ +} +*/ buildscript { repositories { @@ -60,7 +62,7 @@ bootJar { } repositories { - //mavenLocal() +// mavenLocal() maven { url "https://repo.grails.org/grails/core" } maven { url "https://repo.grails.org/grails/plugins" } maven { url "https://jitpack.io" } @@ -125,7 +127,7 @@ dependencies { /* ---- Manually installed dependencies ---- */ implementation 'com.k_int.grails:web-toolkit-ce:9.0.0' - implementation('com.k_int.okapi:grails-okapi:7.0.0') { + implementation('com.k_int.okapi:grails-okapi:7.0.1') { exclude group: 'com.vaadin.external.google', module: 'android-json' } diff --git a/service/grails-app/conf/logback-development.xml b/service/grails-app/conf/logback-development.xml index 128ee02f..1c410be0 100644 --- a/service/grails-app/conf/logback-development.xml +++ b/service/grails-app/conf/logback-development.xml @@ -35,6 +35,12 @@ + + + + + + diff --git a/service/grails-app/conf/logback.xml b/service/grails-app/conf/logback.xml index ca425989..289eb3e5 100644 --- a/service/grails-app/conf/logback.xml +++ b/service/grails-app/conf/logback.xml @@ -34,6 +34,10 @@ + + + + @@ -125,6 +129,7 @@ +