Ok,
more substantive review this time.

So, StyleGroup is really a link to a multi-layer SLD.

The statement "StyleGroupInfo will not be stored as catalog objects" threw
me off a few times, I believe that what you mean is that
StyleGroupInfo is not a top level catalog object, so it has no identity,
and cannot be searched directly from a catalog object,
but can only be found by scanning the containing group (I had no
familiarity with LegendInfo so could not really see what you meant there
at a first read).

Making it a PublishedInfo will cause some confusion in Catalog, as one can
search for PublishedInfo in the list method
of the Catalog, e.g. in the preview page we have:

 CloseableIterator<PublishedInfo> pi = catalog.list(PublishedInfo.class,
filter, (int) first, ...

Since this thing exist only in a group, does it make sense to list it?
Basically a style is just a style, but becomes a group
the moment it's being referred as one in another layer group... and you say
it can be chosen and previewed in the style editor
(but really, what you're doing there is previewing the current style no?)
but when you refer to it from the layer preview, then it can only be
previewed through the containing group?
My head is starting to spin.

Then I see the rest representation, and it really looks like what you're
after there is a empty layer slot with
a style attached to it, and when that happens, then the style is to be used
as as a sub-group definition.

Before I make a counter proposal, one more thing, I have a patch sitting on
my disk allowing someone to
preview a group while editing a style, because it's common to have to look
at a whole map while tweaking
a single style of it.

With this in mind, wouldn't it be simpler to just allow the layer group to
have an empty layer entry, and when
that happens, then make the style spec compulsory and use it as the source
of layers, without the need
to be talking about a style group to start with?
If you modify the LayerGroupHelper to perform the style expansion when no
layer is found, most of the
current code should be working already.

In the style editor, then also allow a "preview style as a group"
link/option, that would activate
after checking that the style is indeed referring to layers (existing ones).

Now, some other concerns the proposal is not covering.
Using GetMap with SLD or SLD_BODY the code will perform a few consistency
checks (assuming you're not entering library mode, mind), in particular,
that all layers referred to by name are there.
However, nowadays most styles are not referring to layers, or are referring
to layers that do not exist, so some checking
should be done, either before or after the fact, to ensure a style used as
a group definition is indeed suitable for the job.

Also, taken into consideration deletion and cascading deletion via UI and
REST. Nowadays if a layer gets removed
groups containing it are found and listed in a warning dialog, and on
confirmation they are updated removing the
layer.
If a layer is referred by a style group, what do we do? Update the style?
Disallow the deletion?
Or allow the catalog to get into an inconsistent state? The latter does not
seem acceptable to me, as the current behavior does better than that.

Also, nowadays references to layers are done via ID, which is an immutable,
but style groups would be referring layer by
name, which is mutable instead. So a new set of checks should be enacted,
making sure that when a store changes
workspace, or a layer gets renamed, the style groups referring to it are
modified.
I see no other option here, as disallowing the change would require the
user to first remove the reference from the style,
then modify the store/layer, and then go back and add the new name in the
style.

One final note on the editing of such styles. Nowadays when validating a
style we just check it's valid in generic
terms, that it can be parsed. But binding it to a layer seems to suggest
two more validations could be performed:

   - Check that the layer exists
   - Check the attributes referred from the styles are usable with that
   layer

I say could because at least the second one could also be performed when
associating a layer to a style, and
nowadays it's not done. So don't take this a requirement.

Cheers
Andrea


On Tue, Jul 11, 2017 at 6:55 PM, David Vick <[email protected]> wrote:

> https://github.com/geoserver/geoserver/wiki/GSIP-161
>
> We have submitted a GeoServer Improvement Proposal for adding the ability
> to define LayerGroups through a SLD.  Currently this can be achieved by
> using external SLD's and we propose to enable GeoServer to create/publish
> LayerGroups from it's styles stored in the catalog.
>
> Looking forward to your comments.
>
> David Vick
>
> Professional Services Engineer | Boundless <http://www.boundlessgeo.com>
> [email protected]
> mobile: +1-636-698-3174 <(636)%20698-3174>
>
>
> ------------------------------------------------------------
> ------------------
> 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

Reply via email to