[META-168] Importing Concepts exported from OpenMRS 1.6 to OpenMRS 1.7/1.8 Created: 2012-01-01  Updated: 2012-08-31  Resolved: 2012-02-21

Status: Closed
Project: Metadata Sharing Module
Component/s: None
Affects Version/s:
Fix Version/s: 1.0

Type: New Feature Priority: Must
Reporter: Rafal Korytkowski Assignee: Wyclif Luyima
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relates to META-44 Allow conversion of data, so exports ... Closed
resolves META-180 NPE when importing a package Closed
Complexity: High


Convert XMLs using XSLTs or Java transformations while importing to account for differences between Concept classes. Do not convert other metadata classes than those required to import Concepts. It is ok to fail if Concept validators fail upon import (for instance due to duplicate Concept names). Think about a way to include other transformations in the future (e.g. from 1.8 to 1.9/1.10).

Ideally transformations would be executed within org.openmrs.module.metadatasharing.serializer.MetadataSerializer#deserialize (maybe via an XStream Converter).

See http://svn.openmrs.org/openmrs-contrib/occ/src/main/resources/1.6-to-1.7.xslt
and http://svn.openmrs.org/openmrs-contrib/occ/src/main/java/org/openmrs/occ/VersionConverter.java

Comment by Wyclif Luyima [ 2012-01-11 ]

code committed in rev:25326 and created review at CR-MOD-352

Comment by Rafal Korytkowski [ 2012-01-11 ]

See my questions in the review in ConceptNameTypeConverter. Can we get rid off that?

Comment by Wyclif Luyima [ 2012-01-11 ]

I added a comment why we need the converter, it is because the xstream enum converter assumes there are never null values yet synonyms are represented by a blank value.

Comment by Wyclif Luyima [ 2012-01-11 ]

On another note, i think to handle this in a more generic way, i think we need to define a list of converters that implement an a specific interface from the module application context so that the import engine looks up all registered converters and invokes them. This has the benefit that we can handle future version changes by writing and registering new converters.

Comment by Rafal Korytkowski [ 2012-01-11 ]

See me other comment

Comment by Wyclif Luyima [ 2012-01-11 ]

FYI, i tested this before in the past with xstream serialization and i tested when i was working on the ticket, the reason it booms is because the value is null or blank as i mentioned in my comment above and not because of a missing tag.

Comment by Rafal Korytkowski [ 2012-01-12 ]

Okay, so I did it for you I removed the ConceptNameTypeConverter and it worked the way I described. See rev:25352. I'd like you to write proper tests for the conversion that happen similar to the convertFrom16To18 test I wrote. Actually change it so that it really tests what the conversion does not just assertNotNull

Comment by Rafal Korytkowski [ 2012-01-12 ]

See my other review comments.

Comment by Wyclif Luyima [ 2012-01-12 ]

Cool, may be they fixed that in a later version of the library and the mistake i made was to coppy over the code snippet you added in the code review which also doesn't work. For the unit tests, if i get time before the end of the sprint i will add them otherwise i will create a follow up ticket to add the ones like you mentioned

Comment by Rafal Korytkowski [ 2012-01-12 ]

Right, the trick was not to include a null property at all. Took 5 minutes to realize though If you in fact successfully imported a concept from 1.6 to 1.8 then I agree these tests are not a must right now.

Comment by Rafal Korytkowski [ 2012-01-16 ]

Wyclif, if you have anything else to commit please do so today or unassign yourself from this ticket.

Comment by Wyclif Luyima [ 2012-01-16 ]

Refactored the convertXX methods into a single method that takes in both the to and from version in rev:25372.

Generated at Wed Nov 21 14:38:57 UTC 2018 using JIRA 7.5.1#75006-sha1:7df2574a6cc842da727f00de4c5ce9ac07701368.