New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[(master)] Added support for extensions in Meta resource #776

Merged
merged 19 commits into from Mar 12, 2018

Conversation

Projects
None yet
3 participants
@sjanic
Contributor

sjanic commented Nov 1, 2017

A fix for issue: #772

Known limitations:

  • If you add an extension to a resource's Meta, you have to make the url used as key unique. When we deserialize the meta resource back we use the extension url as key, and duplicate keys will be ignored.

  • In my own organisation we only use the Json part of Hapi Fhir, so I haven't added Xml support given other priorities. If I have the time, or somebody else wants to, feel free to add it.

- Subextensions are not supported in extensions that are located in Meta.

@sjanic

This comment has been minimized.

Contributor

sjanic commented Nov 1, 2017

Hmm... It seemed when I cloned the branch and made my changes, the Devicestatus and DevicestatusEnumFactory classes got changed. I didn't intent to do that, as my fix doesn't involve these two files, so I assume the files got auto generated when running mvn install.

I have reverted the files, but it seems that the class names differs from the file names. Class Devicestatus has the file name DeviceStatus.java and DevicestatusEnumFactory has file name DeviceStatusEnumFactory. According to Java file name and class name should be the same.
@jamesagnew Is this as expected?

@sjanic

This comment has been minimized.

Contributor

sjanic commented Nov 1, 2017

Can see that build server gave me the following error in the tests:

[ERROR] Tests run: 5, Failures: 0, Errors: 1, Skipped: 1, Time elapsed: 11.373 s <<< FAILURE! - in ca.uhn.fhir.jpa.subscription.RestHookTestDstu3Test
[ERROR] testRestHookSubscriptionApplicationXml(ca.uhn.fhir.jpa.subscription.RestHookTestDstu3Test)  Time elapsed: 4.164 s  <<< ERROR!
ca.uhn.fhir.rest.server.exceptions.InternalErrorException: HTTP 500 Server Error: Failed to call access method
	at ca.uhn.fhir.jpa.subscription.RestHookTestDstu3Test.testRestHookSubscriptionApplicationXml(RestHookTestDstu3Test.java:238)

I can see that it is a server error, but I wasn't able to reproduce it on my local machine. It is possible to get some log files from the build servers?

@sjanic sjanic closed this Nov 2, 2017

@sjanic sjanic reopened this Nov 2, 2017

@coveralls

This comment has been minimized.

coveralls commented Nov 2, 2017

Coverage Status

Coverage decreased (-0.1%) to 81.823% when pulling 11dc917 on sjanic:master into dea9cdd on jamesagnew:master.

@coveralls

This comment has been minimized.

coveralls commented Nov 21, 2017

Coverage Status

Coverage decreased (-0.09%) to 72.476% when pulling 97a1cca on sjanic:master into 2e4f80d on jamesagnew:master.

@sjanic

This comment has been minimized.

Contributor

sjanic commented Nov 25, 2017

I have looked at the changes again, and it seems that extensions on meta can reuse some of the logic that is used for encoding extensions on resources.
It allows me to remove the limitation of not having sub-extensions in meta.

I need to clean up the code, and I'll post it to my fork when that is done.

sjanic and others added some commits Nov 26, 2017

Refactored the meta extensions, so it reuses the existing logic that is
used for encoding and parsing extensions on resources.

@jamesagnew jamesagnew merged commit ad69625 into jamesagnew:master Mar 12, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

jamesagnew added a commit that referenced this pull request Mar 12, 2018

jamesagnew added a commit that referenced this pull request Mar 12, 2018

@jamesagnew

This comment has been minimized.

Owner

jamesagnew commented Mar 12, 2018

Thanks for the PR! It has been merged. Please send me a note if you'd like your real name to appear in the contributors section of our website!

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