Fix equality check for non-data classes

[ellykits]

Fixes #36

It's a good idea to open an issue first for discussion.

• Tests pass
• Appropriate changes to documentation are included in the PR

[jingtang10]

Thanks @ellykits!

As discussed earlier today, can you please see if you can write equality tests for all the example resources we have? so load all the example resources, for each resource load twice and check that they are equal.

I'm not sure how to check inequality... seems like we should, but I don't have a good idea how to do that systematically.

[ellykits]

@jingtang10 I have implemented the round-trip test for the resource comparison for R4 via this commit message - 0ca74e1c8a (investigating NullPointer exception for R4B and R5), as a placeholder inside the current file SerializationRoundTripTest, to reuse some of the logic we need for running the round-trip tests. We could have it this way and rename the file, or implement this in a separate file and extract the shared code. What do you think?

[ellykits]

@jingtang10 I have implemented the round-trip test for the resource comparison for R4 via this commit message - 0ca74e1c8a (investigating NullPointer exception for R4B and R5), as a placeholder inside the current file SerializationRoundTripTest, to reuse some of the logic we need for running the round-trip tests. We could have it this way and rename the file, or implement this in a separate file and extract the shared code. What do you think?

NullPointerException resolved. I've added the test cases for R4B and R5.

[jingtang10]

@jingtang10 I have implemented the round-trip test for the resource comparison for R4 via this commit message - 0ca74e1c8a (investigating NullPointer exception for R4B and R5), as a placeholder inside the current file SerializationRoundTripTest, to reuse some of the logic we need for running the round-trip tests. We could have it this way and rename the file, or implement this in a separate file and extract the shared code. What do you think?

I think should be in a separate file - and extract the common utilities.

[ellykits]

@jingtang10 I have implemented the round-trip test for the resource comparison for R4 via this commit message - 0ca74e1c8a (investigating NullPointer exception for R4B and R5), as a placeholder inside the current file SerializationRoundTripTest, to reuse some of the logic we need for running the round-trip tests. We could have it this way and rename the file, or implement this in a separate file and extract the shared code. What do you think?

I think should be in a separate file - and extract the common utilities.

Done!

[jingtang10]

Thanks Elly for addressing the comments - the only thing remaining is the SecurityLabel.kt file I think.

[ellykits]

From my investigations:

• The security labels enum has a similar issue we were facing when generating the enum class from PublicationStatus enum, where two value sets were used with the same common binding name. Refer to the README-> Excluded value sets from enum generation.
• To resolve the above, we excluded one of the value set URLs that was causing problems, then used one that generated enum constants matching R4 and R4B.
• The reason why the security label issue could not be reproduced earlier is that there are many instances where the SecurityLabel name is used for these value set URLs: http://hl7.org/fhir/ValueSet/security-labels and http://hl7.org/fhir/ValueSet/security-label-examples. Depending on the order in which they are loaded, the Kotlin Poet library would overwrite the filespec during code generation.
• The workaround would be to exclude the http://hl7.org/fhir/ValueSet/security-label-examples (it has fewer constants compared to R4 and R4B) and update the documentation in the README to resolve the problem.

Here as the structure definitions that use common binding name "SecurityLabels" with different value sets.

Structure definition AuditEvent specifies ValueSet with url http://hl7.org/fhir/ValueSet/security-label-examples with binding name SecurityLabels
Structure definition Meta specifies ValueSet with url http://hl7.org/fhir/ValueSet/security-labels with binding name SecurityLabels
Structure definition Consent specifies ValueSet with url http://hl7.org/fhir/ValueSet/security-label-examples with binding name SecurityLabels
Structure definition DocumentReference specifies ValueSet with url http://hl7.org/fhir/ValueSet/security-label-examples with binding name SecurityLabels

[jingtang10]

From my investigations:

• The security labels enum has a similar issue we were facing when generating the enum class from PublicationStatus enum, where two value sets were used with the same common binding name. Refer to the README-> Excluded value sets from enum generation.
• To resolve the above, we excluded one of the value set URLs that was causing problems, then used one that generated enum constants matching R4 and R4B.
• The reason why the security label issue could not be reproduced earlier is that there are many instances where the SecurityLabel name is used for these value set URLs: http://hl7.org/fhir/ValueSet/security-labels and http://hl7.org/fhir/ValueSet/security-label-examples. Depending on the order in which they are loaded, the Kotlin Poet library would overwrite the filespec during code generation.
• The workaround would be to exclude the http://hl7.org/fhir/ValueSet/security-label-examples (it has fewer constants compared to R4 and R4B) and update the documentation in the README to resolve the problem.

Here as the structure definitions that use common binding name "SecurityLabels" with different value sets.

Structure definition AuditEvent specifies ValueSet with url http://hl7.org/fhir/ValueSet/security-label-examples with binding name SecurityLabels
Structure definition Meta specifies ValueSet with url http://hl7.org/fhir/ValueSet/security-labels with binding name SecurityLabels
Structure definition Consent specifies ValueSet with url http://hl7.org/fhir/ValueSet/security-label-examples with binding name SecurityLabels
Structure definition DocumentReference specifies ValueSet with url http://hl7.org/fhir/ValueSet/security-label-examples with binding name SecurityLabels

Thanks for the very detailed explanation @ellykits!

Is it possible to change the codegen so that in such cases we throw an exception? Just so that we're not relying on finding such problems by chance.

[jingtang10]

A couple of additional points: according to this https://chat.fhir.org/#narrow/channel/179166-implementers/topic/Kotlin.20FHIR.20model.20code.20generation.20questions, i wonder if we should use the name of the valueset instead of the binding name extension. @ellykits mentioned there was some issues with that approach for us - please doublecheck and let's document them.

So the approach taken here is that we pick the valueset that is the superset of the other conflicting valuesets, and ignore those valuests... the problem here is that we're actually increasing the number of possible values for fields referring to the smaller valuesets. Another problem is that this is just not a very scalable and robust approach since it requires us inspecting the conflicts. This still might be ok if these are the only cases we're dealing with. (We might find out these aren't after we add the exception in my previous comment).

An alternative approach is to generate local enums for conflicts. So if two bindings are conflicting. We simply avoid generating global enums. I'm not saying we should definitely do it, let's do the investigation above - and have this approach in our back pocket.

[jingtang10]

approving - but pls merge only after #44 is merged this is updated (and verify that the changes in SecurityLabels.kt are reverted).

[jingtang10]

I thought i commented on this :/ let's remove this in a subsequent PR (probably no need to send a 1 line change)

[ellykits]

Sure. I forgot about this, thanks for this reminder.

…

On Thu, Aug 21, 2025, 19:45 Jing Tang ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In fhir-model/build.gradle.kts <#37 (comment)>: > + dependencies { + implementation("org.jetbrains.kotlin:kotlin-reflect") + } I thought i commented on this :/ let's remove this in a subsequent PR (probably no need to send a 1 line change) — Reply to this email directly, view it on GitHub <#37 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGM667DVNRXQFL2EM3NUCQD3OXZRHAVCNFSM6AAAAACDH5FECKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTCNBRGUYTONJXGM> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>