have OMERO server populate database with missing enumeration values #4837

mtbc · 2016-09-12T11:37:34Z

What this PR does

When a new version of OMERO.server starts up (minor or major) it reviews the enumeration values defined in <enum> tags in components/model/resources/mappings/ and ensures that all those strings exist in the corresponding database tables.

Testing this PR

In an older version of OMERO delete a few enumeration values from the database. Upgrade to and start up the latest version of OMERO with this PR included and see if those values are added back.

Additionally the format table should continue to be populated with new image formats as reported by Bio-Formats readers whenever a new version of Bio-Formats is detected so in the above testing maybe also delete a few formats too that you know should be there (beyond just those listed in acquisition.ome.xml).

Related reading

https://trello.com/c/0zNp8HvU/167-enumerations-addition

to be used at server startup as a kind of upsert for enum values

to be superseded by use of the enumeration ensuring bean

sbesson · 2016-09-13T11:30:03Z

With this commit, the following statements are executed at the server startup (DB upgraded from 5.2__0`):

2016-09-13 05:42:12,321 INFO  [           ome.services.util.BaseDBCheck] (      main) performed DB check DBEnumCheck: done for Bio-Formats revision bb58eb1577d2083f448f25ae3b827d35ed2158a2
2016-09-13 05:42:12,360 INFO  [        ome.services.util.ServiceHandler] (      main)  Executor.doWork -- ensure enum values
2016-09-13 05:42:12,360 INFO  [        ome.services.util.ServiceHandler] (      main)  Args:    [null, InternalSF@1034997834]
2016-09-13 05:42:12,364 INFO  [    ome.security.basic.BasicEventContext] (      main)  cctx:    group=1
2016-09-13 05:42:12,376 INFO  [         ome.security.basic.EventHandler] (      main)  Auth:    user=0,group=1,event=260232(Internal),sess=45ed22bf-458a-4194-a34a-cee0ac778b9c
2016-09-13 05:42:12,403 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value AcquisitionMode.BrightField
2016-09-13 05:42:12,405 INFO  [       ome.security.basic.CurrentDetails] (      main) Adding log:INSERT,class ome.model.enums.AcquisitionMode,22
2016-09-13 05:42:12,405 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value AcquisitionMode.SweptFieldConfocal
2016-09-13 05:42:12,406 INFO  [       ome.security.basic.CurrentDetails] (      main) Adding log:INSERT,class ome.model.enums.AcquisitionMode,23
2016-09-13 05:42:12,407 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value AcquisitionMode.SPIM
2016-09-13 05:42:12,410 INFO  [       ome.security.basic.CurrentDetails] (      main) Adding log:INSERT,class ome.model.enums.AcquisitionMode,24
2016-09-13 05:42:12,421 INFO  [                 org.perf4j.TimingLogger] (      main) start[1473741732361] time[60] tag[omero.call.success.ome.services.util.EnsureEnum$1.doWork]

And importing a file with a SPIM acquisition mode works as expected with the acquisition mode properly populated:

The logic looks sensible and certainly reduces the need for DB ugprade scripts making minor schema releases with enumeration additions potentially achievable in patch releases without too much impact. Having looked quickly at the code, I did not spot anything that raised a red flag to me although it might be good to know if there is some departure with the original intent of DBEnumCheck /cc @joshmoore

NB: the Blitz.log also shows the addition a couple of new companion formats as part of the DB check and I am not sure whether these are still useful. This can be addressed as a follow-up effort.

For the next steps, I would suggest we move this PR (and the companion #4820) out of the breaking queue. Is it required that the corresponding Bio-Formats PR should be migrated as well or is there a test case for having the mappings added only at the OMERO level (in which case I would expect no enumeration addition to the DB and normal startup)?

mtbc · 2016-09-13T11:42:23Z

The companion formats are added on the basis of the reader's hasCompanionFiles() returning true.

Happy to move OMERO PRs out of breaking; of course this will add the new values to the normal DB. I can't see why not to take the Bio-Formats one out too but am happy to wait. The bigger question with that one is the strategy of the version number increment cf. recent weeks' namespace discussion on https://trello.com/c/0zNp8HvU/167-enumerations-addition.

jburel · 2016-09-16T08:38:43Z

components/dsl/resources/ome/dsl/object.vm

@@ -513,7 +513,7 @@ implements java.io.Serializable, IObject
    *
    * will fail. Experimenter has no owner, for obvious reasons.
    *
-    * Note: subclasses of this class will return a subclasse of 
+    * Note: subclasses of this class will return a subclass of


could you get the typo fix out this PR?
since it is not related to this work

sure, force-pushed away and put into #4847

joshmoore · 2016-09-19T10:26:02Z

2 high-ish level thoughts:

In general, SqlAction is a valid extension point (even if in practice I don't think many are doing it), so removing method will be a breaking change.
What's the rationale for permitting the new companion types? i.e. for removing the omission list?

mtbc · 2016-09-19T10:31:33Z

Yes, I figured the chances of someone external using SqlAction to add formats to be pretty low. Could deprecate instead if preferred.
The omission list has only "Fake" on it, unless we expect the list to grow much beyond one element than I figured it was nice to simplify the code.

joshmoore · 2016-09-19T10:36:15Z

Yes, I figured the chances of someone external using SqlAction to add formats to be pretty low. Could deprecate instead if preferred.

I'm less worried about someone consuming the method than someone who has implemented SqlAction and now their @Override annotation will fail compilation.

The omission list has only "Fake" on it, unless we expect the list to grow much beyond one element than I figured it was nice to simplify the code.

Is that the only diff you were seeing, @sbesson ?

mtbc · 2016-09-19T10:38:43Z

Ah, okay, can certainly add method back in then but deprecate.

This reverts commit 408c8d1.

joshmoore · 2016-09-20T19:55:00Z

Thanks for the changes. The added values are:

OMERO.server-5.3.0-m4-253-43428df-ice35-b434 jamoore$ grep EnsureEnum var/log/Blitz-0.log | grep adding
2016-09-20 21:51:12,260 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/FilePattern
2016-09-20 21:51:12,398 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/SPC
2016-09-20 21:51:12,562 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/CellWorx
2016-09-20 21:51:12,615 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/Volocity
2016-09-20 21:51:12,654 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/AFI
2016-09-20 21:51:12,724 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/JEOL
2016-09-20 21:51:12,781 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/BD
2016-09-20 21:51:12,788 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/Unisoku
2016-09-20 21:51:12,796 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/PDS
2016-09-20 21:51:12,803 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/Fuji
2016-09-20 21:51:12,833 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/Inveon
2016-09-20 21:51:12,844 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/CellVoyager
2016-09-20 21:51:13,037 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/PCORAW
2016-09-20 21:51:13,130 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/Trestle
2016-09-20 21:51:13,151 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/ZeissTIFF
2016-09-20 21:51:13,282 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/Hitachi
2016-09-20 21:51:13,291 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value Format.Companion/Bruker
2016-09-20 21:51:13,571 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value AcquisitionMode.BrightField
2016-09-20 21:51:13,582 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value AcquisitionMode.SweptFieldConfocal
2016-09-20 21:51:13,587 INFO  [            ome.services.util.EnsureEnum] (      main) adding to database new enumeration value AcquisitionMode.SPIM

which makes sense. 👍

sbesson · 2016-09-22T08:54:29Z

@mtbc I assume this can be merged independently of #4820 which should be coordinated with the 5.2.3 version bump?

mtbc · 2016-09-22T09:21:54Z

Yes, it can.

mtbc · 2016-10-04T08:40:39Z

components/server/src/ome/services/util/DBEnumCheck.java

+            String name = formatReader.getClass().getSimpleName();
+            if (name.endsWith("Reader")) {
+                name = name.substring(0, name.length() - 6);
+                if ("Fake".equals(name)) {


Should "FilePattern"(Reader) also be excluded?

mtbc added 4 commits September 9, 2016 15:20

create new enumeration ensuring bean

26c0d45

to be used at server startup as a kind of upsert for enum values

remove SqlAction.insertFormat method

408c8d1

to be superseded by use of the enumeration ensuring bean

overhaul DBEnumCheck and have it use the enumeration ensurer

cee7f19

close JAR file in iTypes service

1476a7f

mtbc mentioned this pull request Sep 13, 2016

add enumeration values for acquisition mode #4820

Merged

jburel reviewed Sep 16, 2016

View reviewed changes

mtbc added 3 commits September 16, 2016 09:49

reorder constructor operations to correct initialization

c2357e7

for database startup checks provide server version accessor

89f6109

add new database startup for ensuring enumeration values exist

a9b42c6

mtbc force-pushed the enum-autopopulate branch from e63a447 to a9b42c6 Compare September 16, 2016 08:50

jburel mentioned this pull request Sep 19, 2016

fix minor typo in autogen comment #4847

Merged

mtbc added 2 commits September 19, 2016 14:22

Revert "remove SqlAction.insertFormat method"

0962fbb

This reverts commit 408c8d1.

deprecate SqlAction.insertFormat method

ecbd979

jburel added the develop label Sep 20, 2016

sbesson merged commit 6888b58 into ome:develop Sep 22, 2016

mtbc deleted the enum-autopopulate branch September 22, 2016 09:24

This was referenced Sep 26, 2016

remove enums update script #4864

Merged

remove list of image formats from OMERO code #4865

Merged

mtbc commented Oct 4, 2016

View reviewed changes

mtbc mentioned this pull request Oct 21, 2016

prepare database for light administrator roles #4903

Merged

jburel added this to the 5.3.0 milestone Mar 29, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

have OMERO server populate database with missing enumeration values #4837

have OMERO server populate database with missing enumeration values #4837

mtbc commented Sep 12, 2016 •

edited

sbesson commented Sep 13, 2016

mtbc commented Sep 13, 2016

jburel Sep 16, 2016

mtbc Sep 16, 2016 •

edited

jburel Sep 16, 2016

joshmoore commented Sep 19, 2016

mtbc commented Sep 19, 2016 •

edited

joshmoore commented Sep 19, 2016

mtbc commented Sep 19, 2016

joshmoore commented Sep 20, 2016

sbesson commented Sep 22, 2016

mtbc commented Sep 22, 2016

mtbc Oct 4, 2016

have OMERO server populate database with missing enumeration values #4837

have OMERO server populate database with missing enumeration values #4837

Conversation

mtbc commented Sep 12, 2016 • edited

What this PR does

Testing this PR

Related reading

sbesson commented Sep 13, 2016

mtbc commented Sep 13, 2016

jburel Sep 16, 2016

Choose a reason for hiding this comment

mtbc Sep 16, 2016 • edited

Choose a reason for hiding this comment

jburel Sep 16, 2016

Choose a reason for hiding this comment

joshmoore commented Sep 19, 2016

mtbc commented Sep 19, 2016 • edited

joshmoore commented Sep 19, 2016

mtbc commented Sep 19, 2016

joshmoore commented Sep 20, 2016

sbesson commented Sep 22, 2016

mtbc commented Sep 22, 2016

mtbc Oct 4, 2016

Choose a reason for hiding this comment

mtbc commented Sep 12, 2016 •

edited

mtbc Sep 16, 2016 •

edited

mtbc commented Sep 19, 2016 •

edited