From b055016b18b5f25c576d251135413fa07199db4f Mon Sep 17 00:00:00 2001 From: Ethan Freestone Date: Wed, 16 Oct 2024 10:56:13 +0100 Subject: [PATCH] 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;