Flyway: disable DDL generation from EclipseLink

[poikilotherm]

This is a successor issue to #5862.
This kind of relates to #5854.
This should be read by @pdurbin, @MrK191, @donsizemore and @djbrooke at least.

Since the introduction of Flyway in #5344, only minor modifications happened to the schema.
In #5822 a new column has been added and since it had been merged, the develop branch no longer deploys on new "installations".

Some background on the cause:
EclipseLink ORM generates a DDL script from all models on startup and (due to this setting in persistance.xml) creates new tables when not existing. It does not update existing tables. This job is left to Flyway, which is one reason why it had been introduced.

Flyway throws exceptions due to a conflict, trying to change something already present. In #5822, a new column should be added, but on new deployments, EclipseLink already generated the mapping. Thus the deployment fails.

How to get along with this:
Obviously, one could just use workarounds as #5866 does. It would be better to have a discussion and decision on how to deal with database model, mappings, EclipseLink and Flyway in the future, write down a process and adapt.

IMHO there is only one "good" solution, but this is only a proposal and needs to be discussed and/or modified:

1. Do not let EclipseLink generate DDL anymore.
2. Get a base model mapping in the codebase as a proper baseline. This, ideally, should be the exact mapping as it has been when Flyway had been merged.
3. Continue to write migrations in SQL or Java with Flyway, as already done. Document this part of the process if not already done sufficiently.
4. To ensure 100% compatibility between tables and models, the mapping - now maintained with Flyway as migration files - needs to be validated regularly. This is an ideal task for CI on jenkins.dataverse.org.

It might be an option to switch from EclipseLink JPA to Hibernate JPA as it offers easier to use validation mechanisms than EclipseLink does.

[djbrooke]

Thanks @poikilotherm for the tag, I'll tag @landreev and @scolapasta as well.

[MrK191]

@poikilotherm I agree, in our version we just disabled eclipse-link ddl generation. In terms of hibernate you are going to have some troubles, since some annotation syntax changed and you gonna get LazyLoadingException on the face since for whatever reason lazy loading was disabled in this app. But changing to Hibernate will also resolve the problem that you can't use java streams on entities since there is a bug in the eclipse-link.

[poikilotherm]

Thanks @MrK191 for pointing out the monsters lurking in the shadows. 😄
So you think it might be worse the price doing the switch?
Hibernate seems to be more widely used anyway, standard for Spring etc...

[pdurbin]

@poikilotherm @MrK191 if there's a feature missing from the JPA spec can one of you please open an issue at https://github.com/eclipse-ee4j/jpa-api ? (I have one open issue over there. 😄 )

[MrK191]

@pdurbin Nothing is missing, it's just that the glassfish 4 uses eclipse-link version that has this bug.
@poikilotherm Nah I think it's good direction almost no-one heard of eclipse-link when I talk about JPA now :P, so in long-term it's good decision.

[pdurbin]

@MrK191 ok, you're saying there's a bug in the version of eclipse-link in the somewhat ancient version of Glassfish we're stuck on. That's bad news but I'm glad to hear that there's nothing missing from the JPA standard and that we don't have to move to one particular JPA implementation (Hibernate) to resolve this issue. I like standards. 😄

[pdurbin]

@poikilotherm and I had a short chat about this issue just now: http://irclog.iq.harvard.edu/dataverse/2019-10-22#i_110035

My take away is that overall, adding Flyway to the mix has been a step forward but there are further improvements we can make so that upgrading Dataverse is easier.

[pdurbin]

Related:

• Mobilizing "create-tables" in persistence.xml and removing unnecessary errors when table is re-created #4920

[pdurbin]

Related:

• Docker compose postgres volume update #10160

[poikilotherm]

Recently I gave creating a new base migration a shot. Turns out: works perfectly. So we make of this anytime we want to get rid of the DDL generation. @pdurbin thinks this might help with #9686 - it's not so easy to move the columns around with DDL generation getting in the way.

[pdurbin]

@poikilotherm fantastic! Can you please push a branch and maybe even make a pull request so we can see it?

[cmbz]

To focus on the most important features and bugs, we are closing issues created before 2020 (version 5.0) that are not new feature requests with the label 'Type: Feature'.

If you created this issue and you feel the team should revisit this decision, please reopen the issue and leave a comment.

[pdurbin]

Related:

• SQL Warnings on Deployment of WAR File to Test Instance #11074