Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mapfishapp - datadir #1038

Merged
merged 12 commits into from Oct 13, 2015
Merged

mapfishapp - datadir #1038

merged 12 commits into from Oct 13, 2015

Conversation

pmauduit
Copy link
Member

This PR proposes the datadirization of the mapfishapp webapp:

  • Exports the contexts, addons, JS configuration (including GEOR_custom.js) into the datadir
  • Makes the rest of the webapp configurable outside of the webapp WAR (using the same mechanisms as previously)
  • Allows mapfishapp to be easily customized by server-side addons (it allows automatic scanning of jars which classes begining with org.georchestra.mapfishapp.addons.*, as well as a applicationContext.xml to be defined. See https://github.com/pmauduit/osm2geor/tree/master/java as an example of use-case)

As the other PR, it requires the introduction of the georchestra-commons module. Travis-ci will fail until #1035 is merged.

Note for the reviewers: A lot of files are related to default addons inclusion, the majority of files added to src/deb/resources/etc are not necessary to be reviewed IMHO.

TODO:

  • Test compilation
  • testsuite
  • runtime test with georchestra.datadir
  • runtime test without georchestra.datadir

@pmauduit
Copy link
Member Author

pmauduit commented Sep 1, 2015

Regression detected without the georchestra.datadir option: Images for the addons are broken.

@pmauduit
Copy link
Member Author

pmauduit commented Sep 1, 2015

Ok fixed ; everything seems to go well now (without the georchestra.datadir variable)

- Exports the contexts, addons, JS configuration (including
  GEOR_custom.js) into the datadir
- Makes the rest of the webapp configurable outside of the webapp WAR
  (using the same mechanisms as previously)
- Allows mapfishapp to be easily customized by server-side addons (it
  allows automatic scanning of jars which classes begin with
  org.georchestra.mapfishapp.addons)

As the other PR, it requires the introduction of the georchestra-module.
Fixing path to the addons in the webapp when no georchestra.datadir is
provided.
@pmauduit
Copy link
Member Author

pmauduit commented Sep 7, 2015

(FYI I pushed --force after rebasing / integration of the georchestra-commons module)

<packageDescription>Debian package for the geOrchestra's Viewer</packageDescription>
<projectOrganization>geOrchestra</projectOrganization>
<maintainerName>Pierre Mauduit</maintainerName>
<maintainerEmail>pierre.mauduit@gmail.com</maintainerEmail>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maintainer = PSC / psc@georchestra.org

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As said, I fear that I had to regenerate a GPG key so that I can publish the signed packages on build.georchestra.org ; it will deserve a try in my georchestra-datadir branch, and see if the CI is not broken by the change.

Anyway, I've no objection for such a change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pmauduit
Copy link
Member Author

pmauduit commented Sep 8, 2015

I will do extra tests by merging back the branch into my own one.

@pmauduit
Copy link
Member Author

pmauduit commented Sep 8, 2015

Runtime tests: Ok (basic tests, mainly on the addons management, with and without georchestra.datadir).

@landryb
Copy link
Member

landryb commented Sep 14, 2015

I'm testing this, with georchestra.datadir set and some customizations done in /etc/georchestra/mapfishapp/js/GEOR_custom.js and mapfishapp/w?s.servers.json, but it seems the values returned by the w?s.servers.json webservice come from the json files in the webapp, and GEOR_custom.js is fetched from the webapp dir too. Interestingly, i see no addon at all listed in 'tools' window, even if some are listed in the default GEOR_custom.js found/used within the webapp. Guess i'll need to dig a bit more....

the mapfishapp/log4j/log4j.properties and mapfishapp/mapfishapp.properties files are properly read/applied from the datadir.

/mapfishapp/addons/ and /mapfishapp/contexts/ yield 404's..

maybe relevant info from mapfishapp.log :

2015-09-14 09:08:23 BaseMapServlet [INFO] app is null, setting it as default configPath: WEB-INF/print/config.yaml
2015-09-14 09:08:23 BaseMapServlet [INFO] app is null, setting it as default configPath: WEB-INF/print/config.yaml
2015-09-14 09:08:23 BaseMapServlet [INFO] Attempting to locate app config file: 'WEB-INF/print/config.yaml in the webapplication.
2015-09-14 09:08:23 BaseMapServlet [INFO] Attempting to locate app config file: 'WEB-INF/print/config.yaml in the webapplication.
2015-09-14 09:08:23 BaseMapServlet [INFO] Unable to find config file in web application using getRealPath.  Adding a / because that is often dropped
2015-09-14 09:08:23 BaseMapServlet [INFO] Unable to find config file in web application using getRealPath.  Adding a / because that is often dropped
2015-09-14 09:08:23 BaseMapServlet [INFO] Loading app from: /data/tomcat/georchestra/webapps/mapfishapp/WEB-INF/print/config.yaml
2015-09-14 09:08:23 BaseMapServlet [INFO] Loading app from: /data/tomcat/georchestra/webapps/mapfishapp/WEB-INF/print/config.yaml
2015-09-14 09:08:23 BaseMapServlet [INFO] Loading configuration file: /data/tomcat/georchestra/webapps/mapfishapp/WEB-INF/print/config.yaml
2015-09-14 09:08:23 BaseMapServlet [INFO] Loading configuration file: /data/tomcat/georchestra/webapps/mapfishapp/WEB-INF/print/config.yaml

@landryb
Copy link
Member

landryb commented Sep 14, 2015

Hm, i was mistaken, at mapfishapp loading /ws/app/js/GEOR_custom.js is called and it effectively returns the content of the file found in the datadir.

@landryb
Copy link
Member

landryb commented Sep 14, 2015

/ws/addons/ returns

{"success":false, "error":"fileupload_error_ioError", "msg": "null"} 

java.lang.NullPointerException at org.georchestra.mapfishapp.ws.AddonController.buildAddonSpecs(AddonController.java:188)

/ws/contexts/ returns the default geobretagne context, which seems expected, although it isnt loaded at initial mapfishapp display. It seems nothing calls /ws/contexts/ nor /ws/addons/ at startup....

@landryb
Copy link
Member

landryb commented Sep 14, 2015

As for the w?s servers, by default mapfishapp/wms.servers.json is loaded/queried (which returns the default list from the webapp) while i think it should be mapfishapp/ws/wms.servers.json (which contains my custom list..)

@pmauduit
Copy link
Member Author

TODO:

  • Remove contexts and addons from the provided GEOR_custom.js
  • hardenning contexts / addons to avoid NPE in case of non existing directory in the datadir

@landryb
Copy link
Member

landryb commented Sep 14, 2015

For the w?s server list, i think GEOR_LayerBrowser.js just lacks /ws/ being prepended to GEOR.config.OGC_SERVERS_URL[this.id] in line 59, otherwise the list comes from the webapp.

Tested this, works..

--- a/mapfishapp/src/main/webapp/app/js/GEOR_LayerBrowser.js
+++ b/mapfishapp/src/main/webapp/app/js/GEOR_LayerBrowser.js
@@ -56,7 +56,7 @@ GEOR.LayerBrowser = Ext.extend(Ext.Panel, {
         this.serverStore = new Ext.data.Store({
             proxy: new Ext.data.HttpProxy({
                 url: GEOR.util.getValidURI(
-                    GEOR.config.OGC_SERVERS_URL[this.id]
+                    "ws/" + GEOR.config.OGC_SERVERS_URL[this.id]
                 ),
                 method: 'GET',

@landryb
Copy link
Member

landryb commented Sep 14, 2015

Maybe only local to my setup, but there are some encoding issues in various places:

  • in the addons list, their description (coming from webapp/app/addons//.json) is double-encoded, yielding ?? instead of accentued letters. This is not the case with default contexts where the accents coming from datadir/contexts/*.wmc are properly rendered - and the addons description is fine in http://sdi.georchestra.org/mapfishapp/?lang=fr
  • saving a context also double-encodes accentued letters (looking bad in mapfishapp db too, although the db is UTF8), badly displayed when re-sending the wmc. Tested the same, works on sdi.georchestra.org.

@landryb
Copy link
Member

landryb commented Sep 14, 2015

More details on the encodings:

  • if i browse to /mapfishapp/ws/contexts/ in firefox, an accentued in displayed as a single ? but they're properly displayed in the wmc browser. curl -i on the url says that content-type is app/js;charset=UTF-8
  • if i browse to /mapfishapp/ws/addons/ in firefox, an accentued letter is displayed as two ??, and they're also displayed this way (ie badly) in the addons browser. curl -i also says that content-type is app/js;charset=UTF-8
  • if i browse to /mapfishapp/ws/wmts.servers.json in firefox, an accentued letter is displayed as utf vomit (ie GéoPortail) and is displayed 'correctly' in the server list dropdown menu. curl -i doesnt specify a content-type, and the accent is properly rendered in curl output in my term

@pmauduit
Copy link
Member Author

if i browse to /mapfishapp/ws/contexts/ in firefox, an accentued in displayed as a single ? but they're properly displayed in the wmc browser. curl -i on the url says that content-type is app/js;charset=UTF-8

Have you set the URIencoding option onto your tomcat connectors ?

<Connector URIEncoding="UTF-8"  ...>

@landryb
Copy link
Member

landryb commented Sep 15, 2015

I had URIEncoding="UTF-8" set on all the tomcat connectors, but the issue was somewhere else. Setting LANG=C.UTF-8 (or any of the other UTF-8 locales available, check locale -a output) in /etc/default/tomcat-* worked around the issue. Guess that's what i get for not having it set in /etc/default/locale......

@landryb
Copy link
Member

landryb commented Sep 15, 2015

Small buglet in addons handling.. added openls,magnifier,cadastrapp,streetview & annotation in /etc/georchestra/mapfishapp/addons, loaded /ws/addons -> extractorapp was in the list (and enabled) although not present in the datadir - i think this was caused by cadastrapp not having a config.json file (oversight from me) - i had the error message, but somehow some addon was randomly taken from the webapp..

2015-09-15 16:50:54 ws [ERROR] Addon cadastrapp does not have a config.json configuration file, skipping it.

@landryb
Copy link
Member

landryb commented Sep 15, 2015

okay, after more explanation from @pmauduit it makes sense to have extractor in the list of app since it's in the webapp, and enabled by default. To disable it, one just needs to make a copy of it in the datadir, and set enable to false in its config.json.

@landryb
Copy link
Member

landryb commented Sep 16, 2015

And with the current logic in https://github.com/georchestra/georchestra/blob/mapfishapp-datadir/mapfishapp/src/main/java/org/georchestra/mapfishapp/ws/AddonController.java#L200 (and also the two loops over addons arrays in constructAddonsSpec()) it doesnt seem possible to 'disable' an addon that is enabled in the webapp. If i copy extractor to my datadir and change enabled to false in config.json, it will still be listed in the available & enabled addons.

Do not check too early if the addon is activated or not, check it at the
very end of the process.

This allows to deactivate addons provided by default in mapfishapp,
simply by copying it into the datadir and explicitely setting it as
disabled in the config.json spec.
- Hardenning tests when trying to find files on the FS
- Aslo allows to set images from different formats (JPEG) for context
  capions.

Tests: utests added for NPE avoidance
runtime tests for the context captions
Now using the controller systematically, irregardless if
georchestra.datadir is configured or not. Getting the relevant file in
both cases (like it is done for other controllers).
@pmauduit
Copy link
Member Author

Done today:

  • Removing remaining comments in the standard mapfishapp addons
  • Fixing bug with disabled addons in the overriden georchestra.datadir path
  • Rework to harden context / addon controller a little bit (avoids NPE)
  • fixing bug with the WxS access: same access for both configuration (with & without datadir)

@landryb
Copy link
Member

landryb commented Sep 21, 2015

Retested tip of the PR, all the previous issues i had are now fixed, i've been able to disable extractor addon and the wxs server lists are fetched from the datadir. Thanks!

@fvanderbiest
Copy link
Member

Good, please merge.

pmauduit added a commit that referenced this pull request Oct 13, 2015
@pmauduit pmauduit merged commit f6c57ed into master Oct 13, 2015
@fvanderbiest fvanderbiest deleted the mapfishapp-datadir branch October 13, 2015 07:49
@fvanderbiest fvanderbiest mentioned this pull request Oct 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants