[Feature] Use remote WebDAV server to store and retreive files

[mind84]

Provide on LWC the functionality of the Attachment Widget's WebDav Storage property, according to the widget documentation

Funded by Faunalia

[mdouchin]

Thanks a lot for this contribution. We will take time to audit the code. Not sure if it can land in 3.7, as we would like to release in a very short term

[mind84]

Thanks a lot for this contribution. We will take time to audit the code. Not sure if it can land in 3.7, as we would like to release in a very short term

Thanks to you, this is an early stage development and contains only the basic structure of the desired feature. It can be a start point

I'm available for any questions

Cheers!

[pcav]

Any plans to push this in the next LM release? We can patch it, but it would be nice (and I believe useful for others) to have in stable version.

[nworr]

Look good to me, some small code improvements can be done. (see suggestion)

[Gustry]

Regarding the last comment on the PR, in the timeline

Thanks to you, this is an early stage development and contains only the basic structure of the desired feature. It can be a start point

So I think now, it's more than "early stage development" ? Can you just confirm that it's ready for a production use now ? Or any known bullet list to check ?

I will checkout the PR, thanks for this work.

[mind84]

So I think now, it's more than "early stage development" ? Can you just confirm that it's ready for a production use now ? Or any known bullet list to check ?

I will checkout the PR, thanks for this work.

There are still a couple of things that need to be done:

• hide webdav url for getMedia
  if the layer is a "postgre layer", Lizmap collects the information directly from the db so it is possible via php to modify the returned url and prepare it for viewing by the client. Otherwise, the feature data is collected with the getfeatureQgis method. If I'm not mistaken, this function forwards the request to qgis-lizmap-server-plugin so I left out this part thinking that the best way to hide the URL is to modify the server plugin

• displaying url in Popup
  basically the same for the previous point, HTML of the popup is generated server-side and I left out this part for the same reasons

Do you think that resolving this two points by intercepting the qgis-lizmap-server-plugin response via php could be fine?

Initially I had discarded this option because I thought it was more correct to make the changes on the server plugin.

[Gustry]

-- Note, I started the review and screenshots before your previous comment about known bottlenecks. I would be nice to write explicitly what is still a work in progress, before we start a review. I didn't edit my comment below, that I started before your comment ---

I just give it a try :

• Minor note, maybe in the your new test tables, I would add some data with a webdav data in the attribute table ? I would make it easier for testing ? (for attribute table and popup tests) I have noticed you always start playwright test with addition of new features ? Just an idea
• The popup doesn't have a proper link to the new GetMedia, it's leading to http://webdav/ksnip_20231017-111339.png which obvously giving an error in web-browser when clicking on the link in the popup

it's the first think I tried, create a new feature with a webdav picture, open on it's popup... This is a blocker I think.

The data from WFS has the raw URL, I think all data from LWC should be a proxy to hide the webdav server

If I understood correctly, data can only be stored in the root directory of the web dav server ? Because of the QGIS expression used for now ?

Sorry, I might other tests coming

[mind84]

-- Note, I started the review and screenshots before your previous comment about known bottlenecks. I would be nice to write explicitly what is still a work in progress, before we start a review. I didn't edit my comment below, that I started before your comment ---

Right, sorry for the lack of information

I just give it a try :

* Minor note, maybe in the your new test tables, I would add some data with a webdav data in the attribute table ? I would make it easier for testing ? (for attribute table and popup tests) I have noticed you always start playwright test with addition of new features ? Just an idea

Yes, but in this case during application building some files should be moved in webdav folder right? In this case can you point me the files I should update?

* The popup doesn't have a proper link to the new GetMedia, it's leading to `http://webdav/ksnip_20231017-111339.png` which obvously giving an error in web-browser when clicking on the link in the popup

it's the first think I tried, create a new feature with a webdav picture, open on it's popup... This is a blocker I think.

Yes, as pointed out, this is a work in progress to discuss together

The data from WFS has the raw URL, I think all data from LWC should be a proxy to hide the webdav server

Yes, as pointed out, this is true for non postgre layers. Work in progress same as the previous point

If I understood correctly, data can only be stored in the root directory of the web dav server ? Because of the QGIS expression used for now ?

No, you can store data also in subfolders.
If you check out the test project tests/qgis-projects/tests/form_upload_webdav.qgs the layer "_geom" stores the files in the root directory, the other layer stores the files in the remoteData subfolder.
The storageUrl must include the expression file_name(@selected_file_path)

Sorry, I might other tests coming

Thank you

[Gustry]

Yes, but in this case during application building some files should be moved in webdav folder right? In this case can you point me the files I should update?

Why not committing just a text file and a very small picture in ./qgis-projects/webdav/test ? (just add it in the gitignore to be able to commit) and fill the data in the SQL table like you did with other fields.

Yes, as pointed out, this is a work in progress to discuss together

I think, popup is blocker (whatever the mode : automatic, QGIS HTML maptip, QGIS form) to have a proper replacement, the popup feature is a common feature, after creation of a new feature in the table. Users are expecting to see the data they just added.

Yes, as pointed out, this is true for non postgre layers.

I will let my colleague answer, I don't know exactly the path in the PHP source code, but in PHP, there should be a place where data is replaced. (not JS, so it work in QGIS with WFS stream, and making a replacement in JS side is not secure) for all kind of requests (WFS attribute table, WMS GetFeatureInfo for popup...) whatever the backend.

Indeed with PG layers, lizmap indeed tries if possible to make a straight SQL query, but if for any reason it fails, it will still use the WFS service from QGIS server (because we know for sure QGIS server can access the data). And QGIS server is used for all other backend indeed. And Lizmap QGIS server plugin is not involved in this part. (only for the GetFeatureInfo, to have the "form" popup about Drag&Drop design).

Side note, for playwright, I would avoid pixel comparaison, even if you are using the same picture as upload, so it should work ? But it seems plawyright tests are failing on that ? I would just check HTTP code 200, maybe just mime type. Pixel comparaison is very fragile

I hope I will have some insights from @mdouchin

[mind84]

@mdouchin,

I didn't mark as resolved the last conversation since I'm not sure if the changes are what you are expected.

A couple of question:

• the WFS XML request is forwarded to the function getfeatureQgis before $this->qgisVectorLayer is set. I ended up not parsing the WFS response in this case, but I'm not sure if this is the right solution. Alternatively, is it safe to get a local instance of qgisVectorLayer to get its webdav configuration?

• as suggested by @laurentj, it might be useful to set a maximum upload file size. In this case, this parameter should be configurable to provide some flexibility to users. Do you think it could be set in profiles.ini.php in the webdav configuration? If maxUploadFileSize is not set, then fall back to a default value (2 MB?)

Thanks

[laurentj]

it might be useful to set a maximum upload file size. this parameter should be configurable to provide some flexibility to users

It can be, however, keep in mind that there is a also a such limit into the PHP configuration (upload_max_filesize and post_max_size), and so this parameter cannot be higher than this limit, and the PHP limits cannot be changed by Lizmap.

[rldhont]

Thanks @mind84 !

[Gustry]

I just tried the PR in QGIS desktop and in the docker stack.
Looks good to me and thanks for taking care of previous comments and rollback !

Just some comments/suggestions. Related to what need to be documented as well.

[mdouchin]

@mind84 do you prefer that we merge your commits "as is" (i.e. add them all to the master branch with an additional merge commit), or you would prefer to reorganise/merge/rename your commits ?

[mind84]

do you prefer that we merge your commits "as is" (i.e. add them all to the master branch with an additional merge commit), or you would prefer to reorganise/merge/rename your commits ?

@mdouchin,
Makes no difference to me, I'll follow your indications.

Thanks

[mdouchin]

Thanks a lot