From 4da0773df75ff1ad0c684ae650baddbfbabada7a Mon Sep 17 00:00:00 2001 From: "taha.attari@smilecdr.com" Date: Wed, 12 Apr 2023 16:20:44 -0400 Subject: [PATCH 1/4] [APHL-535][APHL-481] Made $draft transactional and updated to allow multiple drafts --- .../ruler/cql/KnowledgeArtifactAdapter.java | 32 +++ .../ruler/cql/KnowledgeArtifactProcessor.java | 207 +++++++++++++----- .../cqf/ruler/cql/RepositoryService.java | 14 +- .../cqf/ruler/cql/RepositoryServiceTest.java | 102 ++++++++- ...inimal-draft-to-test-version-conflict.json | 6 + 5 files changed, 293 insertions(+), 68 deletions(-) create mode 100644 plugin/cql/src/test/resources/minimal-draft-to-test-version-conflict.json diff --git a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactAdapter.java b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactAdapter.java index f4c01c53c..bcd39997a 100644 --- a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactAdapter.java +++ b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactAdapter.java @@ -1,18 +1,21 @@ package org.opencds.cqf.ruler.cql; import java.util.ArrayList; +import java.util.Comparator; import java.util.Date; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; import org.hl7.fhir.r4.model.ActivityDefinition; +import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.ContactDetail; import org.hl7.fhir.r4.model.Library; import org.hl7.fhir.r4.model.Measure; import org.hl7.fhir.r4.model.MetadataResource; import org.hl7.fhir.r4.model.PlanDefinition; import org.hl7.fhir.r4.model.RelatedArtifact; +import org.hl7.fhir.r4.model.Resource; import org.hl7.fhir.r4.model.ValueSet; public class KnowledgeArtifactAdapter { @@ -262,4 +265,33 @@ public MetadataResource copy() { //TODO: Is calling MetadataResource.copy() the right default behavior? } } + + public static Optional findLatestVersion(List resources) { + Comparator versionComparator = SemanticVersion.getVersionComparator(); + MetadataResource latestResource = null; + + for (MetadataResource resource : resources) { + String version = resource.getVersion(); + if (latestResource == null || versionComparator.compare(version, latestResource.getVersion()) > 0) { + latestResource = resource; + } + } + + return Optional.ofNullable(latestResource); + } + + public static Optional findLatestVersion(Bundle bundle) { + List entries = bundle.getEntry(); + List metadataResources = new ArrayList<>(); + + for (Bundle.BundleEntryComponent entry : entries) { + Resource resource = entry.getResource(); + if (resource instanceof MetadataResource) { + MetadataResource metadataResource = (MetadataResource) resource; + metadataResources.add(metadataResource); + } + } + + return findLatestVersion(metadataResources); + } } diff --git a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java index 4ddb1b4c8..7702b6768 100644 --- a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java +++ b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java @@ -8,29 +8,43 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.UUID; import java.util.stream.Collectors; import org.apache.commons.lang3.NotImplementedException; import org.cqframework.fhir.api.FhirDal; import org.hl7.fhir.exceptions.FHIRException; +import org.hl7.fhir.instance.model.api.IBaseBundle; import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.CanonicalType; import org.hl7.fhir.r4.model.CodeType; import org.hl7.fhir.r4.model.ContactDetail; +import org.hl7.fhir.r4.model.DataRequirement; import org.hl7.fhir.r4.model.DateTimeType; +import org.hl7.fhir.r4.model.ElementDefinition; import org.hl7.fhir.r4.model.Enumerations; +import org.hl7.fhir.r4.model.Expression; +import org.hl7.fhir.r4.model.Extension; import org.hl7.fhir.r4.model.IdType; +import org.hl7.fhir.r4.model.Library; import org.hl7.fhir.r4.model.MarkdownType; import org.hl7.fhir.r4.model.MetadataResource; +import org.hl7.fhir.r4.model.PlanDefinition; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.RelatedArtifact; +import org.hl7.fhir.r4.model.StructureDefinition; +import org.hl7.fhir.r4.model.Type; +import org.hl7.fhir.r4.model.UsageContext; +import org.hl7.fhir.r4.model.ValueSet; import org.jetbrains.annotations.Nullable; import org.opencds.cqf.cql.engine.exception.InvalidOperatorArgument; import org.opencds.cqf.cql.evaluator.fhir.util.Canonicals; +import org.opencds.cqf.ruler.builder.BundleBuilder; import org.opencds.cqf.ruler.cql.r4.ArtifactAssessment; import org.opencds.cqf.ruler.cql.r4.ArtifactAssessment.ArtifactAssessmentContentInformationType; import org.springframework.beans.factory.annotation.Configurable; +import org.springframework.web.client.ResourceAccessException; import ca.uhn.fhir.model.api.IQueryParameterType; import ca.uhn.fhir.rest.param.TokenParam; @@ -42,10 +56,9 @@ // TODO: This belongs in the Evaluator. Only included in Ruler at dev time for // shorter cycle. public class KnowledgeArtifactProcessor { - - private List finalRelatedArtifactList = new ArrayList<>(); - private List finalRelatedArtifactListUpdated = new ArrayList<>(); - private List bundleEntryComponentList = new ArrayList<>(); + public static final String CPG_INFERENCEEXPRESSION = "http://hl7.org/fhir/uv/cpg/StructureDefinition/cpg-inferenceExpression"; + public static final String CPG_ASSERTIONEXPRESSION = "http://hl7.org/fhir/uv/cpg/StructureDefinition/cpg-assertionExpression"; + public static final String CPG_FEATUREEXPRESSION = "http://hl7.org/fhir/uv/cpg/StructureDefinition/cpg-featureExpression"; private Bundle searchResourceByUrl(String url, FhirDal fhirDal) { Map>> searchParams = new HashMap<>(); @@ -87,6 +100,36 @@ private Bundle searchResourceByUrlAndStatus(String url, String status, FhirDal f return searchResultsBundle; } + private List getResourcesFromBundle(Bundle bundle) { + List resourceList = new ArrayList<>(); + + if (!bundle.getEntryFirstRep().isEmpty()) { + List referencedResourceEntries = bundle.getEntry(); + for (Bundle.BundleEntryComponent entry: referencedResourceEntries) { + if (entry.hasResource() && entry.getResource() instanceof MetadataResource) { + MetadataResource referencedResource = (MetadataResource) entry.getResource(); + resourceList.add(referencedResource); + } + } + } + + return resourceList; + } + + private MetadataResource retrieveResourcesByReference(String reference, FhirDal fhirDal) { + MetadataResource resource = null; + + Bundle referencedResourceBundle = searchResourceByUrl(reference, fhirDal); + MetadataResource referencedResource = KnowledgeArtifactAdapter.findLatestVersion(referencedResourceBundle); + if (referencedResource != null) { + resource = referencedResource; + } else { + throw new ResourceNotFoundException(String.format("Resource for Canonical '%s' not found.", reference)); + } + + return resource; + } + /* approve */ /* * The operation sets the date and approvalDate elements of the approved @@ -138,15 +181,39 @@ ArtifactAssessment createApprovalAssessment(IdType id, String artifactCommentTyp } /* $draft */ - public MetadataResource draft(IdType idType, FhirDal fhirDal, String version) { - //TODO: Needs to be transactional - MetadataResource resource = (MetadataResource) fhirDal.read(idType); - if (resource == null) { - throw new ResourceNotFoundException(idType); - } + /* + * The operation creates a draft of the Base Artifact and + * related resources. + * + * This method generates the transaction bundle for this operation. + * + * This bundle consists of: + * 1. A new version of the base artifact where status is changed to + * draft and version changed to a new version number + "-draft" + * + * 2. New versions of related artifacts where status is changed to + * draft and version changed to a new version number + "-draft" + * + * Links and references between Bundle resources are updated to point to + * the new versions. + */ + public Bundle createDraftBundle(IdType baseArtifactId, FhirDal fhirDal, String version) throws ResourceNotFoundException, UnprocessableEntityException { + MetadataResource baseArtifact = (MetadataResource) fhirDal.read(baseArtifactId); - if (version.contains(".")) { + if (baseArtifact == null) { + throw new ResourceNotFoundException(baseArtifactId); + } + // Check if version is valid + if(version.contains("/") || version.contains("\\") || version.contains("|")){ + throw new UnprocessableEntityException("The version contains illegal characters"); + } + if (!version.contains(".")) { + throw new UnprocessableEntityException("The version must be in the format MAJOR.MINOR.PATCH"); + } else { String[] versionParts = version.split("\\."); + if(versionParts.length != 3){ + throw new UnprocessableEntityException("The version must be in the format MAJOR.MINOR.PATCH"); + } for(int i = 0; i < versionParts.length; i++) { String section = ""; if(Integer.parseInt(versionParts[i]) < 0) { @@ -157,81 +224,111 @@ public MetadataResource draft(IdType idType, FhirDal fhirDal, String version) { } else if (i == 2) { section = "Patch"; } - throw new IllegalArgumentException("The " + section + " version part should be greater than 0."); + throw new UnprocessableEntityException("The " + section + " version part should be greater than 0."); } } - - } - - // Root artifact must have status of 'Active'. Existing drafts of reference artifacts will be adopted. This check is - // performed here to facilitate that different treatment for the root artifact and those referenced by it. - if (resource.getStatus() != Enumerations.PublicationStatus.ACTIVE) { - throw new IllegalStateException( - String.format("Drafts can only be created from artifacts with status of 'active'. Resource '%s' has a status of: %s", resource.getUrl(), resource.getStatus().toString())); } - Bundle existingArtifactsForUrl = searchResourceByUrl(resource.getUrl(), fhirDal); - Optional existingDrafts = existingArtifactsForUrl.getEntry().stream().filter( - e -> ((MetadataResource) e.getResource()).getStatus() == Enumerations.PublicationStatus.DRAFT).findFirst(); + String draftVersion = version + "-draft"; + String draftVersionUrl = Canonicals.getUrl(baseArtifact.getUrl()) + "|" + draftVersion; - if (existingDrafts.isPresent()) { - throw new IllegalStateException( - String.format("A draft of Program '%s' already exists with ID: '%s'. Only one draft of a program can exist at a time.", resource.getUrl(), ((MetadataResource) existingDrafts.get().getResource()).getId())); + // Root artifact must have status of 'Active'. Existing drafts of + // reference artifacts with the right verison number will be adopted. + // This check is performed here to facilitate that different treatment + // for the root artifact and those referenced by it. + if (baseArtifact.getStatus() != Enumerations.PublicationStatus.ACTIVE) { + throw new UnprocessableEntityException( + String.format("Drafts can only be created from artifacts with status of 'active'. Resource '%s' has a status of: %s", baseArtifact.getUrl(), baseArtifact.getStatus().toString())); } - - return internalDraft(resource, fhirDal, version); + // Ensure only one resource exists with this URL + Bundle existingArtifactsForUrl = searchResourceByUrl(draftVersionUrl, fhirDal); + if(existingArtifactsForUrl.getEntry().size() != 0){ + throw new UnprocessableEntityException( + String.format("A draft of Program '%s' already exists with version: '%s'. Only one draft of a program version can exist at a time.", baseArtifact.getUrl(), draftVersionUrl)); + } + List resourcesToCreate = createDraftsOfArtifactAndRelated(baseArtifact, fhirDal, version, new ArrayList()); + Bundle transactionBundle = new Bundle() + .setType(Bundle.BundleType.TRANSACTION); + List urnList = resourcesToCreate.stream().map(res -> new IdType("urn:uuid:" + UUID.randomUUID().toString())).collect(Collectors.toList()); + for(int i = 0; i < resourcesToCreate.size(); i++){ + KnowledgeArtifactAdapter newResourceAdapter = new KnowledgeArtifactAdapter(resourcesToCreate.get(i)); + updateUsageContextReferencesWithUrns(resourcesToCreate.get(i), resourcesToCreate, urnList); + updateRelatedArtifactUrlsWithNewVersions(newResourceAdapter, draftVersion); + MetadataResource updateIdForBundle = newResourceAdapter.copy(); + updateIdForBundle.setId(urnList.get(i)); + transactionBundle.addEntry(createEntry(updateIdForBundle)); + } + return transactionBundle; } + private void updateUsageContextReferencesWithUrns(MetadataResource newResource, List resourceListWithOriginalIds, List idListForTransactionBundle){ + List useContexts = newResource.getUseContext(); + for(UsageContext useContext : useContexts){ + if(useContext.hasValueReference()){ + Reference useContextRef = useContext.getValueReference(); + if(useContextRef != null){ + resourceListWithOriginalIds.stream() + .filter(resource -> (resource.getClass().getSimpleName() + "/" + resource.getIdElement().getIdPart()).equals(useContextRef.getReference())) + .findAny() + .ifPresent(resource -> { + int indexOfDraftInIdList = resourceListWithOriginalIds.indexOf(resource); + useContext.setValue(new Reference(idListForTransactionBundle.get(indexOfDraftInIdList))); + }); + } + } + } + } + private void updateRelatedArtifactUrlsWithNewVersions(KnowledgeArtifactAdapter newResourceAdapter, String draftVersion){ + // For each Resource relatedArtifact, update the version of the reference. + newResourceAdapter.getRelatedArtifact().stream() + .filter(ra -> ra.hasResource()).collect(Collectors.toList()) + .replaceAll(ra -> ra.setResource(Canonicals.getUrl(ra.getResource()) + "|" + draftVersion)); + } + private List createDraftsOfArtifactAndRelated(MetadataResource resourceToDraft, FhirDal fhirDal, String version, List resourcesToCreate) { + String draftVersion = version + "-draft"; + String draftVersionUrl = Canonicals.getUrl(resourceToDraft.getUrl()) + "|" + draftVersion; - private MetadataResource internalDraft(MetadataResource resource, FhirDal fhirDal, String version) { - Bundle existingArtifactsForUrl = searchResourceByUrl(resource.getUrl(), fhirDal); - Optional existingDrafts = existingArtifactsForUrl.getEntry().stream().filter( - e -> ((MetadataResource) e.getResource()).getStatus() == Enumerations.PublicationStatus.DRAFT).findFirst(); - + // TODO: Decide if we need both of these checks + Optional existingArtifactsWithMatchingUrl = KnowledgeArtifactAdapter.findLatestVersion(searchResourceByUrl(draftVersionUrl, fhirDal)); + Optional draftVersionAlreadyInBundle = resourcesToCreate.stream().filter(res -> res.getUrl().equals(Canonicals.getUrl(draftVersionUrl)) && res.getVersion().equals(draftVersion)).findAny(); MetadataResource newResource = null; - if (existingDrafts.isPresent()) { - newResource = (MetadataResource) existingDrafts.get().getResource(); + if (existingArtifactsWithMatchingUrl.isPresent()) { + newResource = existingArtifactsWithMatchingUrl.get(); + } else if(draftVersionAlreadyInBundle.isPresent()){ + newResource = draftVersionAlreadyInBundle.get(); } if (newResource == null) { - KnowledgeArtifactAdapter sourceResourceAdapter = new KnowledgeArtifactAdapter<>(resource); + KnowledgeArtifactAdapter sourceResourceAdapter = new KnowledgeArtifactAdapter<>(resourceToDraft); newResource = sourceResourceAdapter.copy(); newResource.setStatus(Enumerations.PublicationStatus.DRAFT); - newResource.setId((String)null); - newResource.setVersion(null); - newResource.setVersion(version + "-draft"); - - KnowledgeArtifactAdapter newResourceAdapter = new KnowledgeArtifactAdapter<>(newResource); - - // For each Resource relatedArtifact, strip the version of the reference. - newResourceAdapter.getRelatedArtifact().stream().filter(ra -> ra.hasResource()).collect(Collectors.toList()) - .replaceAll(ra -> ra.setResource(Canonicals.getUrl(ra.getResource()))); - - fhirDal.create(newResource); - + newResource.setVersion(draftVersion); + resourcesToCreate.add(newResource); for (RelatedArtifact ra : sourceResourceAdapter.getRelatedArtifact()) { - // If it is a composed-of relation then do a deep copy, else shallow + // If it’s a compose-of then we want to copy it + // If it’s a depends-on, we just want to reference it, but not copy it + // (references are updated in createDraftBundle before adding to the bundle) if (ra.getType() == RelatedArtifact.RelatedArtifactType.COMPOSEDOF) { if (ra.hasUrl()) { Bundle referencedResourceBundle = searchResourceByUrl(ra.getUrl(), fhirDal); - processReferencedResourceForDraft(fhirDal, referencedResourceBundle, ra, version); + processReferencedResourceForDraft(fhirDal, referencedResourceBundle, ra, version, resourcesToCreate); } else if (ra.hasResource()) { Bundle referencedResourceBundle = searchResourceByUrl(ra.getResourceElement().getValueAsString(), fhirDal); - processReferencedResourceForDraft(fhirDal, referencedResourceBundle, ra, version); + processReferencedResourceForDraft(fhirDal, referencedResourceBundle, ra, version, resourcesToCreate); } } } } - return newResource; + return resourcesToCreate; } - - private void processReferencedResourceForDraft(FhirDal fhirDal, Bundle referencedResourceBundle, RelatedArtifact ra, String version) { + + private void processReferencedResourceForDraft(FhirDal fhirDal, Bundle referencedResourceBundle, RelatedArtifact ra, String version, List transactionBundle) { if (!referencedResourceBundle.getEntryFirstRep().isEmpty()) { Bundle.BundleEntryComponent referencedResourceEntry = referencedResourceBundle.getEntry().get(0); if (referencedResourceEntry.hasResource() && referencedResourceEntry.getResource() instanceof MetadataResource) { MetadataResource referencedResource = (MetadataResource) referencedResourceEntry.getResource(); - internalDraft(referencedResource, fhirDal, version); + createDraftsOfArtifactAndRelated(referencedResource, fhirDal, version, transactionBundle); } } } diff --git a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/RepositoryService.java b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/RepositoryService.java index db8b446d7..b06a71a79 100644 --- a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/RepositoryService.java +++ b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/RepositoryService.java @@ -102,13 +102,21 @@ public Bundle approveOperation( } return transaction(transactionBundle, requestDetails); } - + /** + * Creates a draft of an existing artifact if it has status Active. + * + * @param requestDetails the {@link RequestDetails RequestDetails} + * @param theId the {@link IdType IdType}, always an argument for instance level operations + * @param version new version in the form MAJOR.MINOR.PATCH + * TODO: should return OperationOutcome + * @return A transaction bundle result of the newly created resources + */ @Operation(name = "$draft", idempotent = true, global = true, type = MetadataResource.class) @Description(shortDefinition = "$draft", value = "Create a new draft version of the reference artifact") - public Library draftOperation(RequestDetails requestDetails, @IdParam IdType theId, @OperationParam(name = "version") String version) + public Bundle draftOperation(RequestDetails requestDetails, @IdParam IdType theId, @OperationParam(name = "version") String version) throws FHIRException { FhirDal fhirDal = this.fhirDalFactory.create(requestDetails); - return (Library) this.artifactProcessor.draft(theId, fhirDal, version); + return transaction(this.artifactProcessor.createDraftBundle(theId, fhirDal, version)); } @Operation(name = "$release", idempotent = true, global = true, type = MetadataResource.class) diff --git a/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java b/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java index dc979e661..d7a621d4f 100644 --- a/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java +++ b/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java @@ -34,6 +34,7 @@ import org.springframework.boot.test.context.SpringBootTest; import ca.uhn.fhir.model.api.TemporalPrecisionEnum; +import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, @@ -43,26 +44,107 @@ class RepositoryServiceTest extends RestIntegrationTest { private final String specificationLibReference = "Library/SpecificationLibrary"; @Test - void draftOperation_active_test() { + void draftOperation_test() { loadTransaction("ersd-active-transaction-bundle-example.json"); - - Parameters params = parameters(part("version", "1.1.1") ); + String version = "1.1.1"; + String draftedVersion = version + "-draft"; + Parameters params = parameters(part("version", version) ); Resource returnResource = getClient().operation() .onInstance(specificationLibReference) .named("$draft") .withParameters(params) - .returnResourceType(Library.class) + .returnResourceType(Bundle.class) .execute(); - Library returnedLibrary = (Library) returnResource; - assertNotNull(returnedLibrary); - assertTrue(((Library) returnedLibrary).getStatus() == Enumerations.PublicationStatus.DRAFT); - List relatedArtifacts = returnedLibrary.getRelatedArtifact(); + Bundle returnedBundle = (Bundle) returnResource; + assertNotNull(returnedBundle); + Optional maybeLib = returnedBundle.getEntry().stream().filter(entry -> entry.getResponse().getLocation().contains("Library")).findAny(); + assertTrue(maybeLib.isPresent()); + Library lib = getClient().fetchResourceFromUrl(Library.class,maybeLib.get().getResponse().getLocation()); + assertNotNull(lib); + assertTrue(lib.getStatus() == Enumerations.PublicationStatus.DRAFT); + assertTrue(lib.getVersion().equals(draftedVersion)); + List relatedArtifacts = lib.getRelatedArtifact(); assertTrue(!relatedArtifacts.isEmpty()); - assertTrue(Canonicals.getVersion(relatedArtifacts.get(0).getResource()) == null); - assertTrue(Canonicals.getVersion(relatedArtifacts.get(1).getResource()) == null); + assertTrue(Canonicals.getVersion(relatedArtifacts.get(0).getResource()).equals(draftedVersion)); + assertTrue(Canonicals.getVersion(relatedArtifacts.get(1).getResource()).equals(draftedVersion)); } + @Test + void draftOperation_draft_test() { + loadTransaction("ersd-draft-transaction-bundle-example.json"); + Parameters params = parameters(part("version", "1.1.1") ); + try { + getClient().operation() + .onInstance("Library/DraftSpecificationLibrary") + .named("$draft") + .withParameters(params) + .returnResourceType(Bundle.class) + .execute(); + } catch (UnprocessableEntityException e) { + assertNotNull(e); + assertTrue(e.getMessage().contains("status of 'active'")); + } + } + @Test + void draftOperation_wrong_id_test() { + loadTransaction("ersd-draft-transaction-bundle-example.json"); + Parameters params = parameters(part("version", "1.1.1") ); + try { + getClient().operation() + .onInstance("Library/there-is-no-such-id") + .named("$draft") + .withParameters(params) + .returnResourceType(Bundle.class) + .execute(); + } catch (ResourceNotFoundException e) { + assertNotNull(e); + } + } + @Test + void draftOperation_version_conflict_test() { + loadTransaction("ersd-active-transaction-bundle-example.json"); + loadResource("minimal-draft-to-test-version-conflict.json"); + Parameters params = parameters(part("version", "1.1.1") ); + try { + getClient().operation() + .onInstance(specificationLibReference) + .named("$draft") + .withParameters(params) + .returnResourceType(Bundle.class) + .execute(); + } catch (UnprocessableEntityException e) { + assertNotNull(e); + assertTrue(e.getMessage().contains("already exists")); + } + } + @Test + void draftOperation_version_format_test() { + loadTransaction("ersd-active-transaction-bundle-example.json"); + loadResource("minimal-draft-to-test-version-conflict.json"); + List testValues = Arrays.asList(new String[]{ + "11asd1", + "1.1.3.1", + "1.|1.1", + "1/.1.1", + "-1.-1.2", + "1.-1.2", + "1.1.-2" + }); + for(String version:testValues){ + Parameters params = parameters(part("version", version) ); + try { + getClient().operation() + .onInstance(specificationLibReference) + .named("$draft") + .withParameters(params) + .returnResourceType(Bundle.class) + .execute(); + } catch (UnprocessableEntityException e) { + assertNotNull(e); + } + } + } //@Test //void releaseResource_test() { // loadTransaction("ersd-draft-transaction-bundle-example.json"); diff --git a/plugin/cql/src/test/resources/minimal-draft-to-test-version-conflict.json b/plugin/cql/src/test/resources/minimal-draft-to-test-version-conflict.json new file mode 100644 index 000000000..d3219c1dc --- /dev/null +++ b/plugin/cql/src/test/resources/minimal-draft-to-test-version-conflict.json @@ -0,0 +1,6 @@ +{ + "resourceType": "Library", + "id": "SpecificationLibraryDraftVersion-1-1-1", + "url": "http://hl7.org/fhir/us/ecr/Library/SpecificationLibrary", + "version": "1.1.1" +} From 2129d3089b21e480f2287df6fb755e605d6f139f Mon Sep 17 00:00:00 2001 From: Adam Stevenson Date: Thu, 13 Apr 2023 16:31:16 -0600 Subject: [PATCH 2/4] Add null check on version argument of $draft; updated $draft test cases to avoid ValueSet collisions --- .../opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java | 4 ++++ .../org/opencds/cqf/ruler/cql/RepositoryServiceTest.java | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java index 7702b6768..519d80b1d 100644 --- a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java +++ b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java @@ -198,6 +198,10 @@ ArtifactAssessment createApprovalAssessment(IdType id, String artifactCommentTyp * the new versions. */ public Bundle createDraftBundle(IdType baseArtifactId, FhirDal fhirDal, String version) throws ResourceNotFoundException, UnprocessableEntityException { + if (version == null || version.isEmpty()) { + throw new InvalidOperatorArgument("The version argument is required"); + } + MetadataResource baseArtifact = (MetadataResource) fhirDal.read(baseArtifactId); if (baseArtifact == null) { diff --git a/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java b/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java index d7a621d4f..796874111 100644 --- a/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java +++ b/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java @@ -73,7 +73,7 @@ void draftOperation_test() { @Test void draftOperation_draft_test() { loadTransaction("ersd-draft-transaction-bundle-example.json"); - Parameters params = parameters(part("version", "1.1.1") ); + Parameters params = parameters(part("version", "1.2.1") ); try { getClient().operation() .onInstance("Library/DraftSpecificationLibrary") @@ -89,7 +89,7 @@ void draftOperation_draft_test() { @Test void draftOperation_wrong_id_test() { loadTransaction("ersd-draft-transaction-bundle-example.json"); - Parameters params = parameters(part("version", "1.1.1") ); + Parameters params = parameters(part("version", "1.3.1") ); try { getClient().operation() .onInstance("Library/there-is-no-such-id") @@ -105,7 +105,7 @@ void draftOperation_wrong_id_test() { void draftOperation_version_conflict_test() { loadTransaction("ersd-active-transaction-bundle-example.json"); loadResource("minimal-draft-to-test-version-conflict.json"); - Parameters params = parameters(part("version", "1.1.1") ); + Parameters params = parameters(part("version", "1.4.1") ); try { getClient().operation() .onInstance(specificationLibReference) From e52122661f26a6b9951438c5392b95bf959e4858 Mon Sep 17 00:00:00 2001 From: "taha.attari@smilecdr.com" Date: Thu, 13 Apr 2023 19:48:29 -0400 Subject: [PATCH 3/4] [APHL-535][APHL-481] cleanup and updated tests --- .../ruler/cql/KnowledgeArtifactProcessor.java | 65 +++++++++-------- .../cqf/ruler/cql/RepositoryServiceTest.java | 70 +++++++++++-------- ...inimal-draft-to-test-version-conflict.json | 4 +- 3 files changed, 76 insertions(+), 63 deletions(-) diff --git a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java index 519d80b1d..d1305e80f 100644 --- a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java +++ b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java @@ -198,41 +198,12 @@ ArtifactAssessment createApprovalAssessment(IdType id, String artifactCommentTyp * the new versions. */ public Bundle createDraftBundle(IdType baseArtifactId, FhirDal fhirDal, String version) throws ResourceNotFoundException, UnprocessableEntityException { - if (version == null || version.isEmpty()) { - throw new InvalidOperatorArgument("The version argument is required"); - } - + checkIfVersionIsValid(version); MetadataResource baseArtifact = (MetadataResource) fhirDal.read(baseArtifactId); if (baseArtifact == null) { throw new ResourceNotFoundException(baseArtifactId); } - // Check if version is valid - if(version.contains("/") || version.contains("\\") || version.contains("|")){ - throw new UnprocessableEntityException("The version contains illegal characters"); - } - if (!version.contains(".")) { - throw new UnprocessableEntityException("The version must be in the format MAJOR.MINOR.PATCH"); - } else { - String[] versionParts = version.split("\\."); - if(versionParts.length != 3){ - throw new UnprocessableEntityException("The version must be in the format MAJOR.MINOR.PATCH"); - } - for(int i = 0; i < versionParts.length; i++) { - String section = ""; - if(Integer.parseInt(versionParts[i]) < 0) { - if(i == 0) { - section = "Major"; - } else if(i == 1) { - section = "Minor"; - } else if (i == 2) { - section = "Patch"; - } - throw new UnprocessableEntityException("The " + section + " version part should be greater than 0."); - } - } - } - String draftVersion = version + "-draft"; String draftVersionUrl = Canonicals.getUrl(baseArtifact.getUrl()) + "|" + draftVersion; @@ -242,7 +213,7 @@ public Bundle createDraftBundle(IdType baseArtifactId, FhirDal fhirDal, String v // for the root artifact and those referenced by it. if (baseArtifact.getStatus() != Enumerations.PublicationStatus.ACTIVE) { throw new UnprocessableEntityException( - String.format("Drafts can only be created from artifacts with status of 'active'. Resource '%s' has a status of: %s", baseArtifact.getUrl(), baseArtifact.getStatus().toString())); + String.format("Drafts can only be created from artifacts with status of 'active'. Resource '%s' has a status of: %s", baseArtifact.getUrl(), String.valueOf(baseArtifact.getStatus()))); } // Ensure only one resource exists with this URL Bundle existingArtifactsForUrl = searchResourceByUrl(draftVersionUrl, fhirDal); @@ -287,6 +258,38 @@ private void updateRelatedArtifactUrlsWithNewVersions(KnowledgeArtifactAdapter ra.hasResource()).collect(Collectors.toList()) .replaceAll(ra -> ra.setResource(Canonicals.getUrl(ra.getResource()) + "|" + draftVersion)); } + private void checkIfVersionIsValid(String version) throws UnprocessableEntityException{ + if (version == null || version.isEmpty()) { + throw new UnprocessableEntityException("The version argument is required"); + } + if(version.contains("draft")){ + throw new UnprocessableEntityException("The version cannot contain 'draft'"); + } + if(version.contains("/") || version.contains("\\") || version.contains("|")){ + throw new UnprocessableEntityException("The version contains illegal characters"); + } + if (!version.contains(".")) { + throw new UnprocessableEntityException("The version must be in the format MAJOR.MINOR.PATCH"); + } else { + String[] versionParts = version.split("\\."); + if(versionParts.length != 3){ + throw new UnprocessableEntityException("The version must be in the format MAJOR.MINOR.PATCH"); + } + for(int i = 0; i < versionParts.length; i++) { + String section = ""; + if(Integer.parseInt(versionParts[i]) < 0) { + if(i == 0) { + section = "Major"; + } else if(i == 1) { + section = "Minor"; + } else if (i == 2) { + section = "Patch"; + } + throw new UnprocessableEntityException("The " + section + " version part should be greater than 0."); + } + } + } + } private List createDraftsOfArtifactAndRelated(MetadataResource resourceToDraft, FhirDal fhirDal, String version, List resourcesToCreate) { String draftVersion = version + "-draft"; String draftVersionUrl = Canonicals.getUrl(resourceToDraft.getUrl()) + "|" + draftVersion; diff --git a/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java b/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java index 796874111..0b63db863 100644 --- a/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java +++ b/plugin/cql/src/test/java/org/opencds/cqf/ruler/cql/RepositoryServiceTest.java @@ -26,7 +26,7 @@ import org.hl7.fhir.r4.model.Parameters; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.RelatedArtifact; -import org.hl7.fhir.r4.model.Resource; +import org.hl7.fhir.r4.model.StringType; import org.junit.jupiter.api.Test; import org.opencds.cqf.cql.evaluator.fhir.util.Canonicals; import org.opencds.cqf.ruler.cql.r4.ArtifactAssessment; @@ -43,20 +43,20 @@ class RepositoryServiceTest extends RestIntegrationTest { private final String specificationLibReference = "Library/SpecificationLibrary"; + private final String minimalLibReference = "Library/SpecificationLibraryDraftVersion-1-1-1"; @Test void draftOperation_test() { loadTransaction("ersd-active-transaction-bundle-example.json"); - String version = "1.1.1"; + String version = "1.0.1"; String draftedVersion = version + "-draft"; Parameters params = parameters(part("version", version) ); - Resource returnResource = getClient().operation() + Bundle returnedBundle = getClient().operation() .onInstance(specificationLibReference) .named("$draft") .withParameters(params) .returnResourceType(Bundle.class) .execute(); - Bundle returnedBundle = (Bundle) returnResource; assertNotNull(returnedBundle); Optional maybeLib = returnedBundle.getEntry().stream().filter(entry -> entry.getResponse().getLocation().contains("Library")).findAny(); assertTrue(maybeLib.isPresent()); @@ -69,58 +69,63 @@ void draftOperation_test() { assertTrue(Canonicals.getVersion(relatedArtifacts.get(0).getResource()).equals(draftedVersion)); assertTrue(Canonicals.getVersion(relatedArtifacts.get(1).getResource()).equals(draftedVersion)); } - @Test - void draftOperation_draft_test() { - loadTransaction("ersd-draft-transaction-bundle-example.json"); - Parameters params = parameters(part("version", "1.2.1") ); + void draftOperation_version_conflict_test() { + loadTransaction("ersd-active-transaction-bundle-example.json"); + loadResource("minimal-draft-to-test-version-conflict.json"); + Parameters params = parameters(part("version", "1.1.1") ); + UnprocessableEntityException maybeException = null; try { getClient().operation() - .onInstance("Library/DraftSpecificationLibrary") + .onInstance(specificationLibReference) .named("$draft") .withParameters(params) .returnResourceType(Bundle.class) .execute(); } catch (UnprocessableEntityException e) { - assertNotNull(e); - assertTrue(e.getMessage().contains("status of 'active'")); + maybeException = e; } + assertNotNull(maybeException); + assertTrue(maybeException.getMessage().contains("already exists")); } + @Test - void draftOperation_wrong_id_test() { - loadTransaction("ersd-draft-transaction-bundle-example.json"); - Parameters params = parameters(part("version", "1.3.1") ); + void draftOperation_draft_test() { + loadResource("minimal-draft-to-test-version-conflict.json"); + Parameters params = parameters(part("version", "1.2.1") ); + UnprocessableEntityException maybeException = null; try { getClient().operation() - .onInstance("Library/there-is-no-such-id") + .onInstance(minimalLibReference) .named("$draft") .withParameters(params) .returnResourceType(Bundle.class) .execute(); - } catch (ResourceNotFoundException e) { - assertNotNull(e); + } catch (UnprocessableEntityException e) { + maybeException = e; } + assertNotNull(maybeException); + assertTrue(maybeException.getMessage().contains("status of 'active'")); } @Test - void draftOperation_version_conflict_test() { - loadTransaction("ersd-active-transaction-bundle-example.json"); - loadResource("minimal-draft-to-test-version-conflict.json"); - Parameters params = parameters(part("version", "1.4.1") ); + void draftOperation_wrong_id_test() { + loadTransaction("ersd-draft-transaction-bundle-example.json"); + Parameters params = parameters(part("version", "1.3.1") ); + ResourceNotFoundException maybeException = null; try { getClient().operation() - .onInstance(specificationLibReference) + .onInstance("Library/there-is-no-such-id") .named("$draft") .withParameters(params) .returnResourceType(Bundle.class) .execute(); - } catch (UnprocessableEntityException e) { - assertNotNull(e); - assertTrue(e.getMessage().contains("already exists")); + } catch (ResourceNotFoundException e) { + maybeException = e; } + assertNotNull(maybeException); } @Test void draftOperation_version_format_test() { - loadTransaction("ersd-active-transaction-bundle-example.json"); loadResource("minimal-draft-to-test-version-conflict.json"); List testValues = Arrays.asList(new String[]{ "11asd1", @@ -129,20 +134,25 @@ void draftOperation_version_format_test() { "1/.1.1", "-1.-1.2", "1.-1.2", - "1.1.-2" + "1.1.-2", + "1.2.3-draft", + "", + null }); for(String version:testValues){ - Parameters params = parameters(part("version", version) ); + UnprocessableEntityException maybeException = null; + Parameters params = parameters(part("version", new StringType(version)) ); try { getClient().operation() - .onInstance(specificationLibReference) + .onInstance(minimalLibReference) .named("$draft") .withParameters(params) .returnResourceType(Bundle.class) .execute(); } catch (UnprocessableEntityException e) { - assertNotNull(e); + maybeException = e; } + assertNotNull(maybeException); } } //@Test diff --git a/plugin/cql/src/test/resources/minimal-draft-to-test-version-conflict.json b/plugin/cql/src/test/resources/minimal-draft-to-test-version-conflict.json index d3219c1dc..475dd3d68 100644 --- a/plugin/cql/src/test/resources/minimal-draft-to-test-version-conflict.json +++ b/plugin/cql/src/test/resources/minimal-draft-to-test-version-conflict.json @@ -1,6 +1,6 @@ { "resourceType": "Library", "id": "SpecificationLibraryDraftVersion-1-1-1", - "url": "http://hl7.org/fhir/us/ecr/Library/SpecificationLibrary", - "version": "1.1.1" + "url": "http://ersd.aimsplatform.org/fhir/Library/SpecificationLibrary", + "version": "1.1.1-draft" } From 6cdb2e167120c2ee7f639c847140dd037df6c385 Mon Sep 17 00:00:00 2001 From: "taha.attari@smilecdr.com" Date: Tue, 18 Apr 2023 18:39:56 -0400 Subject: [PATCH 4/4] [APHL-535] rebase to vsm_operations --- .../cqf/ruler/utility/SemanticVersion.java | 42 +++++++++++++ .../ruler/cql/KnowledgeArtifactAdapter.java | 1 + .../ruler/cql/KnowledgeArtifactProcessor.java | 63 ++++++++----------- 3 files changed, 69 insertions(+), 37 deletions(-) create mode 100644 core/src/main/java/org/opencds/cqf/ruler/utility/SemanticVersion.java diff --git a/core/src/main/java/org/opencds/cqf/ruler/utility/SemanticVersion.java b/core/src/main/java/org/opencds/cqf/ruler/utility/SemanticVersion.java new file mode 100644 index 000000000..6ca0f4442 --- /dev/null +++ b/core/src/main/java/org/opencds/cqf/ruler/utility/SemanticVersion.java @@ -0,0 +1,42 @@ +package org.opencds.cqf.ruler.utility; + +import java.util.List; +import java.util.Comparator; + +public class SemanticVersion { + public static Comparator getVersionComparator() { + return new VersionComparator(); + } + + public static String findHighestVersion(List versions) { + String highestVersion = null; + Comparator versionComparator = new VersionComparator(); + + for (String version : versions) { + if (highestVersion == null || versionComparator.compare(version, highestVersion) > 0) { + highestVersion = version; + } + } + + return highestVersion; + } + + public static class VersionComparator implements Comparator { + @Override + public int compare(String v1, String v2) { + String[] v1Parts = v1.split("\\."); + String[] v2Parts = v2.split("\\."); + + for (int i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) { + int num1 = i < v1Parts.length ? Integer.parseInt(v1Parts[i]) : 0; + int num2 = i < v2Parts.length ? Integer.parseInt(v2Parts[i]) : 0; + + if (num1 != num2) { + return num1 - num2; + } + } + + return 0; + } + } +} \ No newline at end of file diff --git a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactAdapter.java b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactAdapter.java index bcd39997a..b1ce162b3 100644 --- a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactAdapter.java +++ b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactAdapter.java @@ -17,6 +17,7 @@ import org.hl7.fhir.r4.model.RelatedArtifact; import org.hl7.fhir.r4.model.Resource; import org.hl7.fhir.r4.model.ValueSet; +import org.opencds.cqf.ruler.utility.SemanticVersion; public class KnowledgeArtifactAdapter { protected T resource; diff --git a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java index d1305e80f..4f44a7976 100644 --- a/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java +++ b/plugin/cql/src/main/java/org/opencds/cqf/ruler/cql/KnowledgeArtifactProcessor.java @@ -14,37 +14,29 @@ import org.apache.commons.lang3.NotImplementedException; import org.cqframework.fhir.api.FhirDal; import org.hl7.fhir.exceptions.FHIRException; -import org.hl7.fhir.instance.model.api.IBaseBundle; +import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.Bundle.BundleEntryComponent; +import org.hl7.fhir.r4.model.Bundle.BundleEntryRequestComponent; import org.hl7.fhir.r4.model.CanonicalType; import org.hl7.fhir.r4.model.CodeType; import org.hl7.fhir.r4.model.ContactDetail; -import org.hl7.fhir.r4.model.DataRequirement; import org.hl7.fhir.r4.model.DateTimeType; -import org.hl7.fhir.r4.model.ElementDefinition; import org.hl7.fhir.r4.model.Enumerations; -import org.hl7.fhir.r4.model.Expression; -import org.hl7.fhir.r4.model.Extension; import org.hl7.fhir.r4.model.IdType; -import org.hl7.fhir.r4.model.Library; import org.hl7.fhir.r4.model.MarkdownType; import org.hl7.fhir.r4.model.MetadataResource; -import org.hl7.fhir.r4.model.PlanDefinition; import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.RelatedArtifact; -import org.hl7.fhir.r4.model.StructureDefinition; -import org.hl7.fhir.r4.model.Type; +import org.hl7.fhir.r4.model.Resource; import org.hl7.fhir.r4.model.UsageContext; -import org.hl7.fhir.r4.model.ValueSet; import org.jetbrains.annotations.Nullable; import org.opencds.cqf.cql.engine.exception.InvalidOperatorArgument; import org.opencds.cqf.cql.evaluator.fhir.util.Canonicals; -import org.opencds.cqf.ruler.builder.BundleBuilder; import org.opencds.cqf.ruler.cql.r4.ArtifactAssessment; import org.opencds.cqf.ruler.cql.r4.ArtifactAssessment.ArtifactAssessmentContentInformationType; import org.springframework.beans.factory.annotation.Configurable; -import org.springframework.web.client.ResourceAccessException; import ca.uhn.fhir.model.api.IQueryParameterType; import ca.uhn.fhir.rest.param.TokenParam; @@ -59,7 +51,25 @@ public class KnowledgeArtifactProcessor { public static final String CPG_INFERENCEEXPRESSION = "http://hl7.org/fhir/uv/cpg/StructureDefinition/cpg-inferenceExpression"; public static final String CPG_ASSERTIONEXPRESSION = "http://hl7.org/fhir/uv/cpg/StructureDefinition/cpg-assertionExpression"; public static final String CPG_FEATUREEXPRESSION = "http://hl7.org/fhir/uv/cpg/StructureDefinition/cpg-featureExpression"; + private BundleEntryComponent createEntry(IBaseResource theResource) { + return new Bundle.BundleEntryComponent() + .setResource((Resource) theResource) + .setRequest(createRequest(theResource)); + } + private BundleEntryRequestComponent createRequest(IBaseResource theResource) { + Bundle.BundleEntryRequestComponent request = new Bundle.BundleEntryRequestComponent(); + if (theResource.getIdElement().hasValue() && !theResource.getIdElement().getValue().contains("urn:uuid")) { + request + .setMethod(Bundle.HTTPVerb.PUT) + .setUrl(theResource.getIdElement().getValue()); + } else { + request + .setMethod(Bundle.HTTPVerb.POST) + .setUrl(theResource.fhirType()); + } + return request; + } private Bundle searchResourceByUrl(String url, FhirDal fhirDal) { Map>> searchParams = new HashMap<>(); @@ -100,34 +110,13 @@ private Bundle searchResourceByUrlAndStatus(String url, String status, FhirDal f return searchResultsBundle; } - private List getResourcesFromBundle(Bundle bundle) { - List resourceList = new ArrayList<>(); - - if (!bundle.getEntryFirstRep().isEmpty()) { - List referencedResourceEntries = bundle.getEntry(); - for (Bundle.BundleEntryComponent entry: referencedResourceEntries) { - if (entry.hasResource() && entry.getResource() instanceof MetadataResource) { - MetadataResource referencedResource = (MetadataResource) entry.getResource(); - resourceList.add(referencedResource); - } - } - } - - return resourceList; - } - - private MetadataResource retrieveResourcesByReference(String reference, FhirDal fhirDal) { - MetadataResource resource = null; - + private MetadataResource retrieveResourcesByCanonical(String reference, FhirDal fhirDal) throws ResourceNotFoundException { Bundle referencedResourceBundle = searchResourceByUrl(reference, fhirDal); - MetadataResource referencedResource = KnowledgeArtifactAdapter.findLatestVersion(referencedResourceBundle); - if (referencedResource != null) { - resource = referencedResource; - } else { + Optional referencedResource = KnowledgeArtifactAdapter.findLatestVersion(referencedResourceBundle); + if (referencedResource.isEmpty()) { throw new ResourceNotFoundException(String.format("Resource for Canonical '%s' not found.", reference)); } - - return resource; + return referencedResource.get(); } /* approve */