-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
XML Attributes instead of Elements #126
Comments
Ah, it should be identical to the source. I now track whether elements with value primitives and primitives were seen as attributes or elements. You can also specify this location at set time: https://github.com/dcarbone/php-fhir-generated/blob/1623dc7d7c00a0cfd84f23b555e0d9b1cd0a368b/src/DCarbone/PHPFHIRGenerated/R4/FHIRElement/FHIRIdentifier.php#L361 If you can provide input xml that causes erroneous output, I can add it as a test case. This is done as I have seen both structure formats, particularly if the element has extensions. |
I created a small test that produces the following incorrect xml <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<Identifier system="http://example.com/identifier-system" value="example"/> Small test with an assertion of what the output should be. public function testXmlAttributes()
{
$identifier = new FHIRIdentifier([
'value' => 'example',
'system' => 'http://example.com/identifier-system'
]);
$xml = $identifier->xmlSerialize()->outputMemory();
$this->assertEquals(
'<Identifier>
<system value="http://example.com/identifier-system"/>
<value value="example"/>
</Identifier>',
$xml
);
} |
Ah, so that is effectively converting JSON to XML, and at that point I cannot predetermine which is the desired positioning. I have thought about adding an xml position map parameter as a constructor arg for such occasions, or perhaps allowing these to be set globally in the config. Thoughts? |
I am not sure if I understand your question, I am not converting JSON to XML. I use the library to create a model and serialize that to XML. Or do you mean the part where I use the constructor? When I change it to this I get the same output. $identifier = new FHIRIdentifier();
$identifier->setValue("example");
$identifier->setSystem("http://example.com/identifier-system"); It looks like the XML serialization does not follow the FHIR standard. |
@leonrenkema Is this still an issue for you? I've released several updates with lots of various improvements. Additionally, the tests I've written specifically assert that the generated code is able to produce output identical to the input, so I'm not quite sure what else I can do here... |
I tried again with the latest master checkout but I see the same behaviour. What happens when you add the following test to your tests or something similiar? $identifier = new FHIRIdentifier();
$identifier->setSystem("http://example.com/identifier-system");
$identifier->setValue("example");
$xml = $identifier->xmlSerialize()->outputMemory();
$this->assertEquals(
'<Identifier>
<system value="http://example.com/identifier-system"/>
<value value="example"/>
</Identifier>',
$xml
); Now it generated |
So, as mentioned if you want the xml serialization to appear in a particular location, you need to use the location enum: https://github.com/dcarbone/php-fhir-generated/blob/92535a6a7adb192288a21f5f4b4f6baab8e9b946/src/DCarbone/PHPFHIRGenerated/R4/FHIRElement/FHIRIdentifier.php#L355 This is because different sources have different serialization standards and I cannot enforce one without breaking it for another. |
Ah, okay. But it is a whole load of work to add this to all the places where I call such methods. I always thought that the FHIR specification defines if it should be an element or an attribute but maybe it works different. |
It probably does, but I have seen both in the wild during the execution of my tests. As you saw in v2, my solution was to just put the values in both places with the intent that receivers of this can just use whichever they were fixed to use. I do agree that the current implementation is limited. I think a solution could be to define a new config field that lets you globally override the default. Thoughts? |
The only exception I can find in an bundle we are using is the url attribute on an extension. There are a few but not much. Can you not determine the correct default from the XSD you are parsing? In the StructureDefinition it is coded like See https://www.hl7.org/fhir/elementdefinition-definitions.html#ElementDefinition.representation |
No, the XSD's do not specify placement in XML serialization. I could add a way to configure the default when using the generator yourself. I also was not aware if that ElementDefinition element. I can look into that, but I won't have time in the near future for larger updates. |
@leonrenkema I have a PR to quickly enable you to configure the default serialization location for Elements: #142 Would something like this work for you? |
I have tried it and I am afraid that it does not solve the problem. With the default to attribute it places every primitivevalue on it's parent element I guess? Also somewhere the fri and 09:00:00 are lost. <?xml version="1.0" encoding="UTF-8" standalone="yes"?>\n
<Dosage sequence="1">
<timing>
<repeat dayOfWeek="mon" timeOfDay="08:00:00"/>
</timing>
</Dosage> With the default set to element the generated XML looks like this <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<Dosage>
<sequence>
<value value="1" />
</sequence>
<timing>
<repeat>
<dayOfWeek>
<value value="mon" />
</dayOfWeek>
<dayOfWeek>
<value value="fri" />
</dayOfWeek>
<timeOfDay>
<value value="08:00:00" />
</timeOfDay>
<timeOfDay>
<value value="09:00:00" />
</timeOfDay>
</repeat>
</timing>
</Dosage> With the version I now use it generates this and that is valid FHIR. <Dosage>
<sequence value="1"/>
<timing>
<repeat>
<dayOfWeek value="mon"/>
<dayOfWeek value="fri"/>
<timeOfDay value="08:00:00"/>
<timeOfDay value="09:00:00"/>
</repeat>
</timing>
</Dosage> Maybe it is more complicated than a simple default. |
Excellent, this is a great example of the issue. I believe this is two separate issues:
|
@leonrenkema I have closed the other PR in favor of #143. Using your provided example, I made this quick little test script: <?php declare(strict_types=1);
require __DIR__ . '/../output/HL7/FHIR/R4/PHPFHIRAutoloader.php';
use HL7\FHIR\R4\FHIRElement\FHIRBackboneElement\FHIRDosage;
use HL7\FHIR\R4\FHIRElement\FHIRBackboneElement\FHIRTiming;
use HL7\FHIR\R4\FHIRElement\FHIRBackboneElement\FHIRTiming\FHIRTimingRepeat;
use HL7\FHIR\R4\PHPFHIRAutoloader;
use HL7\FHIR\R4\PHPFHIRXmlWriter;
PHPFHIRAutoloader::register();
$dosage = new FHIRDosage([
FHIRDosage::FIELD_SEQUENCE => 1,
FHIRDosage::FIELD_TIMING => [
FHIRTiming::FIELD_REPEAT => [
FHIRTimingRepeat::FIELD_DAY_OF_WEEK => [
'mon',
'fri',
],
FHIRTimingRepeat::FIELD_TIME_OF_DAY => [
'08:00:00',
'09:00:00',
],
]
]
]);
$xw = new PHPFHIRXmlWriter();
$xw->openMemory();
$xw->setIndent(true);
$xw->setIndentString(' ');
$dosage->xmlSerialize($xw);
echo $xw->outputMemory(true); Running this script produces the output: <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<Dosage>
<sequence value="1"/>
<timing>
<repeat>
<dayOfWeek value="mon"/>
<dayOfWeek value="fri"/>
<timeOfDay value="08:00:00"/>
<timeOfDay value="09:00:00"/>
</repeat>
</timing>
</Dosage> Hopefully this will resolve this issue for you! If so, please let me know and I will cut a release asap. |
Hi, I tested the new implementation by generating the code based on R4. But I am seeing some strange generated XML.
For example the identifier element has the system and value as an attribute and not as a seperate element (see below for the generated code with the old generator). I don't think
<identifier system="http://fhir.nl/fhir/NamingSystem/aorta-app-id" value="90000274" />
is valid FHIR Xml.The text was updated successfully, but these errors were encountered: