Resource properties should be private?

[shehi]

Hey @dcarbone ,

I have a question regarding a so called resource classes, e.g. FHIRDomainResource which introduce their own set of properties. Patient and similar classes extend from that class, but of course the properties coming from parent FHIRDomainResource have nothing to do with Patient (at least as far as I can see in FHIR docs). Properties like $contained, or $extension. Having those properties as non-private pollutes Doctrine ORM/ODM assets and introduces them as if they are part of sub-classes (Patient etc), which they aren't.

So, question is: Does it make sense to have these resource class properties as private? What are your thoughts on this topic? Currently my primary interest is FHIRDomainResource class. There may exist other similar classes in your package I may be unaware of.

And considering those set of properties have their setter/getter pairs anyway, my belief is: having them as private shouldn't hurt anything. Actually having these setters/getters also hurt a lot. So another question is: actually, what is the purpose of these properties, or even those classes in first place? Would it make sense to create totally unrelated package solely dedicated to creating Doctrine ODM/ORM resources?

Thanks.

[dcarbone]

They’re there as the child classes extend them as part of the FHIR spec. I do not arbitrarily do this; if you have a look at the fhir definition xsd’s, you’ll see what i mean. The generator is a blank slate, it makes no assumptions as to hierarchy, and only defines a class as a child if the xsd defines it as having a base class.

I am away from a computer at the moment, but I will link an example once i am.

[dcarbone]

Additionally, every single type in the FHIR spec with the exception of primitives can be overloaded using extensions. This is true all the way down to simple types, such as FHIRString and etc.

Many places define their own profiles that extend the base types. This is accomplished, according to spec, via adding one or more extension entries to the type they are extending.

[shehi]

Yea, please gimme some links to make heads and tails out of this thing. As far as I can see XSDs, e.g. http://hl7.org/fhir/patient.xsd , I still fail to see mention of Domain resource anywhere. At the end of the day, I need to implement resources like Patient or Practitioner the way their content are described in Docs, by just seeing this picture:

I see what you did only when I view Schematron/Json schema of the resource, e.g. http://hl7.org/fhir/practitioner.sch or http://hl7.org/fhir/practitioner.schema.json.html . And to be honest, it adds to complexity and confusion. At the end of the day, I still am interested only in the realization of resource content, not any of the accompanying Profiles or Extensions. At least not right away.

Any suggestions to solve this whole mess in my mind would be greatly appreciated.

[dcarbone]

@shehi look again at the picture. First two lines inside the <Practitioner /> opening element:

<Practitioner xmlns="http://hl7.org/fhir">
 <!-- from Resource: id, meta, implicitRules, and language -->
 <!-- from DomainResource: text, contained, extension, and modifierExtension -->
...

I don't think I should arbitrarily omit certain fields from types. Would it be possible to see your schema design? Additionally, are you manually adding doctrine's orm annotations to fields or is it being automated in some fashion?

I've been considering adding orm annotations as part of the generation process, but I wasn't sure how useful it would be. It would also require quite a bit of work, as it would need to be generic enough to be portable across, at a minimum, Oracle, MySQL(InnoDB only), PostgreSQL, MSSQL, and SQLite.

I could take a whack at it, but it won't happen quickly.

[shehi]

For the moment, things are quite manual, but not that slow. Example for Patient:

<?php

namespace App\Domain\Document\Fhir\R4;

use App\Domain\Document\Fhir\R4\Element\Reference;
use DateTimeImmutable;
use DateTimeInterface;
use DCarbone\PHPFHIRGenerated\R4\FHIRBooleanPrimitive;
use DCarbone\PHPFHIRGenerated\R4\FHIRElement\FHIRReference;
use DCarbone\PHPFHIRGenerated\R4\FHIRIntegerPrimitive;
use DCarbone\PHPFHIRGenerated\R4\FHIRResource\FHIRDomainResource\FHIRPatient;
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
use Doctrine\ODM\MongoDB\MongoDBException;

/**
 * @ODM\Document(collection="fhir_patient")
 */
class Patient extends FHIRPatient
{
    /**
     * @ODM\Id(strategy="INCREMENT", type="integer")
     */
    protected $id;

    /**
     * @ODM\EmbedMany(targetDocument="App\Domain\Document\Fhir\R4\Element\Identifier")
     */
    protected $identifier;

    /**
     * @ODM\Field(type="fhir-boolean", nullable=true)
     */
    protected $active;

    /**
     * @ODM\EmbedMany(targetDocument="App\Domain\Document\Fhir\R4\Element\HumanName")
     */
    protected $name;

    /**
     * @ODM\EmbedMany(targetDocument="App\Domain\Document\Fhir\R4\Element\ContactPoint")
     */
    protected $telecom;

    /**
     * @ODM\Field(type="string", nullable=true)
     */
    protected $gender;

    /**
     * @ODM\Field(type="date", nullable="true")
     */
    protected $birthDate;

    /**
     * @ODM\Field(type="fhir-boolean", nullable=true)
     */
    protected $deceasedBoolean;

    /**
     * @ODM\Field(type="date", nullable="true")
     */
    protected $deceasedDateTime;

    /**
     * @ODM\EmbedMany(targetDocument="App\Domain\Document\Fhir\R4\Element\Address")
     */
    protected $address;

    /**
     * @ODM\EmbedOne(targetDocument="App\Domain\Document\Fhir\R4\Element\CodeableConcept")
     */
    protected $maritalStatus;

    /**
     * @ODM\Field(type="fhir-boolean", nullable=true)
     */
    protected $multipleBirthBoolean;

    /**
     * @ODM\Field(type="fhir-integer", nullable=true)
     */
    protected $multipleBirthInteger;

    /**
     * @ODM\EmbedMany(targetDocument="App\Domain\Document\Fhir\R4\Element\Attachment")
     */
    protected $photo;

    /**
     * @ODM\EmbedMany(targetDocument="App\Domain\Document\Fhir\R4\Element\Patient\Contact")
     */
    protected $contact;

    /**
     * @ODM\EmbedMany(targetDocument="App\Domain\Document\Fhir\R4\Element\Patient\Communication")
     */
    protected $communication;

    /**
     * @ODM\EmbedMany(nullable=true)
     */
    protected $generalPractitioner;

    /**
     * @ODM\EmbedOne(targetDocument="App\Domain\Document\Fhir\R4\Element\Reference\OrganizationReference")
     */
    protected $managingOrganization;

    /**
     * @ODM\EmbedMany(targetDocument="App\Domain\Document\Fhir\R4\Element\Patient\Link")
     */
    protected $link;

    public function __construct($data = null)
    {
        $this->identifier = new ArrayCollection();
        $this->name = new ArrayCollection();
        $this->telecom = new ArrayCollection();
        $this->address = new ArrayCollection();
        $this->photo = new ArrayCollection();
        $this->contact = new ArrayCollection();
        $this->communication = new ArrayCollection();
        $this->generalPractitioner = new ArrayCollection();
        $this->link = new ArrayCollection();
        parent::__construct($data);
    }

    public function getActive(): ?bool
    {
        $active = parent::getActive();
        if ($active !== null) {
            $value = $active->getValue();
            if ($value instanceof FHIRBooleanPrimitive) {
                return $value->getValue();
            }
        }

        return null;
    }

    public function getGender(): ?string
    {
        /** @var \App\Domain\Document\Fhir\R4\Element\AdministrativeGender $gender */
        $gender = parent::getGender();
        if ($gender !== null) {
            return $gender->getValue();
        }

        return null;
    }

    /**
     * @param null|DateTimeInterface|string $birthDate
     *
     * @return $this
     * @throws \Exception
     */
    public function setBirthDate($birthDate = null): self
    {
        if (is_string($birthDate)) {
            $this->birthDate = new DateTimeImmutable($birthDate);
        } elseif ($birthDate instanceof DateTimeInterface) {
            $this->birthDate = $birthDate;
        }

        return $this;
    }

    public function getDeceasedBoolean(): ?bool
    {
        $deceasedBoolean = parent::getDeceasedBoolean();
        if ($deceasedBoolean !== null) {
            $value = $deceasedBoolean->getValue();
            if ($value instanceof FHIRBooleanPrimitive) {
                return $value->getValue();
            }
        }

        return null;
    }

    /**
     * @param null|DateTimeInterface|string $deceasedDateTime
     *
     * @return $this
     * @throws \Exception
     */
    public function setDeceasedDateTime($deceasedDateTime = null): self
    {
        if (is_string($deceasedDateTime)) {
            $this->deceasedDateTime = new DateTimeImmutable($deceasedDateTime);
        } elseif ($deceasedDateTime instanceof DateTimeInterface) {
            $this->deceasedDateTime = $deceasedDateTime;
        }

        return $this;
    }

    public function getMultipleBirthBoolean(): ?bool
    {
        $multipleBirthBoolean = parent::getMultipleBirthBoolean();
        if ($multipleBirthBoolean !== null) {
            $value = $multipleBirthBoolean->getValue();
            if ($value instanceof FHIRBooleanPrimitive) {
                return $value->getValue();
            }
        }

        return null;
    }

