HI Torben,
thanks a lot for the update. There is only one thing giving me pause as it
read it:
"If a style group refers to an invalid layer, it should just drop 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)."
Is it really working that way, if someone manages (probably due to a bug)
to create a pending reference to
a missing layer or style, then you get no exceptions?
I'd prefer an exception being though (fail early), but if you have
compelling arguments not to go that way, then
at least a log at SEVERE level?
Silent failure would make it hard to figure out.
One more little detail, mostly just a reminder, style treatment in
GeoServer is pretty lax (validation being optional),
so be prepared to find also styles with no named layer reference at all, or
invalid UserLayer structures.
Right, what happens if someone actually put in a style a _valid_ UserLayer,
with a locally defined set of features?
Cheers
Andrea
On Tue, Jul 25, 2017 at 12:49 AM, Torben Barsballe <
[email protected]> wrote:
> 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 <
> [email protected]> 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 <[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
>
>
--
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.
------------------------------------------------------------------------------
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
