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 <[email protected]> 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
>
>
> On Fri, Jul 14, 2017 at 9:04 PM, Torben Barsballe <
> [email protected]> 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
>>
>>
>> On Fri, Jul 14, 2017 at 11:15 AM, Torben Barsballe <
>> [email protected]> wrote:
>>
>>> Part 2:
>>>
>>> On Fri, Jul 14, 2017 at 9:37 AM, Andrea Aime <
>>> [email protected]> wrote:
>>>
>>>> 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:
>>>>
>>>> [image: Inline image 1]
>>>>
>>>>
>>>> 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:
>>>>
>>>> http://demo.geo-solutions.it/geoserver/wms?service=WMS&versi
>>>> on=1.1.0&request=GetMap&layers=test&styles=&bbox=-180.0,-90.
>>>> 0,180.0,90.0&width=768&height=384&srs=EPSG:4326&format=appli
>>>> cation/openlayers
>>>> http://demo.geo-solutions.it/geoserver/geosolutions/wms?serv
>>>> ice=WMS&version=1.1.0&request=GetMap&layers=test&styles=&bbo
>>>> x=-180.0,-90.0,180.0,90.0&width=768&height=384&srs=EPSG:4326
>>>> &format=application/openlayers
>>>> http://demo.geo-solutions.it/geoserver/topp/wms?service=WMS&;
>>>> version=1.1.0&request=GetMap&layers=test&styles=&bbox=-180.0
>>>> ,-90.0,180.0,90.0&width=768&height=384&srs=EPSG:4326&format=
>>>> application/openlayers
>>>>
>>>> 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?
>>>>
>>>> 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.
>>>
>>> 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]? :-)
>>>>
>>>> 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.
>>>
>>>
>>>> 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?
>>>>
>>>> You mentioned this in your earlier mail, and I think that would be a
>>> perfectly workable alternative to an actual StyleGroup object.
>>>
>>>
>>>> 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.
>>>>
>>>> ------------------------------------------------------------
>>>> ------------------
>>>> 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
>>>> [email protected]
>>>> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>>>>
>>>>
>>>
>>
>> ------------------------------------------------------------
>> ------------------
>> 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
>> [email protected]
>> 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 <+39%200584%20962313> fax: +39 0584 1660272
> <+39%200584%20166%200272> mob: +39 339 8844549 <+39%20339%20884%204549>
> 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.
>
>
> ------------------------------------------------------------------------------
> 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
> [email protected]https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>
>
> --
> Kevin Michael Smith<[email protected]> <[email protected]>
>
>
> ------------------------------------------------------------
> ------------------
> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>
>
------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
