geonetwork / core-geonetwork

GeoNetwork is a catalog application to manage spatially referenced resources. It provides powerful metadata editing and search functions as well as an interactive web map viewer. It is currently used in numerous Spatial Data Infrastructure initiatives across the world.
http://geonetwork-opensource.org/
GNU General Public License v2.0
412 stars 487 forks source link

Migration to 3.4 blows on duplicate ui/config settings key #3117

Open landryb opened 5 years ago

landryb commented 5 years ago

Upgrading a database from 3.0.4 to 3.4.1 blows:

java.lang.ClassNotFoundException: org.fao.geonet.api.records.attachments.MetadataResourceDatabaseMigration
        at org.apache.catalina.loader.WebappClassLoaderBase.loadClass(WebappClassLoaderBase.java:1285)
        at org.apache.catalina.loader.WebappClassLoaderBase.loadClass(WebappClassLoaderBase.java:1119)
        at java.lang.Class.forName0(Native Method)
        at java.lang.Class.forName(Class.java:264)
        at org.fao.geonet.DatabaseMigration.runJavaMigration(DatabaseMigration.java:276)
        at org.fao.geonet.DatabaseMigration.doMigration(DatabaseMigration.java:219)
        at org.fao.geonet.DatabaseMigration.migrateDatabase(DatabaseMigration.java:154)
        at org.fao.geonet.DatabaseMigration.postProcessAfterInitialization(DatabaseMigration.java:124)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsAfterInitialization(AbstractAutowireCapableBeanFactory.java:422)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1583)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:545)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:482)
        at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:306)
        at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230)
        at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:302)
        at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:197)
        at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:772)
        at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:839)
        at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:538)
        at jeeves.config.springutil.JeevesContextLoaderListener.contextInitialized(JeevesContextLoaderListener.java:111)
        at org.apache.catalina.core.StandardContext.listenerStart(StandardContext.java:4753)
        at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5215)
        at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
        at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:752)
        at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:728)
        at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:734)
        at org.apache.catalina.startup.HostConfig.deployWAR(HostConfig.java:952)
        at org.apache.catalina.startup.HostConfig$DeployWar.run(HostConfig.java:1823)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
2018-09-26 13:52:34,433 WARN  [geonetwork.database] - SQL failure for:  INSERT INTO Settings (name, value, datatype, position, internal) VALUES   ('ui/config', '{"langDetector":{"fromHtmlTag":false,"regexp":"^/[a-zA-Z0-9_-]+/[a-zA-Z0-9_-]+/([a-z]{3})/","default":"eng"},"nodeDete
ctor":{"regexp":"^/[a-zA-Z0-9_-]+/([a-zA-Z0-9_-]+)/[a-z]{3}/","default":"srv"},"mods":{"header":{"enabled":true,"languages":{"eng":"en","dut":"nl","fre":"fr","ger":"ge","kor":"ko","spa":"es","cze":"cz","cat":"ca","fin":"fi","ice":"is", "rus": "ru", "chi": "zh"}},"home":{"enabled
":true,"appUrl":"../../srv/{{lang}}/catalog.search#/home"},"search":{"enabled":true,"appUrl":"../../srv/{{lang}}/catalog.search#/search","hitsperpageValues":[10,50,100],"paginationInfo":{"hitsPerPage":20},"facetsSummaryType":"details","facetConfig":[],"facetTabField":"","filters
":{},"sortbyValues":[{"sortBy":"relevance","sortOrder":""},{"sortBy":"changeDate","sortOrder":""},{"sortBy":"title","sortOrder":"reverse"},{"sortBy":"rating","sortOrder":""},{"sortBy":"popularity","sortOrder":""},{"sortBy":"denominatorDesc","sortOrder":""},{"sortBy":"denominator
Asc","sortOrder":"reverse"}],"sortBy":"relevance","resultViewTpls":[{"tplUrl":"../../catalog/components/search/resultsview/partials/viewtemplates/grid.html","tooltip":"Grid","icon":"fa-th"}],"resultTemplate":"../../catalog/components/search/resultsview/partials/viewtemplates/gri
d.html","formatter":{"list":[{"label":"full","url":"../api/records/{{uuid}}/formatters/xsl-view?root=div&view=advanced"}]},"grid":{"related":["parent","children","services","datasets"]},"linkTypes":{"links":["LINK","kml"],"downloads":["DOWNLOAD"],"layers":["OGC"],"maps":["ows"]}
,"isFilterTagsDisplayedInSearch":false},"map":{"enabled":true,"appUrl":"../../srv/{{lang}}/catalog.search#/map","is3DModeAllowed":true,"isSaveMapInCatalogAllowed":true,"isExportMapAsImageEnabled":true,"bingKey":"AnElW2Zqi4fI-9cYx1LHiQfokQ9GrNzcjOh_p_0hkO1yo78ba8zTLARcLBIf8H6D","
storage":"sessionStorage","map":"../../map/config-viewer.xml","listOfServices":{"wms":[],"wmts":[]},"useOSM":true,"context":"","layer":{"url":"http://www2.demis.nl/mapserver/wms.asp?","layers":"Countries","version":"1.1.1"},"projection":"EPSG:3857","projectionList":[{"code":"EPS
G:4326","label":"WGS84(EPSG:4326)"},{"code":"EPSG:3857","label":"Googlemercator(EPSG:3857)"}],"disabledTools":{"processes":false,"addLayers":false,"layers":false,"filter":false,"contexts":false,"print":false,"mInteraction":false,"graticule":false,"syncAllLayers":false,"drawVecto
r":false},"searchMapLayers":[],"viewerMapLayers":[]},"editor":{"enabled":true,"appUrl":"../../srv/{{lang}}/catalog.edit","isUserRecordsOnly":false,"isFilterTagsDisplayed":false,"createPageTpl": "../../catalog/templates/editor/new-metadata-horizontal.html"},"admin":{"enabled":tru
e,"appUrl":"../../srv/{{lang}}/admin.console"},"signin":{"enabled":true,"appUrl":"../../srv/{{lang}}/catalog.signin"},"signout":{"appUrl":"../../signout"}}}', 3, 10000, 'n'), error is:ERROR: duplicate key value violates unique constraint "settings_pkey"
  Détail : Key (name)=(ui/config) already exists.
org.postgresql.util.PSQLException: ERROR: duplicate key value violates unique constraint "settings_pkey"
  Détail : Key (name)=(ui/config) already exists.

indeed, both v330 & v340 migrations try to set this key, with slightly different values...

landryb commented 5 years ago

It also seems the first part of the error is because the WEB-INF/config-db/database_migration.xml is looking for a org.fao.geonet.api.records.attachments.MetadataResourceDatabaseMigration java class when doing the 3.1.0 migration, but the actual class is org.fao.geonet.MetadataResourceDatabaseMigration after ea102cf21b49bd5bfdc95de70e07ebdd24c361fd - i think the migration config should be amended.

landryb commented 5 years ago

Fixing the class name in the database migration xml config file still fails at runtime, blows with:

Caused by: java.lang.NullPointerException
        at org.fao.geonet.MetadataResourceDatabaseMigration.update(MetadataResourceDatabaseMigration.java:143)
        ... 29 more

which points to https://github.com/georchestra/geonetwork/blob/georchestra-gn3.4-18.06/core/src/main/java/org/fao/geonet/MetadataResourceDatabaseMigration.java#L143 and i have no idea why this would trigger an NPE. @fxprunayre any idea ?

fxprunayre commented 5 years ago

@fxprunayre any idea ?

Sorry for late reply @landryb but I fear that the migration process may be triggered by spring before the SettingManager is initialized ? If this is the case, I would suggest to start without the Java migration step on startup and call it with the API after startup.

cmangeat commented 5 years ago

Something in this taste maybe (not tested, just an idea) ?

diff --git a/web/src/main/webResources/WEB-INF/config-db/database_migration.xml b/web/src/main/webResources/WEB-INF/config-db/database_migration.xml
index fd28fad..b2df2e6 100644
--- a/web/src/main/webResources/WEB-INF/config-db/database_migration.xml
+++ b/web/src/main/webResources/WEB-INF/config-db/database_migration.xml
@@ -30,7 +30,7 @@
                http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
                http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util.xsd
        ">
-  <bean id="database-migration-bean" class="org.fao.geonet.DatabaseMigration">
+  <bean id="database-migration-bean" class="org.fao.geonet.DatabaseMigration" depends-on="SettingManager">
     <property name="migration" ref="migrationMap"/>
     <property name="initAfter" value="javax.sql.DataSource"/>
   </bean>
jahow commented 5 years ago

Hi,

After investigating I found out that adding a explicit dependency on the SettingManager bean is not enough, as the migration runs before this: https://github.com/geonetwork/core-geonetwork/blob/07ad618e213df7090034f7356ab50d7d5f473e0a/web/src/main/java/org/fao/geonet/Geonetwork.java#L137-L140

As such, ApplicationContextHolder.get() returns null which causes all kinds of RTE.

The migration must somehow run as soon as the application context is available, but this goes beyond my skills in Java. If anyone has a clue, please help fix this as having a broken migration system really is a pain.

fxprunayre commented 5 years ago

ui/config should be migrated using JsonMigration now - see https://github.com/geonetwork/core-geonetwork/pull/3378. The default value is not part of the db creation/migration script anymore.

landryb commented 5 years ago

Still, that doesnt fix the case when someone upgrades from 3.0 to 3.4, and afaict 3.4 is still the stable branch...