From 0718f6b52a5e913c59f07cedc941d2b5e109d5db Mon Sep 17 00:00:00 2001 From: James Agnew Date: Tue, 21 Jan 2020 07:14:16 +0900 Subject: [PATCH] Allow schemalocation in XML validation (#1677) * Allow schemaLocation declaration in XML when validating * Add fix for #1676 --- ...-allow-schemalocation-when-validating.yaml | 5 ++ .../jpa/migrate/taskdef/ArbitrarySqlTask.java | 1 - .../jpa/migrate/taskdef/RenameColumnTask.java | 2 - .../fhir/jpa/migrate/tasks/api/Builder.java | 44 +++++++------- .../taskdef/ExecuteRawSqlTaskTest.java | 48 +++++++++++++++ .../hapi/validation/ValidatorWrapper.java | 1 + .../FhirInstanceValidatorR4Test.java | 58 +++++++++++++++---- 7 files changed, 125 insertions(+), 34 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/1676-allow-schemalocation-when-validating.yaml create mode 100644 hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ExecuteRawSqlTaskTest.java diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/1676-allow-schemalocation-when-validating.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/1676-allow-schemalocation-when-validating.yaml new file mode 100644 index 00000000000..81c38293e40 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/1676-allow-schemalocation-when-validating.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 1676 +title: "When validating an XML resource, the validatin failed if the resource contained an + `xsi:schemaLocation` declaration. This has been corrected. Thanks to Brian Kaney for reporting!" diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ArbitrarySqlTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ArbitrarySqlTask.java index 0941db93f7d..3587add5501 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ArbitrarySqlTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ArbitrarySqlTask.java @@ -187,7 +187,6 @@ public class ArbitrarySqlTask extends BaseTask { public int hashCode() { return new HashCodeBuilder(17, 37) .append(myTableName) - .append(myExecuteOnlyIfTableExists) .toHashCode(); } } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTask.java index 99120586b0d..d3f7b270529 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTask.java @@ -159,8 +159,6 @@ public class RenameColumnTask extends BaseTableTask { .appendSuper(super.hashCode()) .append(myOldName) .append(myNewName) - .append(myIsOkayIfNeitherColumnExists) - .append(myDeleteTargetColumnFirstIfBothExist) .toHashCode(); } } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java index bd14e36113e..a7de3803e1d 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/Builder.java @@ -50,9 +50,10 @@ public class Builder { return new BuilderAddTableRawSql(theVersion, theTableName); } - public Builder executeRawSql(String theVersion, @Language("SQL") String theSql) { - mySink.addTask(new ExecuteRawSqlTask(myRelease, theVersion).addSql(theSql)); - return this; + public BuilderCompleteTask executeRawSql(String theVersion, @Language("SQL") String theSql) { + ExecuteRawSqlTask task = new ExecuteRawSqlTask(myRelease, theVersion).addSql(theSql); + mySink.addTask(task); + return new BuilderCompleteTask(task); } public Builder initializeSchema(String theVersion, ISchemaInitializationProvider theSchemaInitializationProvider) { @@ -72,6 +73,7 @@ public class Builder { return this; } + // Flyway doesn't support these kinds of migrations @Deprecated public Builder startSectionWithMessage(String theMessage) { @@ -395,7 +397,7 @@ public class Builder { } } - public static class BuilderAddColumnWithName { + public class BuilderAddColumnWithName { private final String myRelease; private final String myVersion; private final String myColumnName; @@ -427,11 +429,11 @@ public class Builder { myNullable = theNullable; } - public BuilderAddColumnComplete type(AddColumnTask.ColumnTypeEnum theColumnType) { + public BuilderCompleteTask type(AddColumnTask.ColumnTypeEnum theColumnType) { return type(theColumnType, null); } - public BuilderAddColumnComplete type(AddColumnTask.ColumnTypeEnum theColumnType, Integer theLength) { + public BuilderCompleteTask type(AddColumnTask.ColumnTypeEnum theColumnType, Integer theLength) { AddColumnTask task = new AddColumnTask(myRelease, myVersion); task.setColumnName(myColumnName); task.setNullable(myNullable); @@ -441,24 +443,26 @@ public class Builder { } myTaskSink.addTask(task); - return new BuilderAddColumnComplete(task); + return new BuilderCompleteTask(task); } - public class BuilderAddColumnComplete { - - private final AddColumnTask myTask; - - public BuilderAddColumnComplete(AddColumnTask theTask) { - myTask = theTask; - } - - public BuilderAddColumnComplete failureAllowed() { - myTask.setFailureAllowed(true); - return this; - } - } } } } + + public static class BuilderCompleteTask { + + private final BaseTask myTask; + + public BuilderCompleteTask(BaseTask theTask) { + myTask = theTask; + } + + public BuilderCompleteTask failureAllowed() { + myTask.setFailureAllowed(true); + return this; + } + } + } diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ExecuteRawSqlTaskTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ExecuteRawSqlTaskTest.java new file mode 100644 index 00000000000..96ec78d6aa6 --- /dev/null +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/ExecuteRawSqlTaskTest.java @@ -0,0 +1,48 @@ +package ca.uhn.fhir.jpa.migrate.taskdef; + +import ca.uhn.fhir.jpa.migrate.tasks.api.BaseMigrationTasks; +import ca.uhn.fhir.util.VersionEnum; +import org.junit.Test; + +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertEquals; + +public class ExecuteRawSqlTaskTest extends BaseTest { + + @Test + public void testExecuteSql() { + executeSql("create table SOMETABLE (PID bigint not null, TEXTCOL varchar(255))"); + + BaseMigrationTasks tasks = new BaseMigrationTasks<>(); + tasks + .forVersion(VersionEnum.V4_0_0) + .executeRawSql("2001.01", "INSERT INTO SOMETABLE (PID, TEXTCOL) VALUES (123, 'abc')"); + + getMigrator().addTasks(tasks.getTasks(VersionEnum.V0_1, VersionEnum.V4_0_0)); + getMigrator().migrate(); + + List> output = executeQuery("SELECT PID FROM SOMETABLE"); + assertEquals(1, output.size()); + assertEquals(123L, output.get(0).get("PID")); + } + + @Test + public void testExecuteSql_AllowedToFail() { + executeSql("create table SOMETABLE (PID bigint not null, TEXTCOL varchar(255))"); + + BaseMigrationTasks tasks = new BaseMigrationTasks<>(); + tasks + .forVersion(VersionEnum.V4_0_0) + .executeRawSql("2001.01", "INSERT INTO SOMETABLE (PID_BAD_COLUMN, TEXTCOL) VALUES (123, 'abc')") + .failureAllowed(); + + getMigrator().addTasks(tasks.getTasks(VersionEnum.V0_1, VersionEnum.V4_0_0)); + getMigrator().migrate(); + + List> output = executeQuery("SELECT PID FROM SOMETABLE"); + assertEquals(0, output.size()); + } + +} diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/ValidatorWrapper.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/ValidatorWrapper.java index 61f1358ec1b..d1c28f2b274 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/ValidatorWrapper.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/ValidatorWrapper.java @@ -82,6 +82,7 @@ public class ValidatorWrapper { v.setNoTerminologyChecks(myNoTerminologyChecks); v.setErrorForUnknownProfiles(myErrorForUnknownProfiles); v.getExtensionDomains().addAll(myExtensionDomains); + v.setAllowXsiLocation(true); List messages = new ArrayList<>(); diff --git a/hapi-fhir-validation/src/test/java/org/hl7/fhir/r4/validation/FhirInstanceValidatorR4Test.java b/hapi-fhir-validation/src/test/java/org/hl7/fhir/r4/validation/FhirInstanceValidatorR4Test.java index 41968e97891..ce094c1a8a8 100644 --- a/hapi-fhir-validation/src/test/java/org/hl7/fhir/r4/validation/FhirInstanceValidatorR4Test.java +++ b/hapi-fhir-validation/src/test/java/org/hl7/fhir/r4/validation/FhirInstanceValidatorR4Test.java @@ -12,7 +12,6 @@ import ca.uhn.fhir.validation.ValidationResult; import com.google.common.base.Charsets; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.Validate; -import org.hl7.fhir.dstu3.hapi.validation.ResourceValidatorDstu3Test; import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.r4.conformance.ProfileUtilities; @@ -36,7 +35,11 @@ import org.hl7.fhir.r4.utils.FHIRPathEngine; import org.hl7.fhir.r5.utils.IResourceValidator; import org.hl7.fhir.utilities.validation.ValidationMessage; import org.hl7.fhir.utilities.xhtml.XhtmlNode; -import org.junit.*; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; import org.junit.rules.TestRule; import org.junit.rules.TestWatcher; import org.junit.runner.Description; @@ -44,13 +47,25 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import java.io.IOException; -import java.io.InputStream; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; import java.util.stream.Collectors; -import java.util.zip.GZIPInputStream; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -127,8 +142,8 @@ public class FhirInstanceValidatorR4Test extends BaseTest { }); when(myMockSupport.isCodeSystemSupported(nullable(FhirContext.class), nullable(String.class))).thenAnswer(new Answer() { @Override - public Boolean answer(InvocationOnMock theInvocation) throws Throwable { - boolean retVal = myValidSystems.contains(theInvocation.getArguments()[1]); + public Boolean answer(InvocationOnMock theInvocation) { + boolean retVal = myValidSystems.contains(theInvocation.getArgument(1, String.class)); ourLog.debug("isCodeSystemSupported({}) : {}", new Object[]{theInvocation.getArguments()[1], retVal}); return retVal; } @@ -139,7 +154,7 @@ public class FhirInstanceValidatorR4Test extends BaseTest { IBaseResource retVal; String id = (String) theInvocation.getArguments()[2]; if ("Questionnaire/q_jon".equals(id)) { - retVal = ourCtx.newJsonParser().parseResource(IOUtils.toString(FhirInstanceValidatorR4Test.class.getResourceAsStream("/q_jon.json"))); + retVal = ourCtx.newJsonParser().parseResource(loadResource("/q_jon.json")); } else { retVal = myDefaultValidationSupport.fetchResource((FhirContext) theInvocation.getArguments()[0], (Class) theInvocation.getArguments()[1], id); } @@ -210,7 +225,7 @@ public class FhirInstanceValidatorR4Test extends BaseTest { } private List logResultsAndReturnAll(ValidationResult theOutput) { - List retVal = new ArrayList(); + List retVal = new ArrayList<>(); int index = 0; for (SingleValidationMessage next : theOutput.getMessages()) { @@ -264,6 +279,27 @@ public class FhirInstanceValidatorR4Test extends BaseTest { assertEquals("Primitive types must have a value that is not empty", all.get(0).getMessage()); } + /** + * See #1676 - We should ignore schema location + */ + @Test + public void testValidateResourceWithSchemaLocation() { + String input = "" + + " \n" + + " \n" + + "
AAA
\n" + + "
" + + " " + + "
"; + + FhirValidator val = ourCtx.newValidator(); + val.registerValidatorModule(new FhirInstanceValidator(myDefaultValidationSupport)); + + ValidationResult result = val.validateWithResult(input); + logResultsAndReturnAll(result); + assertTrue(result.isSuccessful()); + } + /** * See #942 */