Hello All,
I have updated the proposal based on feedback:
- Instead of a new StyleGroupInfo object, just use a null entry in the layers list of a layergroup
- Added a section on new Validation steps for style groups / named layers in styles, both when uploading such styles, and when layers get renamed or deleted.
If anyone has further feedback, it would be appreciated. Otherwise, it might be time to move on to voting?
Torben
···
On Fri, Jul 14, 2017 at 3:44 PM, Torben Barsballe <tbarsballe@anonymised.com> wrote:
Giving the user a choice about whether or not to rewrite the style addresses most of the concerns I had about that approach. Given this, I would be okay with an implementation that rewrites the style by default, with the option to leave it unchanged.
Allow invalidation of style, rewrite style (Modify data structure write out if the language supports it), replace style (keep the old one, write a new one and update all references to use the new one), cascade delete (delete anything that would become invalid), cascade disable
This seems like maybe a few too many options, but some subset of these would be good.
In the UI, a warning could show up and allow the user to pick a resolution strategy maybe with a documentation link to explain their strengths and weaknesses.
In REST we would have a parameter on the operation specifying the strategy. I think the safe default would be to deny the operation if it would invalidate the style.
Sounds good, this closely matches the current behaviour of validate by default, but allow a raw if a certain parameter is passed.
I also like the suggestion to preserve the old style, as that helps address the case of large complex styles with complex internal formatting / indentation / comments. That does open a couple cans of worms with respect to cluttering the style directory, and renaming the preserved styles, but both these issues seem easy enough to solve.
Torben
On Fri, Jul 14, 2017 at 3:16 PM, Kevin Smith <smithkm@anonymised.com> wrote:
Maybe we could allow a choice of multiple strategies for handling operations that invalidate styles:
Allow invalidation of style, rewrite style (Modify data structure write out if the language supports it), replace style (keep the old one, write a new one and update all references to use the new one), cascade delete (delete anything that would become invalid), cascade disable (disable anything that would become invalid)
In the UI, a warning could show up and allow the user to pick a resolution strategy maybe with a documentation link to explain their strengths and weaknesses.
In REST we would have a parameter on the operation specifying the strategy. I think the safe default would be to deny the operation if it would invalidate the style.
For rewrite/replace adding a comment explaining what happened to the new style could be helpful. This would probably require an extension to the style language API though.
On 7/14/17 12:59 PM, Andrea Aime wrote:
Hi Torben,
we sort of agree on a few points, besides one major disagreement, which is the handling of
configuration consistency.This statement is provably false: “My preference would be to allow invalid styles caused by catalog changes. This is fairly similar to the current treatment of styles in GeoServer”
Any attempt to change a layer name, a store workspace, or remove it, is today followed by a warning and an automatic re-alignment of the configuration on save.
Any security application is completely transparent to the group configuration too.Yes, it’s possible to make a style not work with a layer, just remove an attribute that the style is using, or fumble and associate
the wrong style to a layer, but this is not a 0 or 1 situation, the current system does a significant effort trying to keep things
consistent (and could do better, I agree), this new one would do nothing at all instead, in other words, it’s anything but transparent and/or automatic.Now, you say it’s an extension of the current SLD/SLD_BODY behavior, and there, I completely agree.
However, SLD/SLD_BODY is non discoverable by the novice/normal user, and not so very well documented… it smells of advanced (here be dragons) a mile away.
Putting style groups in the UI makes them be “front and center”, discoverable and usable by everybody, more rope for the common users to hang themselves,
and annoying for the advanced user that (god forbid) might have to change a layer name (ever made a typo? ever asked by a manager for a reorganization?)
and then has to hunt it down in all styles referring to it. It just cries out for automation.And automation is indeed hard, I completely agree again, as it would require writing a plugin system to do the right thing for each styling language supporting NamedLayer
(btw, CSS would not be one of them, at least not today).I’d be quite a bit more comfortable with the vendor params/empty layer approach in GetMap/GetFeatureInfo instead, as in that case
it would be a true-er parallel to the current SLD/SLD_BODY functionality, less “front and center”, more geared towards the advanced user
(although I’m sure we’ll get complaints about the lack of automatic updates of the styles on layer name changes even in this case,
but hopefully a smaller amount).In its current state, I would advise against the proposed approach, but if everybody else unanimously supports it, I’ll vote -0 to
avoid blocking the proposal.Cheers
Andrea------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! [http://sdm.link/slashdot](http://sdm.link/slashdot)_______________________________________________ Geoserver-devel mailing list [Geoserver-devel@anonymised.comrge.net](mailto:Geoserver-devel@lists.sourceforge.net) [https://lists.sourceforge.net/lists/listinfo/geoserver-devel](https://lists.sourceforge.net/lists/listinfo/geoserver-devel)-- Kevin Michael Smith [<smithkm@anonymised.com...>](mailto:smithkm@anonymised.com)
Check out the vibrant tech community on one of the world’s most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Geoserver-devel mailing list
Geoserver-devel@anonymised.comrge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
On Fri, Jul 14, 2017 at 9:04 PM, Torben Barsballe <tbarsballe@anonymised.com> wrote:
Summary:
Suggestion - use an empty layer entry in the layers list of the Layer Group, instead of adding the StyleGroupInfo class:
This seems a little hacky, but I don’t see any real problems with it.
Overall results in a smaller API change, so I am happy with this suggestion.Style Validation - Deleting or renaming layers referred to by StyleGroups:
Overall, I would prefer going with the current prevalent approach of leaving most of the onus of keeping the style valid upon the user. I think adding warnings to the UI where deleting or renaming a layer would make a StyleGroup invalid would definitely be valuable.
I agree preventing deletion of a layer referred to by a style group is unacceptable.
I think modifying a style group programatically when layers are renamed is a bad idea, for a number of reasons:
- There are several different styling languages supported by GeoServer, and some of them support NamedLayer. Most of these are not 1:1 compatible with SLD, which is to say writing from SLD to one of these layers is not always possible, or has the potential to cause destructive changes (if only to comments or structure, not behaviour).
- In general, programatically modifying a user’s style seems like a very bad idea, especially since even “SLD file” → “parsed SLD in GeoServer” → “SLD file” chain does not always result in something exactly the same.
My preference would be to allow invalid styles caused by catalog changes. This is fairly similar to the current treatment of styles in GeoServer.Handling Virtual Services / Security:
Current behaviour for external SLDs is to fail with a “layer not found”. For a Style Group, I would prefer to implement a softer failure of just dropping the layer from the output, to match with the current treatment of layer groups. It may be valuable to extend this implementation to the treatment of external styles as well (Perhaps as a configurable option if this is necessary to remain compliant with the SLD / WMS spec).
Alternative approach - just use the WMS SLD parameter (extend it to support referencing local styles by name) instead of modifying layer group behaviour:
I would rather extend the layer group functionality, as it allows combining Style Groups with regular layers. However, I think extending the SLD parameter would still be a workable alternative.
Torben
Check out the vibrant tech community on one of the world’s most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Geoserver-devel mailing list
Geoserver-devel@anonymised.comrge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
–
Regards,
Andrea Aime
== GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 http://www.geo-solutions.it http://twitter.com/geosolutions_it
AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
Le informazioni contenute in questo messaggio di posta elettronica e/o nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il loro utilizzo è consentito esclusivamente al destinatario del messaggio, per le finalità indicate nel messaggio stesso. Qualora riceviate questo messaggio senza esserne il destinatario, Vi preghiamo cortesemente di darcene notizia via e-mail e di procedere alla distruzione del messaggio stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso, divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità diverse, costituisce comportamento contrario ai principi dettati dal D.Lgs. 196/2003.
The information in this message and/or attachments, is intended solely for the attention and use of the named addressee(s) and may be confidential or proprietary in nature or covered by the provisions of privacy act (Legislative Decree June, 30 2003, no.196 - Italy’s New Data Protection Code).Any use not in accord with its purpose, any disclosure, reproduction, copying, distribution, or either dissemination, either whole or partial, is strictly forbidden except previous formal approval of the named addressee(s). If you are not the intended recipient, please contact immediately the sender by telephone, fax or e-mail and delete the information in this message that has been received in error. The sender does not give any warranty or accept liability as the content, accuracy or completeness of sent messages and accepts no responsibility for changes made after they were sent or for other risks which arise as a result of e-mail transmission, viruses, etc.
On Fri, Jul 14, 2017 at 11:15 AM, Torben Barsballe <tbarsballe@anonymised.com> wrote:
Part 2:
On Fri, Jul 14, 2017 at 9:37 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:
Good question. My first response would be what do we do currently if you use an external SLD via the WMS SLD parameter while making a WMS request from a workspace specific service.
After trying it out, it handles it as if the layer doesn’t exist, and fails with org.geoserver.platform.ServiceException: Unknown layer: poi.
This is a bit more brittle than I would like, but it does at least seem to respect security and virtual service restrictions. It may be better to change this behaviour to simply exclude layers that can’t be found (and log a warning), although this might be something that can be controlled by an additional parameter, so that we can still treat missing layers in a more strict fashion.
For the StyleGroup, I would expect it would trim out any layers when parsing the SLD. The current code for handling the SLD parameter (In GetMapKVPRequestReader / GetMapXMLRequestReader) already splits each NamedLayer within the SLD into separate MapLayer and Style objects, so I expect it would be possible to trim out any layers that would be hidden to to workspace-specific services or security rules at this point.
It mostly the latter, although it would still be useful to be able to refer to regular layers and StyleGroups in the same WMS, such as if you had a style group that defines a basemap, and wanted to show some additional layers on top of it.
It is actually already possible to pass a URL into the SLD parameter, and you can use a GeoServer-internal style this way via the styles endpoint (localhost:8080/geoserver/styles/stylename.sld). Adding the ability to refer to an internal style by name / relative URL would be a simple improvement, and a potential alternative.
You mentioned this in your earlier mail, and I think that would be a perfectly workable alternative to an actual StyleGroup object.
Check out the vibrant tech community on one of the world’s most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Geoserver-devel mailing list
Geoserver-devel@anonymised.comrge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Ah, forgot one more concern, which is related to workspace specific services.
I setup an example group on a demo server for you to look at, it’s a globa one called test,
referring two layers in different workspaces:
Nowadays the usage of test is allowed from both global and workspace specific services, and the
catalog will automatically shave off the layers that are not part of the current workspace
transparently, please see and compare:
What happens when style groups enter the picture? Is the catalog going to alter the style
on the fly to remove references to layers that are not visible in the current workspace?
And the same should happen with security, if a layer is not visible and referenced by a group today, the
layer is removed transparently from the group as it gets out of the catalog, I assume the same would
have to happen for nested style groups, right?
I’m also wondering, is nesting style groups in standard groups a requirement for you, or just a way to
make style groups viewable without making them catalog first class objects?
If it’s the latter, wouldn’t it be simpler to extend the current GetMap protocol with a new parameter that
would mimick SLD and SLD_BODY, but so that you can refer to an internal style known by GeoServer?
Hell, wouldn’t it be even simpler to allow SLD to take either a URL, or a style name, and be done with it [1]? 
If you need to mix it with other layers, how about allowing an empty layer name in the layer list,
and if that happens then take the style name a group?
Just thinking out loud here!
Cheers
Andrea
[1] Well, almost be done, even using a style like this would incur in security and more in general
catalog filtering related issues.
