3764 WIP Fix for #3764 #3896
3764 WIP Fix for #3764 #3896
Conversation
| @@ -94,7 +94,7 @@ public static void main(String[] args) throws Exception { | |||
|
|
|||
| ValidationSupportChain validationSupportChain = new ValidationSupportChain(); | |||
| validationSupportChain.addValidationSupport((ca.uhn.fhir.context.support.IValidationSupport) new DefaultProfileValidationSupport(ctx)); | |||
| validationSupportChain.addValidationSupport((ca.uhn.fhir.context.support.IValidationSupport) new PrePopulatedValidationSupport(ctx, structureDefinitions, valueSets, codeSystems)); | |||
There was a problem hiding this comment.
Recommend leaving the old constructor in place (the one without the final parameter) but also creating the new constructor, and having the old one just chain to the new one. That way we don't break any existing users' code.
| import static ca.uhn.fhir.util.ClasspathUtil.loadResource; | ||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
|
||
| public class ParserWithValidationR4Test { |
There was a problem hiding this comment.
A couple of comments on this test:
- The naming for this class and the test method don't seem to align with what's actually being tested. Maybe
XverValidationR4Test? - We should include a negative test as well - E.g. same resource being validated but instead of
"valueUri" : "https://r2.smarthealthit.org"it ias"valueInteger": 123and presumably we'd get a validation error because the value is of the wrong datatype? - It'd be nice to include an example on this page that shows how to validate with xver refs. I think the code snippet would basically by the same logic as this test.
| @@ -258,7 +258,8 @@ public List<StructureDefinition> getStructures() { | |||
|
|
|||
| @Override | |||
| public void cacheResource(Resource res) { | |||
| throw new UnsupportedOperationException(Msg.code(660)); | |||
| //FIXME Should we implement anything here? | |||
There was a problem hiding this comment.
@jamesagnew I removed the UnsupportedOperationException here because this chunk of code in the core library was triggering it:
I don't like removing this exception. I think the appropriate thing to do would be to have IWorkerContext include a new method like implementsCacheResource, which is set to false in our implementation, which would prevent cacheResource from being called.
There was a problem hiding this comment.
@dotasek definitely no concerns with making that method a no-op.
WORK IN PROGRESS
This is the bare minimum of code required for our test case to pass, and to allow a review of the solution itself.