Skip to content
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

Subscribed catalogue support for Atom XML and importing from published URLs #309

Merged
merged 15 commits into from
Jun 13, 2022

Conversation

joe-crawford
Copy link
Contributor

@joe-crawford joe-crawford commented May 23, 2022

Add support for subscribing to Atom XML feeds as subscribed catalogues, and to the Mauro JSON format published in PR #309:

  • Add type field to Subscribed Catalogue (Mauro JSON or Atom) and for Atom feeds, store the whole URL to the Atom XML feed. Convert from JSON or Atom formats to published model list in the local catalogue. (mc-9833)
  • Replace Subscribed Model save method with federate. Add parameters (which are not saved) to optionally specify which URL, content type, and/or ImporterProviderService to use when federating the model. Check to ensure the specified URL is published in the Subscribed Catalogue. (mc-9834)
  • Combine getFileType and getProducesContentType methods in ExporterProviderService to getContentType. (mc-9835)
  • Split SubscribedCatalogueFunctionalSpec tests into base tests and AtomSubscribedCatalogueFunctionalSpec and MauroJsonSubscribedCatalogueFunctionalSpec tests. Also add Atom and Mauro JSON tests for relevant SubscribedModelFunctionalSpec tests.

@joe-crawford joe-crawford marked this pull request as draft May 23, 2022 21:53
@joe-crawford joe-crawford changed the title Feature/mc 9833 Subscribed catalogue support for Atom XML and importing from published URLs May 24, 2022
@joe-crawford
Copy link
Contributor Author

Failing test on branch and PR builds is not related.

@joe-crawford
Copy link
Contributor Author

joe-crawford commented May 24, 2022

Compatibility note:

  • The changes are backwards incompatible for subscription to and from older versions of Mauro subscribed catalogues (i.e. the new 'Mauro JSON' subscribed catalogue type is not compatible with the previous subscribed catalogues). The migration script does migrate previous subscribed catalogues if both remote and local instances are updated.
  • The changes to the content type fields in ExporterProviderService are backwards incompatible for plugins.

Notes on changes:

  • In SubscribedModelService, federateSubscribedModel supports federating an MdmDomain if certain properties are found dynamically. The intention is to allow federating a Model or a VersionedFolder, in particular so importers for FHIR JSON multi-item types (FHIR packages/bundles) can import a VersionedFolder and a 1:1 link between SubscribedModels and imported domains is maintained.
  • SubscribedModelFunctionalSpec tests check that this PR is present on the CD instance before running (by checking for SC /types endpoint).
  • Re: mc-9835 - grails.mime.types definitions not used as these affect client-side content type negotiation. URLs are imported as byte arrays and passed to importers.

@joe-crawford joe-crawford marked this pull request as ready for review May 24, 2022 08:48
@joe-crawford joe-crawford marked this pull request as draft May 24, 2022 11:25
@joe-crawford joe-crawford marked this pull request as ready for review May 25, 2022 09:44
@joe-crawford
Copy link
Contributor Author

joe-crawford commented May 25, 2022

Failing PR test unrelated again.

@joe-crawford joe-crawford requested a review from olliefreeman May 25, 2022 12:25
- Fetch bytes from URLs in FederationClient
- Update federation in SubscribedModelService
- Update Mauro MIME types to specify domain types
- Allow federating MdmDomains instead of Models to allow future importing of VersionedFolders
  as well as CatalogueItems.
- Add params to allow specifying which link to subscribe to and which importer service to use
- Replace save method with federate method
- Remove title field from PublishedModel as title from non-Mauro catalogues can't be parsed; use label instead
- Update Publish, SubscribedCatalogue, SubscribedModel and Feed functional tests
- Add Atom tests to SubscribedCatalogue and SubscribedModel functional tests
- Add tests for subscribed catalogues types endpoint
- Replace getFileType with getContentType using custom Mauro MIME types e.g. application/mauro.datamodel+json
- Also replace getProducesContentType with getContentType
…cence headers

- Update SubscribedModelFunctionalSpec tests to connect to the Continuous Deployment instance, requiring a recent version
- Add missing licence headers
- Fix typos in DataModelFunctionalSpec, ImporterProviderServiceData
- Add test for optional federation params in SubscribedModelFunctionalSpec
- Update SubscribedModelFunctionalSpec IDs to match continuous deployment server
- Replace application/mdm+json with application/mauro.<modeltype>+json
- Remove fileType from DomainExportFunctionalSpec JSON - replaced with contentType
@joe-crawford joe-crawford marked this pull request as draft June 7, 2022 16:11
@joe-crawford
Copy link
Contributor Author

Failing test is passing locally and looks unrelated.

@joe-crawford joe-crawford marked this pull request as ready for review June 8, 2022 08:37
Copy link
Contributor

@olliefreeman olliefreeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest adding a public static final CONTENT_TYPE = '' to each exporter service, and then having the getContentType() method returning this variable, then in the importer services you have the handlesContentType check against that variable, that way if we want to change the content type we only have to do it in 1 place and it applies across all things that use it. It also means if we have a importer which handles multiple types they can check a list of psf variables

See the other comments and questions

@joe-crawford joe-crawford marked this pull request as draft June 10, 2022 10:02
- Add ImporterProviderService.CONTENT_TYPE static strings
- Revert XmlImportMapping changes
- Change SubscribedCatalogueConverter interface to trait
- Use command object validation in SubscribedModelController::federate
- Add dynamic check on type of imported domain in SubscribedModelService::federateSubscribedModel

Also:

- Add migration for replacing DomainExport exportFileType with exportContentType
@joe-crawford joe-crawford marked this pull request as ready for review June 10, 2022 11:55
Copy link
Contributor

@olliefreeman olliefreeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just the one question about the validation

@olliefreeman olliefreeman merged commit ff5dc5a into develop Jun 13, 2022
@olliefreeman olliefreeman deleted the feature/mc-9833 branch June 13, 2022 08:14
@joe-crawford joe-crawford added this to the 5.2.0 milestone Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Released
Development

Successfully merging this pull request may close these issues.

2 participants