ensure ElementDefinition properties with longer names are processed first in order to find the property that most matches the given nodeName #523

Merged
merged 1 commit into from Dec 11, 2016

Conversation

Projects
None yet
4 participants
@CarthageKing

The EnrollmentResponse FHIR resource has ElementDefinition definitions for .request[x] and .requestOrganization[x]. Due to the way these definitions are ordered in the EnrollmentResponse StructureDefinition, HAPI will match the "requestOrganizationReference" nodeName to .request[x], which is wrong.

This fixes the error by ensuring that ElementDefinitions with longer names are processed first. So in the case of the EnrollmentResponse FHIR resource, the ElementDefinition for .requestOrganization[x] is processed first before .request[x].

michael.i.calderero
ensure ElementDefinition properties with longer names are processed
first in order to find the property that most matches the given nodeName
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Dec 3, 2016

Coverage Status

Coverage increased (+0.007%) to 87.689% when pulling 7a203fa on CarthageKing:master into ffefb79 on jamesagnew:master.

Coverage Status

Coverage increased (+0.007%) to 87.689% when pulling 7a203fa on CarthageKing:master into ffefb79 on jamesagnew:master.

@jamesagnew

This comment has been minimized.

Show comment
Hide comment
@jamesagnew

jamesagnew Dec 11, 2016

Owner

Cool! Thanks for the PR! Will merge now.

Owner

jamesagnew commented Dec 11, 2016

Cool! Thanks for the PR! Will merge now.

@jamesagnew jamesagnew merged commit 2bca44e into jamesagnew:master Dec 11, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.007%) to 87.689%
Details
@jamesagnew

This comment has been minimized.

Show comment
Hide comment
@jamesagnew

jamesagnew Dec 11, 2016

Owner

Although... with this merged, it seems like the latest FHIR definitions have resolved this weirdness in the actual model, so the unit test doesn't compile anymore so I need to disable it. You don't know of nay other areas in the model that exhibit this behaviour do you?

Owner

jamesagnew commented Dec 11, 2016

Although... with this merged, it seems like the latest FHIR definitions have resolved this weirdness in the actual model, so the unit test doesn't compile anymore so I need to disable it. You don't know of nay other areas in the model that exhibit this behaviour do you?

jamesagnew added a commit that referenced this pull request Dec 11, 2016

@grahamegrieve

This comment has been minimized.

Show comment
Hide comment
@grahamegrieve

grahamegrieve Dec 11, 2016

Collaborator
Collaborator

grahamegrieve commented Dec 11, 2016

@CarthageKing

This comment has been minimized.

Show comment
Hide comment
@CarthageKing

CarthageKing Dec 12, 2016

Hello All,

