From cc89145fd788c2d753789b43eec3b7a53bc0ea38 Mon Sep 17 00:00:00 2001 From: mdnazmulkarim Date: Wed, 18 Oct 2023 15:19:21 -0600 Subject: [PATCH] Apply private access modifier (#1239) * add test for private access modifier * add impl for private access modifier * rectify test function name * cover private access for parameter, codesystem, valueset, code, concept, function, expression * identify possible bug * enable tests after the merged fix --- .../cql/cql2elm/LibraryBuilder.java | 25 +++++++---- .../cqframework/cql/cql2elm/LibraryTests.java | 32 +++++++++++++- .../LibraryTests/AccessModifierBase.cql | 19 ++++++++ .../AccessModifierNonReferencing.cql | 44 +++++++++++++++++++ .../AccessModifierReferencing.cql | 26 +++++++++++ 5 files changed, 137 insertions(+), 9 deletions(-) create mode 100644 Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierBase.cql create mode 100644 Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierNonReferencing.cql create mode 100644 Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierReferencing.cql diff --git a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java index 05dc1440c..ec0390a2c 100644 --- a/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java +++ b/Src/java/cql-to-elm/src/main/java/org/cqframework/cql/cql2elm/LibraryBuilder.java @@ -1,5 +1,6 @@ package org.cqframework.cql.cql2elm; +import org.apache.commons.lang3.StringUtils; import org.cqframework.cql.cql2elm.model.*; import org.cqframework.cql.cql2elm.model.invocation.*; import org.cqframework.cql.elm.tracking.TrackBack; @@ -1274,20 +1275,27 @@ public OperatorResolution resolveCall(CallContext callContext) { } } */ - - if (result != null) { - checkAccessLevel(result.getOperator().getLibraryName(), result.getOperator().getName(), - result.getOperator().getAccessLevel()); - } } } else { result = resolveLibrary(callContext.getLibraryName()).resolveCall(callContext, conversionMap); } + if (result != null) { + checkAccessLevel(result.getOperator().getLibraryName(), result.getOperator().getName(), + result.getOperator().getAccessLevel()); + } + return result; } + private boolean isInterFunctionAccess(String f1, String f2) { + if(StringUtils.isNoneBlank(f1) && StringUtils.isNoneBlank(f2)) { + return !f1.equalsIgnoreCase(f2); + } + return false; + } + public void checkOperator(CallContext callContext, OperatorResolution resolution) { if (resolution == null) { // ERROR: @@ -1307,9 +1315,10 @@ public void checkOperator(CallContext callContext, OperatorResolution resolution } public void checkAccessLevel(String libraryName, String objectName, AccessModifier accessModifier) { - if (accessModifier == AccessModifier.PRIVATE) { + if (accessModifier == AccessModifier.PRIVATE && + isInterFunctionAccess(this.library.getIdentifier().getId(), libraryName)) { // ERROR: - throw new IllegalArgumentException(String.format("Object %s in library %s is marked private and cannot be referenced from another library.", objectName, libraryName)); + throw new CqlSemanticException(String.format("Identifier %s in library %s is marked private and cannot be referenced from another library.", objectName, libraryName)); } } @@ -2732,7 +2741,7 @@ public Expression resolveAccessor(Expression left, String memberIdentifier) { } if (element instanceof CodeSystemDef) { - checkAccessLevel(libraryName, memberIdentifier, ((CodeSystemDef)element).getAccessLevel()); + checkAccessLevel(libraryName, memberIdentifier, ((CodeSystemDef) element).getAccessLevel()); CodeSystemRef result = of.createCodeSystemRef() .withLibraryName(libraryName) .withName(memberIdentifier); diff --git a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/LibraryTests.java b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/LibraryTests.java index 9ff8e22fa..1bc83c7ba 100644 --- a/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/LibraryTests.java +++ b/Src/java/cql-to-elm/src/test/java/org/cqframework/cql/cql2elm/LibraryTests.java @@ -12,11 +12,16 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; import org.cqframework.cql.cql2elm.LibraryBuilder.SignatureLevel; import org.hl7.cql_annotations.r1.CqlToElmError; import org.hl7.elm.r1.*; -import org.testng.annotations.*; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; public class LibraryTests { @@ -142,6 +147,31 @@ public void testInvalidLibraryReferences() { } } + @Test + public void testPrivateAccessModifierReferencing() throws IOException { + CqlTranslator translator = TestUtils.createTranslatorFromStream("LibraryTests/AccessModifierReferencing.cql"); + assertThat(translator.getErrors().size(), is(not(0))); + + Set errors = translator.getErrors().stream() + .map(CqlCompilerException::getMessage) + .collect(Collectors.toSet()); + + assertTrue(errors.contains("Identifier ICD-10:2014 in library Base is marked private and cannot be referenced from another library.")); + assertTrue(errors.contains("Identifier f1 in library AccessModifierBase is marked private and cannot be referenced from another library.")); + assertTrue(errors.contains("Identifier PrivateExpression in library Base is marked private and cannot be referenced from another library.")); + assertTrue(errors.contains("Identifier Test Parameter in library Base is marked private and cannot be referenced from another library.")); + assertTrue(errors.contains("Identifier Female Administrative Sex in library Base is marked private and cannot be referenced from another library.")); + assertTrue(errors.contains("Identifier XYZ Code in library Base is marked private and cannot be referenced from another library.")); + assertTrue(errors.contains("Identifier XYZ Concept in library Base is marked private and cannot be referenced from another library.")); + + } + + @Test + public void testPrivateAccessModifierNonReferencing() throws IOException { + CqlTranslator translator = TestUtils.createTranslatorFromStream("LibraryTests/AccessModifierNonReferencing.cql"); + assertThat(translator.getErrors().size(), is(0)); + } + @Test public void testInvalidLibraryReference() { CqlTranslator translator = null; diff --git a/Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierBase.cql b/Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierBase.cql new file mode 100644 index 000000000..e11173a65 --- /dev/null +++ b/Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierBase.cql @@ -0,0 +1,19 @@ +library AccessModifierBase + +private codesystem "ICD-10:2014": 'ICD-10' version '2014' + +private codesystem "SNOMED-CT:2020": 'http://snomed.info/sct' version '2020' +private codesystem "ICD-9CM:2020": '2.16.840.1.113883.6.103' version '2020' + +private code "XYZ Code": 'XYZ' from "SNOMED-CT:2020" display 'XYZ Code' +private code "ABC Code": 'ABC' from "ICD-9CM:2020" display 'ABC Code' + +private concept "XYZ Concept": { "XYZ Code", "ABC Code" } display 'XYZ Concept' + +private valueset "Female Administrative Sex": '2.16.840.1.113883.3.560.100.2' + +private parameter "Test Parameter" Integer + +define private function f1(arg String): arg + +define private PrivateExpression: Tuple { Id : '12345', Name : 'John Doe' } \ No newline at end of file diff --git a/Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierNonReferencing.cql b/Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierNonReferencing.cql new file mode 100644 index 000000000..7b05660ec --- /dev/null +++ b/Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierNonReferencing.cql @@ -0,0 +1,44 @@ +library AccessModifierNonReferencing + +//********private member definition begin********** +private codesystem "ICD-10:2014": 'ICD-10' version '2014' + +private codesystem "SNOMED-CT:2020": 'http://snomed.info/sct' version '2020' +private codesystem "ICD-9CM:2020": '2.16.840.1.113883.6.103' version '2020' + +private code "XYZ Code": 'XYZ' from "SNOMED-CT:2020" display 'XYZ Code' +private code "ABC Code": 'ABC' from "ICD-9CM:2020" display 'ABC Code' + +private concept "XYZ Concept": { "XYZ Code", "ABC Code" } display 'XYZ Concept' + +private valueset "Female Administrative Sex": '2.16.840.1.113883.3.560.100.2' +//private member access for codesystem +valueset "Chlamydia Screening": '2.16.840.1.113883.3.464.1003.110.12.1052' + codesystems { "ICD-10:2014" } + +private parameter "Test Parameter" Integer + +define private function f1(arg String): arg + +define private PrivateExpression: Tuple { Id : '12345', Name : 'John Doe' } + +//********private member definition end********** + +//private member access for function +define FunctionTestOuterPrivate: f1('hello') + +//private member access for expressionDef +define ReferenceExpression: PrivateExpression.Id + +//private member access for parameter +define "Test Definition": "Test Parameter" + +//private member access for valueset +define GenderExpression: 'Female' in "Female Administrative Sex" + +//private member access for code +define CodeRef: "XYZ Code" + +//private member access for concept +define ConceptRef: "XYZ Concept" + diff --git a/Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierReferencing.cql b/Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierReferencing.cql new file mode 100644 index 000000000..c9d7b9624 --- /dev/null +++ b/Src/java/cql-to-elm/src/test/resources/org/cqframework/cql/cql2elm/LibraryTests/AccessModifierReferencing.cql @@ -0,0 +1,26 @@ +library AccessModifierReferencing + +include AccessModifierBase called Base + +//private member access for codesystem +valueset "Chlamydia Screening": '2.16.840.1.113883.3.464.1003.110.12.1052' + codesystems { Base."ICD-10:2014" } + +//private member access for function +define FunctionTestOuterPrivate: Base.f1('hello') + +//private member access for expressionDef +define ReferenceExpression: Base.PrivateExpression.Id + +//private member access for parameter +define "Test Definition": Base."Test Parameter" + +//private member access for valueset +define GenderExpression: 'Female' in Base."Female Administrative Sex" + +//private member access for code +define CodeRef: Base."XYZ Code" + +//private member access for concept +define ConceptRef: Base."XYZ Concept" +