Skip to content
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

Refactoring to separate out RM-specific code from generic. #589

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

wolandscat
Copy link
Member

This set of changes is renames and some class moves, to more systematically group openEHR RM specific classes, including xml/json/odin specific adaptations in the openehr-rm module.

The design intent is to be able to add other RMs to Archie in a more straightforward way.

Quite a few renames of classes that were openEHR RM specific to names like OpenehrRmXXXX where XXXX was the original name. This makes it easier to know at a glance whether something is specific to one RM or not.

The dependency of OpenEHRTypeNaming (renamed to ArchieTypeNameResolver) on openEHR RM has been removed by giving it an extra constructor parameter.

The class JacksonUtil class (renamed to OpenehrRmJacksonUtil since it is RM-specific) has a small change to correspond with the above, i.e. injecting the OpenEhrRmInfoLookup.getInstance()singleton intoArchieTypeNameResolver`.

Some build.gradle dependencies adjusted. No semantic changes.

Changes all compile and tests run normally.

Further code improvements are possible so DON'T MERGE this yet!

Copy link

codecov bot commented Apr 9, 2024

Codecov Report

Attention: Patch coverage is 90.07353% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 70.32%. Comparing base (c065910) to head (9b4d721).

Files Patch % Lines
...e/openehr/serialisation/xml/OpenEhrRmJAXBUtil.java 55.00% 8 Missing and 1 partial ⚠️
...com/nedap/archie/rminfo/MetaModelsInitialiser.java 84.61% 5 Missing and 1 partial ⚠️
...parser/treewalkers/PrimitivesConstraintParser.java 93.93% 0 Missing and 2 partials ⚠️
...nedap/archie/rminfo/ReflectionModelInfoLookup.java 77.77% 0 Missing and 2 partials ⚠️
...in/java/com/nedap/archie/aom/CPrimitiveObject.java 80.00% 1 Missing ⚠️
...a/com/nedap/archie/definitions/AdlDefinitions.java 0.00% 1 Missing ⚠️
...a/com/nedap/archie/rminfo/ArchieAOMInfoLookup.java 50.00% 1 Missing ⚠️
...edap/archie/rminfo/OpenEhrModelNamingStrategy.java 83.33% 0 Missing and 1 partial ⚠️
...openehr/rminfo/OpenEhrRmMetaModelsInitialiser.java 91.66% 1 Missing ⚠️
...e/openehr/rminfo/OpenEhrRmUpdatedValueHandler.java 80.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #589      +/-   ##
============================================
- Coverage     71.80%   70.32%   -1.48%     
- Complexity     6956     6995      +39     
============================================
  Files           663      686      +23     
  Lines         22691    23299     +608     
  Branches       3676     3703      +27     
============================================
+ Hits          16294    16386      +92     
- Misses         4664     5167     +503     
- Partials       1733     1746      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Remove contents of referencemodels, but retain tests (forn now).
Package moves to org.openehr to facilitate new RM addition to Archie.
Some class renames for clarity.
Revert change to OpenEhrRmInfoLookup.addTypes()
OpenEhrRmMetaModelsInitialiser.getNativeRms() probably not safe for Android.
Use MetaModels during parsing of CPrimitiveObjects to get access to the correct ModelNamingStrategy. Currently conditional since ADLParser can be created with no MetaModels as well.

In PrimitivesConstraintParser, if the ModelNamingStrategy object can be found, write in the correct RmTypeNames into CPrimitiveObjects (CTerminologyCode and so on).

Rename ArchieModelNamingStrategy to OpenEhrModelNamingStrategy.
Move tests with static dependencies on openEHR RM classes from tools to openehr-rm.
Move or copy other test resources to openehr-rm as needed.
Create tools/src/testFixtures area for test helper classes shared across projects.
…ss CkmRepositoryBuilder in openehr-rm.

Use this for any access to the CKM-mirror copy.
…ument so as to use the correct /resources area.

TODO: there might be a better way to do this, but it seems the safest way to share /resources across projects given the Java model of class loading.
…now take a 'class' arg to ensure the corect /resources area is access (i.e. the caller's area, which could be in the same project or another).
…nd Assert. Not sure if this matters functionally - probably just an efficiency thing.

TODO: check whether this change introduces any errors, or whether it can be reverted by some other simple change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant