HSY download basket #74

ollijakobsson · 2017-08-10T16:05:15Z

Download Basket (download-basket) adds the functionality of downloading vector data layers in Oskari platform. Layer cropping functionalities included.

Front end bundle configuration guide: http://oskari.org/api/bundles#1.44.1/framework/download-basket

Dowload Basket also uses properties-files for custom configurations. Examples below.

Properties-files:

oskari.wfs.download.folder.name: The physical path to system where downloaded zip-files should be saved, for example /server/temp/. Cron-based auto-delete recommended, not included. A link to a generated zip-archive will work as long as the file exists
oskari.wfs.download.normal.way.downloads: Cropping mode parameter for WFS requests, for example "rectangle"
oskari.wfs.download.smtp.host: The address of smtp-server, for example "localhost"
oskari.wfs.download.smtp.port: The port of smtp-server, for example "9999"
oskari.wfs.download.email.from: The "from"-field of download basket's return e-mail, for example "noreply@company.com"
oskari.wfs.download.email.subject: The subject of download basket's return e-mail, for example "Company's download service"
oskari.wfs.download.email.header: The header of download basket's return e-mail, for example "Hello,"
oskari.wfs.download.email.message: Main e-mail message that contains the download link, for example "The data is now ready for download in: link"
oskari.wfs.download.email.footer: The footer of download basket's return e-mail, for example "The download URL is valid for 10 days"
oskari.wfs.download.email.message.datadescription: A description of the datadescription_link, for example "Metadata here:"
oskari.wfs.download.email.datadescription_link: A link to the metadata page of the operator. URL has to be valid, and it does not require a prefix word. Example "http://company.com/opendata"
oskari.wfs.download.link.url.prefix: Example: "http://localhost:9999/
oskari.wfs.cropping.namespace: A GeoServer-namespace that contains the data that can be used for cropping of the layers, for example 'cropping_areas:municipal_areas' ("cropping_areas" -workspace in GeoServer that contains polygon layers such as "municipal_areas", "provincial_areas", "states" etc.)
oskari.wfs.service.url: URL for GeoServer, when user is trying to download cropped data, for example http://localhost:9999/geoserver/wfs
oskari.wfs.error.message: Message that indicates the user that an error has occurred, for example "The service is experiencing problems, please try again later" and "This error report was generated automatically by the system"
oskari.wfs.download.error.report.support.email: E-mail address of operator's support center, for example "support@company.com"
oskari.wfs.download.error.report.subject: Topic for error report e-mail for admin, for example "Error has occurred in the download service"

Includes automatic migration to add the functionality to system default and users appsetups for the sample application. It's skipped by default, but can be activated by adding this to oskari-ext.properties (note that the sample-module must be included in db.additional.modules-property):

flyway.sample.1_0_12.skip=false

…st permissions for layers

Merged HSY development branches to HSY Oskari develop branch forked from Oskari master.

ZakarFin · 2017-08-11T08:38:15Z

Replaces PR #55

ZakarFin · 2017-08-11T08:52:44Z

control-base/src/main/java/fi/nls/oskari/control/data/GetGeoPointDataHandler.java

-
-			if (OskariLayer.TYPE_WMS.equals(layerType)) {
+			//HSY doesn't want GFI from basemaps.
+			boolean isBaseMap = false;


How about instead of this you modify https://github.com/oskariorg/oskari-server/blob/master/service-map/src/main/java/fi/nls/oskari/map/layer/formatters/LayerJSONFormatterWMS.java#L127 with

final JSONObject attrs = layer.getAttributes(); boolean disableGFI = false; if(attrs != null) { disableGFI = attrs.optBoolean(KEY_ISQUERYABLE); } JSONHelper.putValue(layerJson, KEY_ISQUERYABLE, !disableGFI && capabilities.optBoolean(KEY_ISQUERYABLE));

Then you would need to have { "isQueryable" : false } for the layer attributes instead of { "basemap" : true }. This would disable GFI requests on the frontend side automatically and the server code can be left as is? Similar logic should be done for arcgislayers. At least this needs be configurable somehow instead of always blocking for basemaps.

Couldn't resist doing it for you: 3ffdac1

And a quick fix for the previous commit: 3e7a95d

ZakarFin · 2017-08-11T10:49:22Z

service-map/src/main/java/fi/mml/map/mapwindow/util/OskariLayerWorker.java

-        } else {
+        } 
+//        else if (user.isGuest()) {
+//            JSONHelper.putValue(permission, "publish", NO_PUBLICATION_PERMISSION);


