mCSD Changes plus more #122

lukeaduncan · 2018-01-31T13:39:33Z

This was the changes I did to add in mCSD and mACM support which are built in STU3. Also added in FHIR.js for XML conversions.

…not supported as it returns data in the metadata collection by default, although that probably needs to be corrected. Allowed upsert with PUT.

…L to update. This needs to be modified to store some configuration about which resources to load on a regular basis. Perhaps also saving them in a separate databse or maybe that's better handled by having a separate server with a different DB configuration.

…richton.

…). Made the update supplier api call more robust depending on what is returned to fail more gracefully. Updated some search queries.

… the ATNA message in searchFilters when it is chained. Added DELETE message to history when a resource is deleted. Fixed history message to display messages from the history collection correctly on the _history action.

rcrichton

@lduncan-intrah Thanks! This looks very good to me. I've added comments to the PR, but mostly what we need for this PR is to:

Add some tests for the new functionality
Update the minor styling issues so the linter (standard.js) doesn't complain and the build passes on travis
A few updates to make the code work with our latest commits. Merging in our new master to this branch would be needed.

Let us know if you can focus on these things and if we can help at all.

rcrichton · 2018-02-08T13:59:32Z

lib/fhir/common.js

@@ -360,9 +360,16 @@ module.exports = exports = (mongo) => {
          if (entry._mpi) {
            search = entry._mpi.search
          }
+        if ( entry._request.method == 'POST' ) {


Minor: we should always use triple equals ===

rcrichton · 2018-02-08T14:00:35Z

lib/fhir/common.js

@@ -543,7 +550,7 @@ module.exports = exports = (mongo) => {
     */
    getPagingParams: (queryParams, callback) => {
      let _getpagesoffset = 0
-      let _count = 10
+      let _count = 100


Revert this to the default of 10. It can always be overridden by the client.

rcrichton · 2018-02-08T14:09:16Z

lib/fhir/core.js

@@ -430,7 +566,7 @@ module.exports = (mongo, modules) => {
            resource.meta.versionId = fhirCommon.util.generateID()

            const options = {
-              upsert: false, // TODO OHIE-168
+              upsert: true, // TODO OHIE-168


We should make this configurable via a config option with a default of false.

rcrichton · 2018-02-08T14:12:18Z

lib/fhir/core.js

@@ -459,7 +585,26 @@ module.exports = (mongo, modules) => {
                  const location = `/fhir/${resourceType}/${id}/_history/${resource.meta.versionId}`
                  callback(null, { httpStatus: 200, location: location, id: id })
                })
-              })
+                //return callback(null, fhirCommon.buildHTTPOutcome(404, 'information', 'not-found', 'Not found'))


Don't we still need this instead of the callback at line 586?

rcrichton · 2018-02-08T14:22:33Z

lib/fhir/resources/location.js

+
+        if (queryObject['organization.active']) {
+          promises.push(new Promise((resolve, reject) => {
+            queryUtils.genChainedParamQuery(queryObject['organization.active'][constants.NO_MODIFER], 'managingOrganization.reference', 'active', organization, (err, clause) => {


In one of our recent commits we changes how fhirModules are passed to this function. All uses of this will need minor adaptation.

rcrichton · 2018-02-09T07:00:25Z

lib/server.js

@@ -178,7 +185,11 @@ app.get('/fhir/:resourceType', (req, res, next) => {
  fhirCore.search(res.locals.ctx, req.params.resourceType, buildCoreCallbackHandler(req, res, next))
 })
 app.get('/fhir/:resourceType/:id', (req, res, next) => {
-  fhirCore.read(res.locals.ctx, req.params.resourceType, req.params.id, buildCoreCallbackHandler(req, res, next))
+  if ( req.params.id == '_history' ) {
+    fhirCore.history(res.locals.ctx, req.params.resourceType, buildCoreCallbackHandler(req, res, next))


We need to add tests to verify that this is working as expected.

rcrichton · 2018-02-09T07:00:57Z

lib/server.js

@@ -220,7 +232,18 @@ const outcomeHandler = (req, res, next) => {
    res.set('Location', res.locals.outcome.location)
  }
  if (res.locals.outcome.resource) {
-    res.status(res.locals.outcome.httpStatus).send(res.locals.outcome.resource)
+    var output = res.locals.outcome.resource;


This can be removed to as it is handled in out latest commits.

rcrichton · 2018-02-09T07:03:27Z

lib/fhir/core.js

+              { $lookup: { from: resourceType+"_history", pipeline: [], as: "history" } },
+              { $project: { all: { $setUnion: [ "$top", "$history" ] } } },
+              { $unwind: "$all" }, 
+              { $replaceRoot: { newRoot: "$all" } }


Perhaps another way to do this would be to just to two separate queries and combine the results? Might be more efficient. Although this way seems fine to until we actually see a performance issue.

rcrichton · 2018-02-09T07:05:39Z

lib/fhir/query-utils.js

@@ -237,7 +244,119 @@ module.exports = (mongo) => {
      }
      return addToClause(value)
    },
+*/
+
+    nameToMongoClause: (fieldToMatch, value, modifier) => {


We normally add test for each new resource we add and all search parameters for this. But in this case that is a lot of work. So, I'd suggest only writing tests for the search parameters were we use these new query utils functions so that we get coverage of these functions and ensure they do what we expect.

rcrichton · 2018-02-09T07:07:33Z

lib/fhir/resources/healthcare-service.js

+
+        if (queryObject['organization.active']) {
+          promises.push(new Promise((resolve, reject) => {
+            queryUtils.genChainedParamQuery(queryObject['organization.active'][constants.NO_MODIFER], 'organization.reference', 'active', organization, (err, clause) => {


We have changed the way genChainedParamQuery works. It not accepts an array of fhirModules in the latest commits. We believe we will need to change how we are passing the fhir module to this function in each case where it's used in this PR. It shouldn't be too much different.

BMartinos

Thanks @lduncan-intrah this is looking great so far, Just added a few comments but overall looking good,

The main points of concern i have is that a lot of the resource updates have been changed to support FHIR DSTU3. At the moment hearth only supports DSTU2. Adding new resources, or additional supported parameters for a resource is fine i think, its more the ones that are changed and remove that might cause some issues.

We still need to come up with a nice way to support all FHIR versions, and any new ones that might come in future.

BMartinos · 2018-02-08T13:56:55Z

lib/fhir/root.js

+        if ( myURL.search ) {
+            path = path+myURL.search
+        }
+        console.log( myURL.hostname +' '+myURL.port+' '+path+' '+myURL.protocol);


remove console.log()

BMartinos · 2018-02-08T13:57:10Z

lib/fhir/root.js

+     * @param {CoreCallback} callback
+     */
+    pullUpdates: (ctx, callback) => {
+        console.log(ctx.query.url);


remove console.log()

BMartinos · 2018-02-08T14:01:49Z

lib/fhir/common.js

@@ -360,9 +360,16 @@ module.exports = exports = (mongo) => {
          if (entry._mpi) {
            search = entry._mpi.search
          }
+        if ( entry._request.method == 'POST' ) {


Check indentation
remove spaces within condition braces
update condition to be ===

BMartinos · 2018-02-08T14:03:31Z

lib/fhir/common.js

+        if ( entry._request.method == 'POST' ) {
+            entry._request.url = entry.resourceType
+        } else {
+            entry._request.url = entry.resourceType+"/"+entry.id


Update variable to be defined in an template Literal way, making it easier to read
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals

BMartinos · 2018-02-08T14:07:24Z

lib/fhir/common.js

@@ -543,7 +550,7 @@ module.exports = exports = (mongo) => {
     */
    getPagingParams: (queryParams, callback) => {
      let _getpagesoffset = 0
-      let _count = 10
+      let _count = 100


This needs to be reverted to the default of 10, A query parameter (_count) can be sent to increase the paging count, If this is set to _count=0 then all results will be returned

BMartinos · 2018-02-09T07:45:02Z

lib/plugins/z_convert.js

@@ -0,0 +1,112 @@
+ /**


I think this module can be removed due to Hearth having integrated content negotiation to transform the payload content type, Payloads can now be returned in XML or JSON based on the Accepts and Content-Type headers

BMartinos · 2018-02-09T07:47:02Z

lib/server.js

@@ -66,7 +67,8 @@ const fhirRoot = require('./fhir/root')(mongo, fhirResources)
 // Setup express
 let app = express()

-app.use(bodyParser.json({limit: '10Mb', type: ['application/json+fhir', 'application/json']}))
+app.use(bodyParser.json({limit: '10Mb', type: ['application/fhir+json', 'application/json+fhir', 'application/json']}))
+app.use(bodyParser.text({limit: '10Mb', type: ['application/fhir+xml', 'application/xml+fhir', 'application/xml']}))


This can be removed, as XML types are now handled with the new content negotiation changes that were added to Hearth

BMartinos · 2018-02-09T07:49:29Z

lib/server.js

@@ -159,6 +162,10 @@ const buildCoreCallbackHandler = (req, res, next) => {
  }
 }

+app.get('/api/supplyUpdates', (req, res, next) => {


Im not sure if this should be included directly into Hearth, and a better fit might be to have this endpoint as a separate microservice sitting next to hearth. If this is quite dependant on some of the hearth core functions, we might need to find a better way for custom endpoints

BMartinos · 2018-02-09T07:50:13Z

lib/server.js

@@ -198,7 +209,8 @@ app.delete('/fhir/:resourceType/:id', (req, res, next) => {

 const returnContentTypeHandler = (req, res, next) => {
  if (res.locals.outcome && res.locals.outcome.resource) {
-    const accepts = req.accepts(['application/json+fhir', 'application/json'])
+    const accepts = req.accepts(['application/fhir+json', 'application/json+fhir', 'application/json', 'json', 
+            'application/fhir+xml', 'application/xml+fhir', 'application/xml', 'xml'])


This can be reverted due to content negotiation being added to hearth

BMartinos · 2018-02-09T07:50:31Z

lib/server.js

+      var fhir = new Fhir(Fhir.STU3);
+      output = fhir.ObjectToXml( output );
+    } 
+    res.status(res.locals.outcome.httpStatus).send(output)


The above changes can be reverted due to content negotiation being added to hearth

citizenrich · 2018-04-30T20:16:24Z

@rcrichton @lduncan-intrah Wanted to see if we are stuck on something here. I don't see commits to the master branch since mid-Feb.

mCSD to CSD translator

…he context to be passed along. e.g. /DBNAME/fhir/RESOURCE. Not including DBNAME will use the default database.

…he context to be passed along. e.g. /DBNAME/fhir/RESOURCE. Not including DBNAME will use the default database. This needs more complete testing and the worker thread isn't set up to attach to multiple databases since that is started at server time and not connected to the database name. Further updates will be needed to configure workers per database.

…exists.

…/grandchildren/etc. of a Location based on the partOf.reference. /fhir/Location/ID/$hierarchy. It returns a Bundle of type "searchset".

…he parents/grandparents/etc of a Location.

…view so the connectToField will always use a direct field instead of a modified field in the view for faster access.

…on so multiple databases are supported.

…view and graphLookup for this, it was changed to pulling all the data and handling it inside Hearth to filter the results. graphLookup was running out of memory for large datasets and even simplifying that for the current data sets still took a long time to run. This option is currently much faster but may need to be reviewed for even larger datasets.

lukeaduncan added 14 commits January 15, 2018 13:18

Initial support for mCSD.

7f384c4

Finished up adding in required queries for mCSD. Removed conformance …

79719e0

…not supported as it returns data in the metadata collection by default, although that probably needs to be corrected. Allowed upsert with PUT.

Removed debugging statement.

e2fdfde

Attempted change to get chained parameters to reference self.

40abee8

Fixed self referential chained parameters with assistance from Ryan C…

d30861f

…richton.

Added in basic ATNA support for mCSD queries (for Practitioner so far…

cf47157

…). Made the update supplier api call more robust depending on what is returned to fail more gracefully. Updated some search queries.

Added in communication and communicationrequest resources.

e02244d

Added in simple echo plugin for mACM for communication request.

c780475

Fixed some content type handling as well as with _format.

57c94af

Fixed typo in conversion method.

0b77116

Fixed username in atna for mCSD and fixed typo in convert plugin.

bceb357

Fixed history query to use the correct resource type.

9f6ec56

rcrichton requested changes Feb 9, 2018

View reviewed changes

BMartinos suggested changes Feb 9, 2018

View reviewed changes

ashaban added 3 commits April 9, 2018 06:40

Adding mCSD to CSD translator as a history

fd18fe1

removing unused module

e7917ad

Minor changes to variable name

4aec672

lukeaduncan added 10 commits May 11, 2018 13:45

Merge pull request #1 from ashaban/master

01b9be6

mCSD to CSD translator

Fixed typo in location partOf query.

f2f7caa

Updated server to allow multiple databases by URL which is saved in t…

a1b0372

…he context to be passed along. e.g. /DBNAME/fhir/RESOURCE. Not including DBNAME will use the default database.

Merge branch 'master' of github.com:intrahealth/hearth

1ec565b

Fixed relation links to include the database name in the URL when it …

1f7b149

…exists.

Added in $hierarchy operation for Location to return all the children…

e132c3d

…/grandchildren/etc. of a Location based on the partOf.reference. /fhir/Location/ID/$hierarchy. It returns a Bundle of type "searchset".

Added in operation to do the reverse order of . (i.e. to return all t…

6638aeb

…he parents/grandparents/etc of a Location.

Updated and operations to also return the requested/starting Location.

c5c8eb3

Updated hierarchy/parent operation to add an additional field to the …

cf86abb

…view so the connectToField will always use a direct field instead of a modified field in the view for faster access.

lukeaduncan and others added 7 commits June 25, 2018 14:40

Updated matching queue to allow a list of databses to create workers …

cd29894

…on so multiple databases are supported.

Added in the allowDiskUse option for the hiearchy aggregation.

6c513b8

Added _tag to Location for queries.

df9a54f

Updated dependencies.

075e13f

Further dependency updates.

7765333

push to dockerhub script

2a9a8d1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mCSD Changes plus more #122

mCSD Changes plus more #122

lukeaduncan commented Jan 31, 2018

rcrichton left a comment

rcrichton Feb 8, 2018

rcrichton Feb 8, 2018

rcrichton Feb 8, 2018

rcrichton Feb 8, 2018

rcrichton Feb 8, 2018

rcrichton Feb 9, 2018

rcrichton Feb 9, 2018

rcrichton Feb 9, 2018

rcrichton Feb 9, 2018

rcrichton Feb 9, 2018

BMartinos left a comment

BMartinos Feb 8, 2018

BMartinos Feb 8, 2018

BMartinos Feb 8, 2018

BMartinos Feb 8, 2018

BMartinos Feb 8, 2018

BMartinos Feb 9, 2018

BMartinos Feb 9, 2018

BMartinos Feb 9, 2018

BMartinos Feb 9, 2018

BMartinos Feb 9, 2018

citizenrich commented Apr 30, 2018

mCSD Changes plus more #122

Are you sure you want to change the base?

mCSD Changes plus more #122

Conversation

lukeaduncan commented Jan 31, 2018

rcrichton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BMartinos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

citizenrich commented Apr 30, 2018