lantanagroup / FHIR.js

Node.JS library for serializing/deserializing FHIR resources between JS/JSON and XML using various node.js XML libraries
Apache License 2.0
102 stars 29 forks source link

Error when loading supplemental FHIR Profiles #27

Closed PSUPing closed 4 years ago

PSUPing commented 4 years ago

Scenario

I've been working with the Davinci Data Exchange for Quality Measure (DEQM) profile. As part of the profile there are a few resources where the base profile is being extended. This is done for a few different reasons: adding relevant extensions, requiring things that are optional in the base FHIR spec, etc.

Ideally, what I'd like to be able to do is use the base FHIR profile and "overlay" the changes for the DEQM spec. I tried the following code:

import { Fhir, Versions } from 'fhir/fhir'
import { ParseConformance } from 'fhir/parseConformance'
import * as fs from 'fs'
import * as deqmPractionerExample from '/mnt/c/Users/Matt/Downloads/package/example/Organization-organization01.json'

let deqmOrg = JSON.parse(fs.readFileSync('./deqm/StructureDefinition-organization-deqm.json').toString())

deqmParser.parseStructureDefinition(deqmOrg)

let fhirValid = new Fhir(deqmParser)
let resp = fhirValid.validate(deqmPractionerExample)

This the StructureDefinition from the DEQM spec that I'm using in the above example. StructureDefinition-organization-deqm.txt

When doing this I get the following error:

/mnt/c/Users/Matt/projects/deqm-test/node_modules/fhir/parseConformance.js:350
                    parentBackboneElement._properties.push(newProperty);
                                                      ^

TypeError: Cannot read property 'push' of undefined
    at ParseConformance.populateBackboneElement (/mnt/c/Users/Matt/projects/deqm-test/node_modules/fhir/parseConformance.js:350:55)
    at ParseConformance.parseStructureDefinition (/mnt/c/Users/Matt/projects/deqm-test/node_modules/fhir/parseConformance.js:152:30)
    at Object.<anonymous> (/mnt/c/Users/Matt/projects/deqm-test/deqm.js:34:12)
    at Module._compile (internal/modules/cjs/loader.js:1138:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1158:10)
    at Module.load (internal/modules/cjs/loader.js:986:32)
    at Function.Module._load (internal/modules/cjs/loader.js:879:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
    at internal/main/run_main_module.js:17:47

After looking through the parser, I traced it back to this specific block in populateBackboneElement (around line 349 in parseConformance.js).

                else if (backboneElement.type.length == 1) {
                    const newProperty = {
                        _name: backboneElementId.substring(backboneElementId.lastIndexOf('.') + 1),
                        _type: backboneElement.type[0].code,
                        _multiple: backboneElement.max !== '1',
                        _required: backboneElement.min === 1,
                        _properties: []
                    };
                    parentBackboneElement._properties.push(newProperty);
                    this.populateValueSet(backboneElement, newProperty);
                    if (backboneElement.type[0].code === 'BackboneElement' || backboneElement.type[0].code == 'Element') {
                        this.populateBackboneElement(resourceType, profile.snapshot.element[y].id, profile);
                    }
                }

It looks like the parentBackboneElement._properties.push(newProperty); is throwing the error because previously if the element is defined without a _properties it's an undefined object and, therefore, cannot push to it. Since I'm trying to in effect "modify" the resource after it's already been created, I believe this is where the error is coming from.

Possible Fixes

There's three possible fixes that I'm thinking of for this.

  1. I'm doing something wrong or thinking about the framework the wrong way. Totally happy to learn what I'm doing wrong.
  2. It would be good to add this block of code just above the line. That would handle the scenario where the object is being "modified" by making sure properties is an array.
    if (parentBackboneElement._properties === undefined || parentBackboneElement._properties === null) { 
     parentBackboneElement._properties = [];
    }
  3. There should likely be a check in that space regardless to throw a more proper error.

For 2 and 3, I'm happy to submit a PR. I wanted to open an issue and discuss first to make sure I wasn't missing anything and this made sense for the library.

seanmcilvenna commented 4 years ago

I'd be happy to entertain a PR for #2 and #3 above. Those are probably good things to do either way.

With that said, right now the validate() functionality only validates against the core spec. Long term, it should be extended to support validating against one or more specific profiles (and maybe also validate against whatever profile(s) is listed in the Resource.meta.profile property). If the profile is not found in the list of parsed profiles, it should probably ask for the profile via an event, and fail with a validation error message if the profile ultimately isn't found. This is something I've been wanting to dig into for a while, but I'm not sure when I'll be able to get to it. Very busy right now.

PSUPing commented 4 years ago

Loading profiles sounds like an interesting addition. Especially with the various standards building on top of FHIR, that could be very useful.

seanmcilvenna commented 4 years ago

I merged your change, and created an enhancement ticket to be addressed later. I believe your immediate problems is solved, at least, so I'm closing the ticket. Let me know if I'm wrong.

PSUPing commented 4 years ago

👍