Why is this commented out?

ZakarFin · 2017-08-11T10:52:40Z

control-base/src/main/java/fi/nls/oskari/control/feature/SaveMultipleFeaturesHandler.java

+import fi.nls.oskari.wfs.WFSLayerConfigurationServiceIbatisImpl;
+
+@OskariActionRoute("SaveMultipleFeatures")
+public class SaveMultipleFeaturesHandler extends ActionHandler {


Could this be extended to function with multiple features https://github.com/oskariorg/oskari-server/blob/master/control-base/src/main/java/fi/nls/oskari/control/feature/SaveFeatureHandler.java Instead of creating a new handler that basically does the same thing. Also I would much prefer constructing the XML with a libraryor at least a template file instead of string concatenation.

File removed

ZakarFin · 2017-08-11T10:53:59Z

control-base/src/main/java/fi/nls/oskari/control/feature/SaveMultipleFeaturesHandler.java

+	            }
+			}
+			if (allUpdatesSuccess) {
+				ResponseHelper.writeResponse(params, "");


Empty response for success? Maybe find a better output

File removed

ZakarFin · 2017-08-11T10:55:35Z

control-base/src/main/java/fi/nls/oskari/control/feature/SaveMultipleFeaturesHandler.java

+			if (allUpdatesSuccess) {
+				ResponseHelper.writeResponse(params, "");
+			} else {
+				ResponseHelper.writeResponse(params, "Exception");


You should throw ActionException wrapping the original exception or an ActionParamsException if stacktrace will not give any help debugging the issue. This is not proper output for exception handling

File removed

ZakarFin · 2017-08-21T13:05:22Z

download-basket/src/main/java/downloadbasket/data/ZipDownloadDetails.java

+ * Created by markokuo on 6.10.2015.
+ */
+public class ZipDownloadDetails {
+    private String FileName = "";


Use lowercase first letter for variables

ZakarFin · 2017-08-21T13:07:09Z

download-basket/src/main/java/downloadbasket/actions/GetFeatureForCropping.java

@@ -0,0 +1,86 @@
+package downloadbasket.actions;


Maybe run an autoformat on the file so indentation is not all over the place.

Also add some comments on the class. What does this action route do?

Formatted & commented.

ZakarFin · 2017-08-21T13:19:47Z

download-basket/src/main/java/downloadbasket/actions/DownloadAll.java

+import downloadbasket.helpers.SendDownloadDetailsToEmailThread;
+import org.json.JSONArray;
+import org.json.JSONObject;
+


Add some comments what does this action route do. Maybe find a better name as well? Downloading "all" seems a bit confusing.

Added comments and changed name.

ZakarFin · 2017-08-21T13:26:10Z

download-basket/src/main/java/downloadbasket/actions/GetFeatureForCropping.java

+				        }
+				        in.close();
+
+			            JSONObject jsoni = new JSONObject(html);


Is the variable name describing the content? html in JSON?

ZakarFin · 2017-08-21T13:26:19Z

download-basket/src/main/java/downloadbasket/actions/GetFeatureForCropping.java

+				        while ((inputLine = in.readLine()) != null) {
+				        	html += inputLine;
+				        }
+				        in.close();


Move close() to finally statement

You can actually do this with less code and respecting layer credentials with:

HttpURLConnection con = IOHelper.getConnection(url, user, pass); con.setRequestProperty("Accept-Charset", "UTF-8"); final String data = IOHelper.readString(con, "UTF-8");

ZakarFin · 2017-08-21T13:28:39Z

download-basket/src/main/java/downloadbasket/actions/GetFeatureForCropping.java

+        final JSONArray data = new JSONArray();
+
+		OskariLayerService mapLayerService = new OskariLayerServiceIbatisImpl();
+		OskariLayer oskariLayer = mapLayerService.find(params.getHttpParam(PARAM_URL));


Should URL be ID?

ZakarFin · 2017-08-21T13:29:34Z

download-basket/src/main/java/downloadbasket/actions/GetFeatureForCropping.java

+		OskariLayer oskariLayer = mapLayerService.find(params.getHttpParam(PARAM_URL));
+
+		if(oskariLayer != null){
+			FINAL_WMS_URL = oskariLayer.getUrl();


Don't set class scope variables between request. Concurrent requests will be a nightmare. Use a local variable and pass it around instead.

ZakarFin · 2017-08-21T13:39:10Z

download-basket/src/main/java/downloadbasket/data/LoadZipDetails.java

+ * Created by markokuo on 6.10.2015.
+ */
+public class LoadZipDetails {
+    private String GetFeatureInfoRequest = null;


Use lowercase variable names here as well

…ver.git into develop

ZakarFin · 2017-12-08T16:43:41Z

Not sure if you are done with the changes yet or still working on this, but can you add some comments to the properties-list in the description?

Like what do they mean/control and any advice on what values one might want to use. What is the difference between oskari.wfs.download.email.message and oskari.wfs.download.email.message.datadescription. Is oskari.wfs.download.email.datadescription_link some text that's used as link-text or is it some url and does some link need a prefix (oskari.wfs.download.link.url.prefix) and why? What's the difference between
oskari.wfs.download.email.error.user.topic and oskari.wfs.download.error.report.subject?

Can "cropping" only occur on a namespace defined here oskari.wfs.cropping.namespace? What is configured by oskari.wfs.service.url and is there some reason it shouldn't be as a layer in oskari_maplayer database table?

What do these configure: oskari.wfs.download.folder.name and oskari.wfs.download.normal.way.downloads?

error-checking moved to SendDownloadDetailsToEmailThread, fixed zipdetail-files, JUnit-tests now working

zipdetail -files fixed, JUnit-tests work, error-checking in DownloadServices moved to SendDownloadDetailsToEmailThread, run-method splitted as well

to SendDownloadDetailsToEmailThread, fixed zipDetails-files, JUnit-tests work now, split run-method

into develop

jampukka · 2018-01-08T14:36:34Z

download-basket/src/main/java/downloadbasket/helpers/SendDownloadDetailsToEmailThread.java

+				ldz.setUserEmail(userDetails.getString("email"));
+				ldz.setLanguage(this.locale.getLanguage());
+				ldz.setDownloadNormalWay(normalDownloads.isBboxCropping(croppingMode, croppingLayer));
+				OskariLayerService mapLayerService = new OskariLayerServiceIbatisImpl();


Shouldn't create multiple instances of OskariLayerServiceIbatisImpl, definitely not inside a loop

jampukka · 2018-01-22T10:50:08Z

download-basket/src/main/java/downloadbasket/actions/DownloadInfo.java

+
+	private static final String PARAM_DOWNLOAD_DETAILS = "downloadDetails";
+	private static final String PARAM_USER_DETAILS = "userDetails";
+	OskariLayerService mapLayerService;


This field should be private

jampukka · 2018-01-22T10:50:19Z

download-basket/src/main/java/downloadbasket/actions/DownloadInfo.java

+		try {
+			JSONObject userDetails = new JSONObject(strUserDetails);
+			JSONArray ddArray = new JSONArray(downloadDetails);
+			mapLayerService = new OskariLayerServiceIbatisImpl();


This should be initialized only once for this ActionHandler in an overridden version of the init() function (https://github.com/oskariorg/oskari-server/blob/develop/service-control/src/main/java/fi/nls/oskari/control/ActionHandler.java#L31-L36)

jampukka · 2018-01-22T10:50:41Z

download-basket/src/main/java/downloadbasket/helpers/SendDownloadDetailsToEmailThread.java

+ * Send download details email service (thread).
+ */
+public class SendDownloadDetailsToEmailThread implements Runnable {
+	OskariLayerService mapLayerService;


All of these fields should be private (and probably final as well)

jampukka · 2018-01-22T13:23:58Z

Looks good for merging.

ZakarFin · 2018-01-22T13:29:40Z

Can you still add some more detail in the description about what this does and what bundles need to be started for the frontend? You can link to the docs in the frontend repository. Also you can just list the properties once without class references and just tell what they mean. In general tell us how to use this functionality.

ZakarFin · 2018-01-23T16:02:25Z

Tried with the zip-download from oskari.org and the following extra configuration in oskari-ext.properties:

##################################
# Download-basket configuration
##################################

# The physical path to system where downloaded zip-files should be saved, for example /server/temp/.
oskari.wfs.download.folder.name=C:/Temp
# Cropping mode parameter for WFS requests, for example "rectangle"
oskari.wfs.download.normal.way.downloads=rectangle
# The address of smtp-server, for example "localhost"
oskari.wfs.download.smtp.host=my.smtp.server
# The port of smtp-server, for example "9999"
oskari.wfs.download.smtp.port=25
# The "from"-field of download basket's return e-mail, for example "noreply@company.com"
oskari.wfs.download.email.from=noreply@company.com
# The subject of download basket's return e-mail, for example "Company's download service"
oskari.wfs.download.email.subject=Company's download service
# The header of download basket's return e-mail, for example "Hello,"
oskari.wfs.download.email.header=Hello,
# Main e-mail message that contains the download link, for example "The data is now ready for download in: link"
oskari.wfs.download.email.message=The data is now ready for download in: link

# The footer of download basket's return e-mail, for example "The download URL is valid for 10 days"
# Cron-based auto-delete recommended, not included. A link to a generated zip-archive will work as long as the file exists in 'oskari.wfs.download.folder.name'.
oskari.wfs.download.email.footer=The download URL is valid for 10 days
# A description of the datadescription_link, for example "Metadata here:"
oskari.wfs.download.email.message.datadescription=Metadata here:
# A link to the metadata page of the operator. URL has to be valid, and it does not require a prefix word. Example "http://company.com/opendata"
oskari.wfs.download.email.datadescription_link= http://company.com/opendata
# Example: "http://localhost:8080/"
oskari.wfs.download.link.url.prefix=http://localhost:8080

# A GeoServer-namespace that contains the data that can be used for cropping of the layers
#  for example 'cropping_areas:municipal_areas' ("cropping_areas" -workspace in GeoServer that contains polygon layers such as "municipal_areas", "provincial_areas", "states" etc.)
oskari.wfs.cropping.namespace=oskari

# URL for GeoServer, when user is trying to download cropped data, for example http://localhost:9999/geoserver/wfs
oskari.wfs.service.url=http://localhost:8080/geoserver/wfs

# Message that indicates the user that an error has occurred, for example "The service is experiencing problems, please try again later" and "This error report was generated automatically by the system"
oskari.wfs.error.message=The service is experiencing problems, please try again later.
# E-mail address of operator's support center, for example "support@company.com"
oskari.wfs.download.error.report.support.email=support@company.com
# Topic for error report e-mail for admin, for example "Error has occurred in the download service"
oskari.wfs.download.error.report.subject=Error has occurred in the download service

##################################
# Flyway config
##################################

# skip geoserver setting as its on the same server -> geoserver is not running when migrations are run
flyway.1_45_0.skip=true
    # add download-basket to appsetups
flyway.sample.1_0_12.skip=false

# mark any addition module property tokens so we can check/add them automagically
# NOTE! 'sample' module updates views to include new functionalities! If you have defined custom views REMOVE IT!
db.additional.modules=myplaces,analysis,userlayer,sample

I also added the code for the functionality as dependency to the webapp-map:

    <dependency>
        <groupId>fi.nls.oskari</groupId>
        <artifactId>download-basket</artifactId>
    </dependency>

Notes:

The download-basket was added to the default view as expected
Usability of the frontend code is not quite there yet. I had trouble knowing what to do with the functionality and the drawing button was some kind of a toggle that didn't reset correctly. With every other button press the drawing worked with every other it didn't.

I feel this needs some more documentation about the layers that are assumed like some WFS-layers I suppose. I tried it with the OSM worldwide WMS-layer and eventually got to a point where the frontend said that I have mail. I got a mail (formatting could be changed a bit to add spaces between sentences and removing the additional "link" text/fix the download url) :

Hello,

The data is now ready for download in: link http://localhost:8080095478da-276c-4448-b99c-c7a63863322b.zip

The download URL is valid for 10 daysMetadata here:http://company.com/opendata

I did get a new file 095478da-276c-4448-b99c-c7a63863322b.zip in the configured download folder "C:/Temp", but I guess there needs to be something that would serve the file to the user. Like an nginx or Apache with a static resource folder to serve content from the configured download folder "C:/Temp". The zip is empty and the server log had this:

2018-01-23 17:25:56,215 DEBUG downloadbasket.data.NormalWayDownloads.isBboxCropping(NormalWayDownloads.java:42) - Checked normal way download. Cropping mode: regtangle, cropping layer: . Is normal way download: false.
2018-01-23 17:25:56,222 ERROR downloadbasket.helpers.SendDownloadDetailsToEmailThread.downloadFromService(SendDownloadDetailsToEmailThread.java:122) - Cannot download shape zip. org.json.JSONException@10bad8ca[
 cause=<null>
 detailMessage=JSONObject["id"] not found.
 cause=org.json.JSONException@10bad8ca
 stackTrace={}
 suppressedExceptions=[]
]

Not really sure what went wrong. Probably that I used WMS-layers (the ones included in the sample app) or some misconfiguration.

Final thoughts:
There are a numerous issues with the functionality still and I wouldn't normally merge it until it's more polished. But this PR has been open for a long time and a lot of people are waiting to develop it further so for their sake I'm willing to merge this in hopes that this will either be more polished in a future PR or this will be moved to the community modules when we have the repository for them or both.

For properties I would suggest that:

oskari.wfs.download.smtp.host is made optional and the value would default to oskari.email.host property which is used in other mail-related stuff.
Also oskari.wfs.download.email.from could default to oskari.email.sender value. At least the mail server is probably always the same in one instance. The sender can be different I suppose, but it could use the value from oskari.email.sender if oskari.wfs.download.email.from is not defined.

Heikki Ylitalo and others added 9 commits February 16, 2017 07:03

Created SaveMultipleFeatures for saving multiple features once

366e18e

Modified the functionality of the SaveMultipleFeatures action_route.

a7afb0e

Commented out the if section in OskariLayerWorker which overrides Gue…

9c8768b

…st permissions for layers

No GetFeatureInfo from basemaps.

e2a6662

Merge branch 'develop' into NoGFIFromBaseMaps

7a2b731

Merged HSY development branches to HSY Oskari develop branch forked from Oskari master.

First set of fixes based on review

d8ac5a4

First set of fixes based on review

c489f3c

db credentials back to default

95dde8a

flyway version numbers fixed

3684dbd

kuosman mentioned this pull request Aug 11, 2017

HSY download basket #55

Closed

ZakarFin reviewed Aug 11, 2017

View reviewed changes

ollijak added 6 commits August 17, 2017 14:49

Commented rows removed as irrelevant

267e483

fix for previous commit

8a1acd4

fix #2 for previous commit

795dccf

This file has nothing to do with download basket, thus deleted

4b1e6e7

reverted to oskari code, changed LayerJSONFormatterWMS.java#L127 instead

644eddb

added download-basket folder from heikkiy:develop

1909eb3

ZakarFin reviewed Aug 21, 2017

View reviewed changes

ollijakobsson and others added 9 commits November 29, 2017 19:45

Fixed namespace to framework

66dcc03

Fixed properties reference

c3861ea

Merge branch 'develop' of https://github.com/ollijakobsson/oskari-ser…

b162bbb

…ver.git into develop

try-with-resources, IOHelper, formation, properties-rename

ec9954a

typo in properties name

aeb8a77

fixed names of properties-files.

e43af2b

fixed properties

a94351f

fixed properties

3b1a895

Merge branch 'develop' of https://github.com/ollijakobsson/oskari-ser…

6c880c3

…ver.git into develop

ollijak added 6 commits December 22, 2017 14:38

realFilename now gets the full file path,

7d7c96f

error-checking moved to SendDownloadDetailsToEmailThread, fixed zipdetail-files, JUnit-tests now working

realFilename gets full path now,

f3ae833

zipdetail -files fixed, JUnit-tests work, error-checking in DownloadServices moved to SendDownloadDetailsToEmailThread, run-method splitted as well

fixed realFilename, moved error-checking

796e396

to SendDownloadDetailsToEmailThread, fixed zipDetails-files, JUnit-tests work now, split run-method

Merge branch 'develop' of https://github.com/oskariorg/oskari-server.git

d488bfa

into develop

formation

525d880

version number, test-folder

4a35f12

jampukka reviewed Jan 8, 2018

View reviewed changes

ollijak added 2 commits January 22, 2018 12:19

OskariLayerServiceIbatisImpl removed from loop

6265798

quick fix

307fa5a

jampukka reviewed Jan 22, 2018

View reviewed changes

ollijak added 2 commits January 22, 2018 13:33

Overriding, privates and finals

38b2a43

deleted tests

66ca8a8

ZakarFin changed the base branch from develop to release/1.45.0 January 23, 2018 16:03

ZakarFin merged commit 66ca8a8 into oskariorg:release/1.45.0 Jan 23, 2018

ZakarFin added this to the 1.45.0 milestone Jan 23, 2018

HSY download basket #74

HSY download basket #74

Conversation

ollijakobsson commented Aug 10, 2017 • edited by ZakarFin Loading

ZakarFin commented Aug 11, 2017

ZakarFin Aug 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZakarFin commented Dec 8, 2017

jampukka Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jampukka commented Jan 22, 2018

ZakarFin commented Jan 22, 2018

ZakarFin commented Jan 23, 2018 • edited Loading

ollijakobsson commented Aug 10, 2017 •

edited by ZakarFin

Loading

ZakarFin Aug 11, 2017 •

edited

Loading

jampukka Jan 8, 2018 •

edited

Loading

ZakarFin commented Jan 23, 2018 •

edited

Loading