From 05c3609c22bd2acb60e5e019a7b8801a1596f11c Mon Sep 17 00:00:00 2001 From: Ethan Freestone Date: Fri, 30 Aug 2024 16:36:26 +0100 Subject: [PATCH 1/7] perf: PushKB improvements Initial crack at improving pushKB ingest times. Currently ~0.17 per title (~0.03 for normal ingest) --- .../olf/general/pushKB/PushKBService.groovy | 85 +++++++++++++++++-- 1 file changed, 80 insertions(+), 5 deletions(-) diff --git a/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy b/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy index 62644fca..c777fc6e 100644 --- a/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy +++ b/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy @@ -111,10 +111,11 @@ class PushKBService implements DataBinder { updatedAccessEnd: 0, ] KBIngressType ingressType = kbManagementBean.ingressType - + log.debug("pushPCIs (Service) called") if (ingressType == KBIngressType.PushKB) { try { pcis.each { Map record -> + log.debug("PCI record: ${record}") // Handle MDC directly? Might not be the right approach MDC.put('title', StringUtils.truncate(record.title.toString())) @@ -129,12 +130,81 @@ class PushKBService implements DataBinder { pc.instanceMedium = 'Electronic' } + log.debug("BeforeBind") bindData(pc, record) + log.debug("AfterBind") if (utilityService.checkValidBinding(pc)) { + log.debug("IsValid") try { Pkg pkg = null; - Pkg.withNewTransaction { status -> + Pkg.withSession { currentSess -> + Pkg.withTransaction { + Pkg.withNewSession { newSess -> + Pkg.withTransaction { + // TODO this will allow the PCI data to update the PKG record... do we want this? + log.debug("Before package look") + + pkg = packageIngestService.lookupOrCreatePackageFromTitle(pc); + log.debug("LOGGING PACKAGE OBTAINED FROM PCI: ${pkg}") + } + newSess.clear() + } + } + } + + TitleInstance.withSession { currentSess -> + TitleInstance.withTransaction { + TitleInstance.withNewSession { newSess -> + TitleInstance.withTransaction { + Map titleIngestResult = titleIngestService.upsertTitleDirect(pc) + log.debug("LOGGING titleIngestResult: ${titleIngestResult}") + + if ( titleIngestResult.titleInstanceId != null ) { + log.debug("Before lookupOrCreateTitleHierarchy") + + Map hierarchyResult = packageIngestService.lookupOrCreateTitleHierarchy( + titleIngestResult.titleInstanceId, + pkg.id, + true, + pc, + result.updateTime, + result.titleCount // FIXME not sure about this + ) + + PackageContentItem pci = PackageContentItem.get(hierarchyResult.pciId) + packageIngestService.hierarchyResultMapLogic(hierarchyResult, result, pci) + log.debug("After lookupOrCreateTitleHierarchy") + + /* TODO figure out if use of removedTimestamp + * should be something harvest also needs to do directly + * And whether we should be doing it after all the above + * or before. + */ + if (pc.removedTimestamp) { + try { + log.debug("Removal candidate: pci.id #${pci.id} (Last seen ${pci.lastSeenTimestamp}, thisUpdate ${result.updateTime}) -- Set removed") + pci.removedTimestamp = pc.removedTimestamp + pci.save(failOnError:true) + } catch ( Exception e ) { + log.error("Problem removing ${pci} in package load", e) + } + result.removedTitles++ + } + } else { + String message = "Skipping \"${pc.title}\". Unable to resolve title from ${pc.title} with identifiers ${pc.instanceIdentifiers}" + log.error(message) + } + } + newSess.clear() + } + } + } + + // FIXME trying with aggressive session managemnet and clearing as in ingest service -- might not work + /* Pkg.withNewTransaction { status -> // TODO this will allow the PCI data to update the PKG record... do we want this? + log.debug("Before package look") + pkg = packageIngestService.lookupOrCreatePackageFromTitle(pc); log.debug("LOGGING PACKAGE OBTAINED FROM PCI: ${pkg}") @@ -142,6 +212,8 @@ class PushKBService implements DataBinder { log.debug("LOGGING titleIngestResult: ${titleIngestResult}") if ( titleIngestResult.titleInstanceId != null ) { + log.debug("Before lookupOrCreateTitleHierarchy") + Map hierarchyResult = packageIngestService.lookupOrCreateTitleHierarchy( titleIngestResult.titleInstanceId, pkg.id, @@ -153,12 +225,13 @@ class PushKBService implements DataBinder { PackageContentItem pci = PackageContentItem.get(hierarchyResult.pciId) packageIngestService.hierarchyResultMapLogic(hierarchyResult, result, pci) + log.debug("After lookupOrCreateTitleHierarchy") /* TODO figure out if use of removedTimestamp * should be something harvest also needs to do directly * And whether we should be doing it after all the above * or before. - */ + * / if (pc.removedTimestamp) { try { log.debug("Removal candidate: pci.id #${pci.id} (Last seen ${pci.lastSeenTimestamp}, thisUpdate ${result.updateTime}) -- Set removed") @@ -173,7 +246,7 @@ class PushKBService implements DataBinder { String message = "Skipping \"${pc.title}\". Unable to resolve title from ${pc.title} with identifiers ${pc.instanceIdentifiers}" log.error(message) } - } + } */ } catch ( IngestException ie ) { // When we've caught an ingest exception, should have helpful error log message String message = "Skipping \"${pc.title}\": ${ie.message}" @@ -182,6 +255,7 @@ class PushKBService implements DataBinder { String message = "Skipping \"${pc.title}\". System error: ${e.message}" log.error(message,e) } + log.debug("Near end") // FIXME seems to be taking 0.1s from log.debug("After lookupOrCreateTitleHierarchy")??? result.titleCount++ @@ -196,7 +270,7 @@ class PushKBService implements DataBinder { } */ } - long finishedTime = (System.currentTimeMillis()-result.startTime)/1000 + long finishedTime = (System.currentTimeMillis()-result.startTime) // Don't divide by 1000 here result.success = true // TODO Logging may need tweaking between pushKB and harvest @@ -212,6 +286,7 @@ class PushKBService implements DataBinder { result.errorMessage = "pushPCIs not valid when kbManagementBean is configured with type (${ingressType})" } + log.debug("Before return") return result } } From 00f85b2af4d6b306fd76d1840abb0c0de399d230 Mon Sep 17 00:00:00 2001 From: Ethan Freestone Date: Wed, 4 Sep 2024 16:32:17 +0100 Subject: [PATCH 2/7] chore: Commented out logging for now to see if we get a speed boost by happy-accident --- .../org/olf/PackageIngestService.groovy | 2 +- .../olf/general/pushKB/PushKBService.groovy | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/service/grails-app/services/org/olf/PackageIngestService.groovy b/service/grails-app/services/org/olf/PackageIngestService.groovy index df2949c3..26ebb847 100644 --- a/service/grails-app/services/org/olf/PackageIngestService.groovy +++ b/service/grails-app/services/org/olf/PackageIngestService.groovy @@ -142,7 +142,7 @@ class PackageIngestService implements DataBinder { PackageContentItem.withNewTransaction { status -> // Delegate out to TitleIngestService so that any shared steps can move there. Map titleIngestResult = titleIngestService.upsertTitle(pc, kb, trustedSourceTI) - + //log.debug("LOGDEBUG RESOLVED TITLE: ${titleIngestResult}") // titleIngestResult.titleInstanceId will be non-null IFF TitleIngestService managed to find a title with that Id. if ( titleIngestResult.titleInstanceId != null ) { // Pass off to new hierarchy method (?) diff --git a/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy b/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy index c777fc6e..ad2030c6 100644 --- a/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy +++ b/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy @@ -111,11 +111,11 @@ class PushKBService implements DataBinder { updatedAccessEnd: 0, ] KBIngressType ingressType = kbManagementBean.ingressType - log.debug("pushPCIs (Service) called") + //log.debug("pushPCIs (Service) called") if (ingressType == KBIngressType.PushKB) { try { pcis.each { Map record -> - log.debug("PCI record: ${record}") + //log.debug("PCI record: ${record}") // Handle MDC directly? Might not be the right approach MDC.put('title', StringUtils.truncate(record.title.toString())) @@ -130,11 +130,11 @@ class PushKBService implements DataBinder { pc.instanceMedium = 'Electronic' } - log.debug("BeforeBind") + //log.debug("BeforeBind") bindData(pc, record) - log.debug("AfterBind") + //log.debug("AfterBind") if (utilityService.checkValidBinding(pc)) { - log.debug("IsValid") + //log.debug("IsValid") try { Pkg pkg = null; Pkg.withSession { currentSess -> @@ -142,10 +142,10 @@ class PushKBService implements DataBinder { Pkg.withNewSession { newSess -> Pkg.withTransaction { // TODO this will allow the PCI data to update the PKG record... do we want this? - log.debug("Before package look") + //log.debug("Before package look") pkg = packageIngestService.lookupOrCreatePackageFromTitle(pc); - log.debug("LOGGING PACKAGE OBTAINED FROM PCI: ${pkg}") + //log.debug("LOGGING PACKAGE OBTAINED FROM PCI: ${pkg}") } newSess.clear() } @@ -157,10 +157,10 @@ class PushKBService implements DataBinder { TitleInstance.withNewSession { newSess -> TitleInstance.withTransaction { Map titleIngestResult = titleIngestService.upsertTitleDirect(pc) - log.debug("LOGGING titleIngestResult: ${titleIngestResult}") + //log.debug("LOGGING titleIngestResult: ${titleIngestResult}") if ( titleIngestResult.titleInstanceId != null ) { - log.debug("Before lookupOrCreateTitleHierarchy") + //log.debug("Before lookupOrCreateTitleHierarchy") Map hierarchyResult = packageIngestService.lookupOrCreateTitleHierarchy( titleIngestResult.titleInstanceId, @@ -173,7 +173,7 @@ class PushKBService implements DataBinder { PackageContentItem pci = PackageContentItem.get(hierarchyResult.pciId) packageIngestService.hierarchyResultMapLogic(hierarchyResult, result, pci) - log.debug("After lookupOrCreateTitleHierarchy") + //log.debug("After lookupOrCreateTitleHierarchy") /* TODO figure out if use of removedTimestamp * should be something harvest also needs to do directly @@ -255,7 +255,7 @@ class PushKBService implements DataBinder { String message = "Skipping \"${pc.title}\". System error: ${e.message}" log.error(message,e) } - log.debug("Near end") // FIXME seems to be taking 0.1s from log.debug("After lookupOrCreateTitleHierarchy")??? + //log.debug("Near end") // FIXME seems to be taking 0.1s from log.debug("After lookupOrCreateTitleHierarchy")??? result.titleCount++ @@ -286,7 +286,7 @@ class PushKBService implements DataBinder { result.errorMessage = "pushPCIs not valid when kbManagementBean is configured with type (${ingressType})" } - log.debug("Before return") + //log.debug("Before return") return result } } From a873a8981b1989f631f08c8b3fede97d3ebe303a Mon Sep 17 00:00:00 2001 From: Ethan Freestone Date: Wed, 16 Oct 2024 10:44:28 +0100 Subject: [PATCH 3/7] chore: Removed commented code Remove old commented out code --- .../org/olf/TitleIngestService.groovy | 2 +- .../olf/general/pushKB/PushKBService.groovy | 47 ------------------- 2 files changed, 1 insertion(+), 48 deletions(-) diff --git a/service/grails-app/services/org/olf/TitleIngestService.groovy b/service/grails-app/services/org/olf/TitleIngestService.groovy index 9d48f256..3ed9e787 100644 --- a/service/grails-app/services/org/olf/TitleIngestService.groovy +++ b/service/grails-app/services/org/olf/TitleIngestService.groovy @@ -36,7 +36,7 @@ class TitleIngestService implements DataBinder { TitleInstance.withNewTransaction { if (!kb) { // This KB is created without a Type... so if it was trusted for TI data then it'd fail secondary enrichment - kb = new RemoteKB( name:remotekbname, + kb = new RemoteKB( name:remotekbname, rectype: RemoteKB.RECTYPE_TITLE, active: Boolean.TRUE, readonly:readOnly, diff --git a/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy b/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy index ad2030c6..68078593 100644 --- a/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy +++ b/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy @@ -200,53 +200,6 @@ class PushKBService implements DataBinder { } } - // FIXME trying with aggressive session managemnet and clearing as in ingest service -- might not work - /* Pkg.withNewTransaction { status -> - // TODO this will allow the PCI data to update the PKG record... do we want this? - log.debug("Before package look") - - pkg = packageIngestService.lookupOrCreatePackageFromTitle(pc); - log.debug("LOGGING PACKAGE OBTAINED FROM PCI: ${pkg}") - - Map titleIngestResult = titleIngestService.upsertTitleDirect(pc) - log.debug("LOGGING titleIngestResult: ${titleIngestResult}") - - if ( titleIngestResult.titleInstanceId != null ) { - log.debug("Before lookupOrCreateTitleHierarchy") - - Map hierarchyResult = packageIngestService.lookupOrCreateTitleHierarchy( - titleIngestResult.titleInstanceId, - pkg.id, - true, - pc, - result.updateTime, - result.titleCount // FIXME not sure about this - ) - - PackageContentItem pci = PackageContentItem.get(hierarchyResult.pciId) - packageIngestService.hierarchyResultMapLogic(hierarchyResult, result, pci) - log.debug("After lookupOrCreateTitleHierarchy") - - /* TODO figure out if use of removedTimestamp - * should be something harvest also needs to do directly - * And whether we should be doing it after all the above - * or before. - * / - if (pc.removedTimestamp) { - try { - log.debug("Removal candidate: pci.id #${pci.id} (Last seen ${pci.lastSeenTimestamp}, thisUpdate ${result.updateTime}) -- Set removed") - pci.removedTimestamp = pc.removedTimestamp - pci.save(failOnError:true) - } catch ( Exception e ) { - log.error("Problem removing ${pci} in package load", e) - } - result.removedTitles++ - } - } else { - String message = "Skipping \"${pc.title}\". Unable to resolve title from ${pc.title} with identifiers ${pc.instanceIdentifiers}" - log.error(message) - } - } */ } catch ( IngestException ie ) { // When we've caught an ingest exception, should have helpful error log message String message = "Skipping \"${pc.title}\": ${ie.message}" From b055016b18b5f25c576d251135413fa07199db4f Mon Sep 17 00:00:00 2001 From: Ethan Freestone Date: Wed, 16 Oct 2024 10:56:13 +0100 Subject: [PATCH 4/7] refactor: PackageIngestService lookupOrCreatePackage Split out lookup/update/create package methods so that we can have more granular decisions on when and where packages get created for pushKB process --- .../org/olf/PackageIngestService.groovy | 103 ++++++++++++------ 1 file changed, 69 insertions(+), 34 deletions(-) diff --git a/service/grails-app/services/org/olf/PackageIngestService.groovy b/service/grails-app/services/org/olf/PackageIngestService.groovy index 26ebb847..57a1fa67 100644 --- a/service/grails-app/services/org/olf/PackageIngestService.groovy +++ b/service/grails-app/services/org/olf/PackageIngestService.groovy @@ -4,6 +4,8 @@ import static org.springframework.transaction.annotation.Propagation.REQUIRES_NE import java.util.concurrent.TimeUnit +import org.olf.general.Org + import org.olf.general.StringUtils import org.olf.general.IngestException @@ -243,14 +245,13 @@ class PackageIngestService implements DataBinder { } } - /* - * Separate out the create or lookup pkg code, so that it can - * be used both by the ingest service (via upsert pkg), as well - * as the pushKBService (directly) - * - * This method ALSO provides update information for packages. + /* + * We split out the lookup vs the update vs the create code for packages, + * as there is potentially differences in behaviour between pushKB and harvest. */ - public Pkg lookupOrCreatePkg(PackageSchema package_data) { + + // This WILL NOT update or create a package from the data. + public Pkg lookupPkg(PackageSchema package_data) { Pkg pkg = null // header.packageSlug contains the package maintainers authoritative identifier for this package. @@ -266,8 +267,16 @@ class PackageIngestService implements DataBinder { } } - def vendor = null - if ( ( package_data.header?.packageProvider?.name != null ) && ( package_data.header?.packageProvider?.name.trim().length() > 0 ) ) { + return pkg; + } + + public Org getVendorFromPackageData(PackageSchema package_data) { + Org vendor = null; + + if ( + package_data.header?.packageProvider?.name != null && + package_data.header?.packageProvider?.name.trim().length() > 0 + ) { log.debug("Package contains provider information: ${package_data.header?.packageProvider?.name} -- trying to match to an existing organisation.") vendor = dependentModuleProxyService.coordinateOrg(package_data.header?.packageProvider?.name) // reference has been removed at the request of the UI team @@ -277,10 +286,58 @@ class PackageIngestService implements DataBinder { log.warn('Package ingest - no provider information present') } + return vendor; + } + + public Pkg lookupPkgAndUpdate(PackageSchema package_data) { + Pkg pkg = lookupPkg(package_data) + Org vendor = getVendorFromPackageData(package_data) + // FIXME do update step but NOT create step + if (pkg != null) { + pkg.sourceDataUpdated = package_data.header.sourceDataUpdated + + if (package_data.header.lifecycleStatus) { + pkg.lifecycleStatusFromString = package_data.header.lifecycleStatus + } + + if (package_data.header.availabilityScope) { + pkg.availabilityScopeFromString = package_data.header.availabilityScope + } + + pkg.vendor = vendor + pkg.description = package_data.header.description + pkg.name = package_data.header.packageName + pkg.save(failOnError:true) + + // Call separate methods for updating collections for code cleanliness + // These methods are responsible for their own saves + updateContentTypes(pkg.id, package_data) + updateAlternateNames(pkg.id, package_data) + updateAvailabilityConstraints(pkg.id, package_data) + updatePackageDescriptionUrls(pkg.id, package_data) + } + + return pkg; + } + + + /* + * Separate out the create or lookup pkg code, so that it can + * be used both by the ingest service (via upsert pkg), as well + * as the pushKBService (directly) + * + * This method ALSO updates information for packages. + */ + public Pkg lookupOrCreatePkg(PackageSchema package_data) { + // This takes care of any updates + Pkg pkg = lookupPkgAndUpdate(package_data); + + // If pkg is null, then we can safely create a new one if ( pkg == null ) { + Org vendor = getVendorFromPackageData(package_data) pkg = new Pkg( - name: package_data.header.packageName, - source: package_data.header.packageSource, + name: package_data.header.packageName, + source: package_data.header.packageSource, reference: package_data.header.packageSlug, description: package_data.header.description, sourceDataCreated: package_data.header.sourceDataCreated, @@ -288,7 +345,7 @@ class PackageIngestService implements DataBinder { 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, + vendor: vendor, ).save(flush:true, failOnError:true) (package_data?.header?.contentTypes ?: []).each { @@ -308,28 +365,6 @@ class PackageIngestService implements DataBinder { } pkg.save(failOnError: true) - } else { - pkg.sourceDataUpdated = package_data.header.sourceDataUpdated - - if (package_data.header.lifecycleStatus) { - pkg.lifecycleStatusFromString = package_data.header.lifecycleStatus - } - - if (package_data.header.availabilityScope) { - pkg.availabilityScopeFromString = package_data.header.availabilityScope - } - - pkg.vendor = vendor - pkg.description = package_data.header.description - pkg.name = package_data.header.packageName - pkg.save(failOnError:true) - - // Call separate methods for updating collections for code cleanliness - // These methods are responsible for their own saves - updateContentTypes(pkg.id, package_data) - updateAlternateNames(pkg.id, package_data) - updateAvailabilityConstraints(pkg.id, package_data) - updatePackageDescriptionUrls(pkg.id, package_data) } return pkg; From 1858ebef311da4f42b64af489f91434f38c79733 Mon Sep 17 00:00:00 2001 From: Ethan Freestone Date: Fri, 18 Oct 2024 09:48:14 +0100 Subject: [PATCH 5/7] chore: added test1 tenant sample data file --- .../tenant/sample_data/test1-data.groovy | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 service/src/main/okapi/tenant/sample_data/test1-data.groovy diff --git a/service/src/main/okapi/tenant/sample_data/test1-data.groovy b/service/src/main/okapi/tenant/sample_data/test1-data.groovy new file mode 100644 index 00000000..3b1a6c86 --- /dev/null +++ b/service/src/main/okapi/tenant/sample_data/test1-data.groovy @@ -0,0 +1,40 @@ +// DEV NOTE -- This is what will be used for the default rancher-desktop-db/dc environments +// For vagrant development (which I think is deprecated now) there may need to be a different file +// to avoid making changes to diku-data or _data. +log.info "Running test1-data tenant sample data file" + +import org.olf.kb.RemoteKB + +/* RemoteKB.findByName('GOKb_TEST') ?: (new RemoteKB( + name:'GOKb_TEST', + type:'org.olf.kb.adapters.GOKbOAIAdapter', + uri:'https://gokbt.gbv.de/gokb/oai/index', + fullPrefix:'gokb', + rectype: RemoteKB.RECTYPE_PACKAGE, + active:Boolean.TRUE, + supportsHarvesting:true, + activationEnabled:false, + //cursor: "2022-08-09T19:34:42Z" +).save(failOnError:true)) */ + +RemoteKB.findByName('GOKb') ?: (new RemoteKB( + name:'GOKb', + type:'org.olf.kb.adapters.GOKbOAIAdapter', + uri:'https://gokb.org/gokb/oai/index', + fullPrefix:'gokb', + rectype: RemoteKB.RECTYPE_PACKAGE, + active:Boolean.TRUE, + supportsHarvesting:true, + activationEnabled:false +).save(failOnError:true)) + +/* RemoteKB.findByName('DEBUG') ?: (new RemoteKB( + name:'DEBUG', + type:'org.olf.kb.adapters.DebugGoKbAdapter', + // uri can be used to directly force a package from the resources folder + // uri: 'src/integration-test/resources/DebugGoKbAdapter/borked_ids.xml' + rectype: RemoteKB.RECTYPE_PACKAGE, + active:Boolean.TRUE, + supportsHarvesting:true, + activationEnabled:false +).save(failOnError:true)) */ \ No newline at end of file From 680aab21679b7f8a76dc28a0a18fb0e0d751d12a Mon Sep 17 00:00:00 2001 From: Ethan Freestone Date: Tue, 29 Oct 2024 14:11:09 +0000 Subject: [PATCH 6/7] chore: Comment removal Removed commented out logdebugs --- .../services/org/olf/PackageIngestService.groovy | 8 +------- .../org/olf/general/jobs/JobRunnerService.groovy | 1 - .../org/olf/general/pushKB/PushKBService.groovy | 14 +------------- 3 files changed, 2 insertions(+), 21 deletions(-) diff --git a/service/grails-app/services/org/olf/PackageIngestService.groovy b/service/grails-app/services/org/olf/PackageIngestService.groovy index 57a1fa67..78e574d9 100644 --- a/service/grails-app/services/org/olf/PackageIngestService.groovy +++ b/service/grails-app/services/org/olf/PackageIngestService.groovy @@ -137,14 +137,11 @@ class PackageIngestService implements DataBinder { // ENSURE MDC title is set as early as possible MDC.put('title', StringUtils.truncate(pc.title.toString())) - // log.debug("Try to resolve ${pc}") - try { PackageContentItem.withNewSession { tsess -> PackageContentItem.withNewTransaction { status -> // Delegate out to TitleIngestService so that any shared steps can move there. Map titleIngestResult = titleIngestService.upsertTitle(pc, kb, trustedSourceTI) - //log.debug("LOGDEBUG RESOLVED TITLE: ${titleIngestResult}") // titleIngestResult.titleInstanceId will be non-null IFF TitleIngestService managed to find a title with that Id. if ( titleIngestResult.titleInstanceId != null ) { // Pass off to new hierarchy method (?) @@ -292,7 +289,7 @@ class PackageIngestService implements DataBinder { public Pkg lookupPkgAndUpdate(PackageSchema package_data) { Pkg pkg = lookupPkg(package_data) Org vendor = getVendorFromPackageData(package_data) - // FIXME do update step but NOT create step + // Do update step but NOT create step if (pkg != null) { pkg.sourceDataUpdated = package_data.header.sourceDataUpdated @@ -510,7 +507,6 @@ class PackageIngestService implements DataBinder { } Platform platform = Platform.resolve(platform_url_to_use, pc.platformName) - // log.debug("Platform: ${platform}") if ( platform == null && PROXY_MISSING_PLATFORM ) { platform = Platform.resolve('http://localhost.localdomain', 'This platform entry is used for error cases') @@ -563,8 +559,6 @@ class PackageIngestService implements DataBinder { pciStatus: 'none' // This should be 'none', 'updated' or 'new' ] - // log.debug("platform ${pc.platformUrl} ${pc.platformName} (item URL is ${pc.url})") - // lets try and work out the platform for the item Platform platform = lookupOrCreatePlatform(pc); diff --git a/service/grails-app/services/org/olf/general/jobs/JobRunnerService.groovy b/service/grails-app/services/org/olf/general/jobs/JobRunnerService.groovy index 1e942784..8e7b0bcd 100644 --- a/service/grails-app/services/org/olf/general/jobs/JobRunnerService.groovy +++ b/service/grails-app/services/org/olf/general/jobs/JobRunnerService.groovy @@ -537,7 +537,6 @@ order by pj.dateCreated org.slf4j.MDC.clear() org.slf4j.MDC.setContextMap( jobId: '' + jid, tenantId: '' + tid, 'tenant': '' + tenantName) JobContext.current.set(new JobContext( jobId: jid, tenantId: tid )) - //log.debug("LOGDEBUG TID: ${tId}") Tenants.withId(tid) { beginJob(jid) } diff --git a/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy b/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy index 68078593..15070769 100644 --- a/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy +++ b/service/grails-app/services/org/olf/general/pushKB/PushKBService.groovy @@ -111,11 +111,9 @@ class PushKBService implements DataBinder { updatedAccessEnd: 0, ] KBIngressType ingressType = kbManagementBean.ingressType - //log.debug("pushPCIs (Service) called") if (ingressType == KBIngressType.PushKB) { try { pcis.each { Map record -> - //log.debug("PCI record: ${record}") // Handle MDC directly? Might not be the right approach MDC.put('title', StringUtils.truncate(record.title.toString())) @@ -130,11 +128,8 @@ class PushKBService implements DataBinder { pc.instanceMedium = 'Electronic' } - //log.debug("BeforeBind") bindData(pc, record) - //log.debug("AfterBind") if (utilityService.checkValidBinding(pc)) { - //log.debug("IsValid") try { Pkg pkg = null; Pkg.withSession { currentSess -> @@ -142,10 +137,8 @@ class PushKBService implements DataBinder { Pkg.withNewSession { newSess -> Pkg.withTransaction { // TODO this will allow the PCI data to update the PKG record... do we want this? - //log.debug("Before package look") pkg = packageIngestService.lookupOrCreatePackageFromTitle(pc); - //log.debug("LOGGING PACKAGE OBTAINED FROM PCI: ${pkg}") } newSess.clear() } @@ -157,10 +150,8 @@ class PushKBService implements DataBinder { TitleInstance.withNewSession { newSess -> TitleInstance.withTransaction { Map titleIngestResult = titleIngestService.upsertTitleDirect(pc) - //log.debug("LOGGING titleIngestResult: ${titleIngestResult}") if ( titleIngestResult.titleInstanceId != null ) { - //log.debug("Before lookupOrCreateTitleHierarchy") Map hierarchyResult = packageIngestService.lookupOrCreateTitleHierarchy( titleIngestResult.titleInstanceId, @@ -168,12 +159,11 @@ class PushKBService implements DataBinder { true, pc, result.updateTime, - result.titleCount // FIXME not sure about this + result.titleCount // Not totally sure this is valuable here ) PackageContentItem pci = PackageContentItem.get(hierarchyResult.pciId) packageIngestService.hierarchyResultMapLogic(hierarchyResult, result, pci) - //log.debug("After lookupOrCreateTitleHierarchy") /* TODO figure out if use of removedTimestamp * should be something harvest also needs to do directly @@ -208,7 +198,6 @@ class PushKBService implements DataBinder { String message = "Skipping \"${pc.title}\". System error: ${e.message}" log.error(message,e) } - //log.debug("Near end") // FIXME seems to be taking 0.1s from log.debug("After lookupOrCreateTitleHierarchy")??? result.titleCount++ @@ -239,7 +228,6 @@ class PushKBService implements DataBinder { result.errorMessage = "pushPCIs not valid when kbManagementBean is configured with type (${ingressType})" } - //log.debug("Before return") return result } } From 49bed89dd3150464594260aecea4fd8d9ba016d3 Mon Sep 17 00:00:00 2001 From: Ethan Freestone Date: Tue, 29 Oct 2024 14:16:28 +0000 Subject: [PATCH 7/7] chore: Newline --- service/src/main/okapi/tenant/sample_data/test1-data.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/src/main/okapi/tenant/sample_data/test1-data.groovy b/service/src/main/okapi/tenant/sample_data/test1-data.groovy index 3b1a6c86..b5c03fb1 100644 --- a/service/src/main/okapi/tenant/sample_data/test1-data.groovy +++ b/service/src/main/okapi/tenant/sample_data/test1-data.groovy @@ -37,4 +37,4 @@ RemoteKB.findByName('GOKb') ?: (new RemoteKB( active:Boolean.TRUE, supportsHarvesting:true, activationEnabled:false -).save(failOnError:true)) */ \ No newline at end of file +).save(failOnError:true)) */