Problem saving Metadata Mapping module domain objects when testing against OpenMRS 2.1.x

mogoodrich · January 8, 2018, 10:24pm

After our successful upgrade from 1.10.x to 1.11.x, we are getting ambitious and seeing if it would be possible to upgrade all the way to 2.1.x …

To start doing this, we are upgrading some of our key modules to compile and test against OpenMRS 2.1.x, but we are running into problems with tests that attempt to create domain objects (specifically Metadata Sources and Metadata Term Mappings) from the Metadata Mapping module.

We are seeing errors like this:

ERROR - SqlExceptionHelper.logExceptions(146) |2018-01-08 17:08:54,205| NULL not allowed for column "METADATA_TERM_MAPPING_ID"

… which suggests to me that in the test context Hibernate isn’t properly generating the primary key for these new entities when saving.

Peering into the Hibernate mapping files for these entities I see that they are using something called the “NativeIfNotAssignedGenerator”

<id name="metadataTermMappingId" type="java.lang.Integer" column="metadata_term_mapping_id"
			unsaved-value="undefined">
			<generator class="org.openmrs.api.db.hibernate.NativeIfNotAssignedIdentityGenerator" />
		</id>

My guess was that something changed between 1.10.x and 2.1.x and this generator is no longer working in the test context, but I checked for other uses and see that it is used by Concept, and tests that save Concepts do work correctly. I’ve tried to look into the differences in the configuration (in the hbm file) between Concepts and Metadata Mapping entities, but haven’t been able to find anything.

Doing a little debugging, for both Concept and Metadata Mapping entities a flag called “use identity column” is set when saving, suggesting that it should rely on the primary key auto-generated by the database (if I’m understanding it correctly). I understand how this would work with a prod database, but is there some place in the test setup I’m missing where H2 is informed that the Concept id column should be auto-generated? Any other ideas?

Thanks and take care! Mark

raff · January 9, 2018, 11:06am

NativeIfNotAssignedIdentityGenerator is definitely not necessary for metadata_term_mapping_id (or in any other entity in the metadatamapping module), which probably ended up there by copy & paste error. It should be changed to:

<id name="metadataTermMappingId" type="java.lang.Integer" column="metadata_term_mapping_id">
	<generator class="native" />
</id>

Could you please see if it fixes the issue?

mogoodrich · January 9, 2018, 3:12pm

Thanks @raff, that was the next thing I was going to ask… assumedly that will fix the issue, I will confirm though.

mogoodrich · January 9, 2018, 4:00pm

This worked, thanks @raff!

Ticket and fix here:

mogoodrich · January 10, 2018, 5:42pm

So I discovered why inserting Concepts was working but not the entities from Metadata Mapping… we explicitly set up the Concept autoincrement here:

github.com

openmrs/openmrs-core/blob/master/api/src/test/java/org/openmrs/test/BaseContextSensitiveTest.java#L576


	 */
	public void initializeInMemoryDatabase() throws SQLException {
		//Don't allow the user to overwrite data
		if (!useInMemoryDatabase())
			throw new RuntimeException(
			        "You shouldn't be initializing a NON in-memory database. Consider unoverriding useInMemoryDatabase");


		setAutoIncrementOnTablesWithNativeIfNotAssignedIdentityGenerator();
		executeDataSet(INITIAL_XML_DATASET_PACKAGE_PATH);
	}


	public void setAutoIncrementOnTablesWithNativeIfNotAssignedIdentityGenerator() throws SQLException {
		/*
		 * Hbm2ddl used in tests creates primary key columns, which are not auto incremented if
		 * NativeIfNotAssignedIdentityGenerator is used. We need to alter those columns in tests.
		 */
		List<String> tables = Collections.singletonList("concept");
		for (String table : tables) {
			getConnection().prepareStatement("ALTER TABLE " + table + " ALTER COLUMN " + table + "_id INT AUTO_INCREMENT")
					.execute();
		}

We ended up discovering this because we had some tests that were skipping this base setup and failing when trying to insert Concepts, which was rather mystifying we found this which explained everything…

I’m going to pull out the part that sets the auto-increment into it’s own public method so that subclasses can set up the auto-increments without have to initialize the in-memory database.

Take care, Mark