    public function getMultipleBirthInteger(): ?int
    {
        $multipleBirthInteger = parent::getMultipleBirthInteger();
        if ($multipleBirthInteger !== null) {
            $value = $multipleBirthInteger->getValue();
            if ($value instanceof FHIRIntegerPrimitive) {
                return $value->getValue();
            }
        }

        return null;
    }

    /**
     * @param \DCarbone\PHPFHIRGenerated\R4\FHIRElement\FHIRReference|null $generalPractitioner
     *
     * @return \DCarbone\PHPFHIRGenerated\R4\FHIRResource\FHIRDomainResource\FHIRPatient
     * @throws \Doctrine\ODM\MongoDB\MongoDBException
     */
    public function addGeneralPractitioner(FHIRReference $generalPractitioner = null): FHIRPatient
    {
        if (!($generalPractitioner instanceof Reference\OrganizationReference) && !($generalPractitioner instanceof Reference\PractitionerReference)) {
            throw MongoDBException::invalidValueForType(__CLASS__ . '::generalPractitioner', [Reference\OrganizationReference::class, Reference\PractitionerReference::class],
                get_class($generalPractitioner));
        }

        return parent::addGeneralPractitioner($generalPractitioner);
    }

    /**
     * @param \DCarbone\PHPFHIRGenerated\R4\FHIRElement\FHIRReference[] $generalPractitioner
     *
     * @return $this|\DCarbone\PHPFHIRGenerated\R4\FHIRResource\FHIRDomainResource\FHIRPatient
     * @throws \Doctrine\ODM\MongoDB\MongoDBException
     */
    public function setGeneralPractitioner(array $generalPractitioner = [])
    {
        $this->generalPractitioner = new ArrayCollection();
        foreach ($generalPractitioner as $value) {
            $this->addGeneralPractitioner($value);
        }

        return $this;
    }

    /**
     * @param \DCarbone\PHPFHIRGenerated\R4\FHIRElement\FHIRReference|null $managingOrganization
     *
     * @return \DCarbone\PHPFHIRGenerated\R4\FHIRResource\FHIRDomainResource\FHIRPatient
     * @throws \Doctrine\ODM\MongoDB\MongoDBException
     */
    public function setManagingOrganization(FHIRReference $managingOrganization = null): FHIRPatient
    {
        if (!($managingOrganization instanceof Reference\OrganizationReference)) {
            throw MongoDBException::invalidValueForType(__CLASS__ . '::managingOrganization', Reference\OrganizationReference::class, get_class($managingOrganization));
        }

        return parent::setManagingOrganization($managingOrganization);
    }
}

No validation or (de-)serialization attached to them yet. Probably will manually implement those based on Schematron docs, e.g.: http://hl7.org/fhir/practitioner.sch

And no, please don't add Doctrine annotations :) Many people will implement this thing in so many ways - you actually need to be as minimalistic as possible, at the same time maintaining full compliance to the standard.

My problem with those FHIRDomainResource properties is this: when I use these documents I created as ApiResource's, I get pollution from these props: Guess I will have to create serialization profiles to filter them out. I will need to do that anyway, eventually. Was hoping of some quicker solution of getting rid of these unneeded props.

[dcarbone]

I'm not entirely certain what you mean by pollution; the fields are a part of the type. What problems are they causing you, specifically?

One possible guess at a solution would be to define a trait in your extended classes that overloads the base methods as you have done in your child class. This trait could be re-used in all of your child classes, without the need to re-define the methods / annotations in each one.

[shehi]

Currently, I successfully save these docs to MongoDb. Screenshot for you:

[shehi]

I'm not entirely certain what you mean by pollution; the fields are a part of the type. What problems are they causing you, specifically?

Well, I only need to implement what I see in Structure and UML diagrams:

Everything else is just those extensions and profiles I don't need in my current implementation.

[shehi]

Even test repo searches don't talk about these "fields": http://test.fhir.org/r4/Patient/_search

So, overall, my API consumers shouldn't be made confused because of them. Ok, I guess you can't remove them then. :)

[dcarbone]

Ok, so a couple things:

First, what are you using to generate the swagger doc json? https://github.com/zircote/swagger-php? I'm quite familiar with this package, and I have often thought of adding its annotations as part of the generation. Would take a little bit, but overall would be fairly straightforward. Let me know if that is something that would be helpful to you.

Secondly, simply because those specific examples do not demonstrate the parent field's use does not mean they are not in use by somebody, somewhere :)

The definition of an R4 Patient, for example:

<Patient xmlns="http://hl7.org/fhir">
 <!-- from Resource: id, meta, implicitRules, and language -->
 <!-- from DomainResource: text, contained, extension, and modifierExtension -->
</Patient>

Third, I am still unsure of the actual problem. If the issue is those fields will never be used by you and you don't want them as part of your API until you do, I suppose I can understand that. From my personal experience, however, I don't really see the benefit of not having the full specification present from the outset, even if the fields aren't used initially. Extensions in particular may come in use one day (many places use extensions), so I would recommend making your consumers immediately aware of their possibility.

Finally, I think there may be benefit to creating additional repos based on additional generators that target ORM models specifically. They are just different enough (use of ArrayCollection classes in lieu of just array() types, for example), that I think it would be difficult to properly support both from the get-go.

It may be possible to circumvent this by having a schema xml or yaml file defined; this would eliminate the need for in-class annotations and allow for one generator to generate schema definitions that would work for both the relational db's supported by Doctrine and Mongo.

[shehi]

First, what are you using to generate the swagger doc json? https://github.com/zircote/swagger-php? I'm quite familiar with this package, and I have often thought of adding its annotations as part of the generation. Would take a little bit, but overall would be fairly straightforward. Let me know if that is something that would be helpful to you.

I use API-Platform (api-platform.com) which does all the automation. It's basically Symfony 4 with Flex (autowiring and all that niceties). I don't touch Swagger stuff. The whole idea is to automate as much as possible and make things as fast as possible for prototype/MVP creation.

Secondly, simply because those specific examples do not demonstrate the parent field's use does not mean they are not in use by somebody, somewhere :)

The definition of an R4 Patient, for example:

<Patient xmlns="http://hl7.org/fhir">
 <!-- from Resource: id, meta, implicitRules, and language -->
 <!-- from DomainResource: text, contained, extension, and modifierExtension -->
</Patient>

I agree to an extent. You are right by "somebody somewhere". I believe they are a "gray area" useful only for those who want to extend base standards. They don't look to be part of base standard itself.

Third, I am still unsure of the actual problem. If the issue is those fields will never be used by you and you don't want them as part of your API until you do, I suppose I can understand that. From my personal experience, however, I don't really see the benefit of not having the full specification present from the outset, even if the fields aren't used initially. Extensions in particular may come in use one day (many places use extensions), so I would recommend making your consumers immediately aware of their possibility.

Yea, but as per YAGNI principle, I prefer not to implement what I don't need. Such implementations are "costly" :) As I said before, I will try to solve this particular "problem" with Symfony's data Serialization.

Finally, I think there may be benefit to creating additional repos based on additional generators that target ORM models specifically. They are just different enough (use of ArrayCollection classes in lieu of just array() types, for example), that I think it would be difficult to properly support both from the get-go.

Yea, that's what I meant when I said earlier, that maybe it'd be a good idea to create Doctrine-specific repo for this thing. But, as I demonstrated above, ArrayCollection() works quite fine in conjunction with PHP arrays, and mostly whole implementation is solid. So creating a separate repo: I will wait with that.

It maybe possible to circumvent this by having a schema xml or yaml file defined; this would eliminate the need for in-class annotations and allow for one generator to generate schema definitions that would work for both the relational db's supported by Doctrine and Mongo.

Hmm, it's a good idea actually. I will also try creating .mongodb.xml files for these Docs and see how they behave with that. I kinda doubt XML would enforce what Annotations don't (i.e. as you can see, even those fields in DomainResource classes aren't annotated at all, I get them listed as part of ApiResources - I doubt XML configuration will behave any differently. I will test it tho.)

[shehi]

Regarding the last bit above: as long as we have setter/getter pairs in the code, Doctrine will include those fields in ApiResources. Since we can't get rid of accessors/mutators even if we declared those props private, I guess the best course of action is to use Serialization in Symfony.

[dcarbone]

@shehi let me know if I can help, or if there is something else I can add to the generated output to assist with your efforts!

[dcarbone]

@shehi reviewing this post: it is odd that those fields are being picked up at all...

I wonder if that is a peculiarity with api-platform. Doctrine should entirely ignore properties on classes that are not explicitly reference by either annotation or external schema definition. It has been awhile since I used doctrine annotations (i really like doctrine, though, my favorite orm / dbal layer for php), but I could have sworn it entirely ignored non-explicitly mapped fields.

Maybe try something like this: https://github.com/api-platform/api-platform/issues/393? I'm not familiar with api-platform, but it would seem that this would be a pretty obvious feature for them to support...

edit: this seems to be the correct way to do it: https://github.com/api-platform/core/issues/1120

[shehi]

Yea, that's what I meant above: Serialization :)

https://symfony.com/doc/current/components/serializer.html