The EligibilityResponse resource as of FHIR 1.8.0 (i.e. http://hl7.org/fhir/2017Jan/eligibilityresponse.html) still has this weirdness in the .benefit[x] and .benefitUsed[x] properties.

Using the following test below (in ResourceValidatorDstu3Test.java):

	@Test
	public void testValidateDifferentPropertyButSameStartsWithPath() throws Exception {

		EligibilityResponse fhirObj = new EligibilityResponse();
		BenefitComponent benComp = fhirObj.addInsurance().addBenefitBalance().addFinancial();
		// Test between .benefit[x] and benefitUsed[x]
		benComp.setBenefitUsed(new UnsignedIntType(2));
		
		String input = ourCtx.newXmlParser().encodeResourceToString(fhirObj);
		
		FhirValidator validator = ourCtx.newValidator();
		validator.registerValidatorModule(new FhirInstanceValidator());

		ValidationResult result = validator.validateWithResult(input);
		// we should get some results, not an exception
		assertEquals(4, result.getMessages().size());
	}

I am able to reproduce the same problem if I comment out the fix in this PR. The detailed error is shown below (build against master):

ca.uhn.fhir.rest.server.exceptions.InternalErrorException: Unexpected failure while validating resource
	at org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator.validate(FhirInstanceValidator.java:166)
	at org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator.validate(FhirInstanceValidator.java:204)
	at org.hl7.fhir.dstu3.hapi.validation.BaseValidatorBridge.doValidate(BaseValidatorBridge.java:24)
	at org.hl7.fhir.dstu3.hapi.validation.BaseValidatorBridge.validateResource(BaseValidatorBridge.java:52)
	at org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator.validateResource(FhirInstanceValidator.java:1)
	at ca.uhn.fhir.validation.FhirValidator.validateWithResult(FhirValidator.java:287)
	at org.hl7.fhir.dstu3.hapi.validation.ResourceValidatorDstu3Test.testValidateDifferentPropertyButSameStartsWithPath(ResourceValidatorDstu3Test.java:268)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: org.hl7.fhir.exceptions.DefinitionException: Unable to find class 'UsedUnsignedInt' for name 'benefitUsedUnsignedInt' on property EligibilityResponse.insurance.benefitBalance.financial.benefit[x]
	at org.hl7.fhir.dstu3.elementmodel.Property.getChildProperties(Property.java:245)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parseChildren(XmlParser.java:219)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parseChildren(XmlParser.java:281)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parseChildren(XmlParser.java:281)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parseChildren(XmlParser.java:281)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parseChildren(XmlParser.java:281)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parse(XmlParser.java:156)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parse(XmlParser.java:140)
	at org.hl7.fhir.dstu3.validation.InstanceValidator.validate(InstanceValidator.java:424)
	at org.hl7.fhir.dstu3.validation.InstanceValidator.validate(InstanceValidator.java:416)
	at org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator.validate(FhirInstanceValidator.java:164)
	... 30 more

Hello All,

The EligibilityResponse resource as of FHIR 1.8.0 (i.e. http://hl7.org/fhir/2017Jan/eligibilityresponse.html) still has this weirdness in the .benefit[x] and .benefitUsed[x] properties.

Using the following test below (in ResourceValidatorDstu3Test.java):

	@Test
	public void testValidateDifferentPropertyButSameStartsWithPath() throws Exception {

		EligibilityResponse fhirObj = new EligibilityResponse();
		BenefitComponent benComp = fhirObj.addInsurance().addBenefitBalance().addFinancial();
		// Test between .benefit[x] and benefitUsed[x]
		benComp.setBenefitUsed(new UnsignedIntType(2));
		
		String input = ourCtx.newXmlParser().encodeResourceToString(fhirObj);
		
		FhirValidator validator = ourCtx.newValidator();
		validator.registerValidatorModule(new FhirInstanceValidator());

		ValidationResult result = validator.validateWithResult(input);
		// we should get some results, not an exception
		assertEquals(4, result.getMessages().size());
	}

I am able to reproduce the same problem if I comment out the fix in this PR. The detailed error is shown below (build against master):

ca.uhn.fhir.rest.server.exceptions.InternalErrorException: Unexpected failure while validating resource
	at org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator.validate(FhirInstanceValidator.java:166)
	at org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator.validate(FhirInstanceValidator.java:204)
	at org.hl7.fhir.dstu3.hapi.validation.BaseValidatorBridge.doValidate(BaseValidatorBridge.java:24)
	at org.hl7.fhir.dstu3.hapi.validation.BaseValidatorBridge.validateResource(BaseValidatorBridge.java:52)
	at org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator.validateResource(FhirInstanceValidator.java:1)
	at ca.uhn.fhir.validation.FhirValidator.validateWithResult(FhirValidator.java:287)
	at org.hl7.fhir.dstu3.hapi.validation.ResourceValidatorDstu3Test.testValidateDifferentPropertyButSameStartsWithPath(ResourceValidatorDstu3Test.java:268)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: org.hl7.fhir.exceptions.DefinitionException: Unable to find class 'UsedUnsignedInt' for name 'benefitUsedUnsignedInt' on property EligibilityResponse.insurance.benefitBalance.financial.benefit[x]
	at org.hl7.fhir.dstu3.elementmodel.Property.getChildProperties(Property.java:245)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parseChildren(XmlParser.java:219)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parseChildren(XmlParser.java:281)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parseChildren(XmlParser.java:281)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parseChildren(XmlParser.java:281)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parseChildren(XmlParser.java:281)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parse(XmlParser.java:156)
	at org.hl7.fhir.dstu3.elementmodel.XmlParser.parse(XmlParser.java:140)
	at org.hl7.fhir.dstu3.validation.InstanceValidator.validate(InstanceValidator.java:424)
	at org.hl7.fhir.dstu3.validation.InstanceValidator.validate(InstanceValidator.java:416)
	at org.hl7.fhir.dstu3.hapi.validation.FhirInstanceValidator.validate(FhirInstanceValidator.java:164)
	... 30 more

jmandel pushed a commit to hl7-fhir/fhir-svn that referenced this pull request Dec 12, 2016

jamesagnew added a commit that referenced this pull request Dec 12, 2016

@jamesagnew

This comment has been minimized.

Show comment
Hide comment
@jamesagnew

jamesagnew Dec 12, 2016

Owner

Ok, test has been updated, and fix merged into upstream. Thanks!

Owner

jamesagnew commented Dec 12, 2016

Ok, test has been updated, and fix merged into upstream. Thanks!

@grahamegrieve

This comment has been minimized.

Show comment
Hide comment
@grahamegrieve

grahamegrieve Dec 12, 2016

Collaborator
Collaborator

grahamegrieve commented Dec 12, 2016

@jamesagnew

This comment has been minimized.

Show comment
Hide comment
@jamesagnew

jamesagnew Dec 12, 2016

Owner

@grahamegrieve Cool, I saw the discussion on Slack.

I've merged this patch into the RI anyhow.. I guess it doesn't help if the model is well formed, but it doesn't hurt either.

Owner

jamesagnew commented Dec 12, 2016

@grahamegrieve Cool, I saw the discussion on Slack.

I've merged this patch into the RI anyhow.. I guess it doesn't help if the model is well formed, but it doesn't hurt either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment