[Geoserver-devel] Pluggable styles GSIP

GeoServer GeoServer Developer
1

Hi all,

I updated the code based on feedback from last week and wrote up the proposal.

https://github.com/geoserver/geoserver/wiki/GSIP-117-Pluggable-Styles

It would be great to get this in by feature freeze but I realize I am not leaving all that much time for folks to review.

-Justin

–

Justin Deoliveira
VP Engineering | Boundless
jdeolive@anonymised.com
@boundlessgeo

2

interesting. I like the idea of possible alternatives to SLD (preferably something a lot less verbose - you can allow users to some interesting map analysis which translates down into sending a different SLD.)

However, surely the reality is going to be a lot duplication on the renderer code require for each style interpreter? I dont know the code structure, but it seems to me that you would still want some intermediate step to further decouple renderer from style. If you put in an innovation in the renderer (eg things like the curved geometries), it would not be good if you could only use them through a particular style format.

Notice: This email and any attachments are confidential.
If received in error please destroy and immediately notify us.
Do not copy or disclose the contents.

3

Quick feedback:

WMS and GetMapRequest: The STYLE_URL, STYLE_BODY, STYLE_FORMAT are all clear … do you need a STYLE_VERSION as par of this?

Feedback / Questions on implementation:

  • Would it be more clear to have SLD 1.0 and SE 1.1 each with their own StyleHandler?
  • Recommend chaining:

void encode( StyleLayerDescriptor sld, Version version, boolean pretty, OutputStream output)

StyledLayerDescriptor parse(Object input, Version version, ResourceLocator resourceLocator, EntityResolver entityResolver)

To use GeoTools Style.

void encode( Style style, Version version, boolean pretty, OutputStream output)
Style parse(Object input, Version version, ResourceLocator resourceLocator, EntityResolver entityResolver)

As far as I know we often wrap a single Style in an SLD document for output and then throw the wrapper away when we parse it back in. This wrap/unwrap could take place inside the encode and parse methods. Allowing SE 1.1 to output SE documents, and better matching our internal use of Style.

···

Jody Garnett

On Mon, Jul 14, 2014 at 4:30 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

Hi all,

I updated the code based on feedback from last week and wrote up the proposal.

https://github.com/geoserver/geoserver/wiki/GSIP-117-Pluggable-Styles

It would be great to get this in by feature freeze but I realize I am not leaving all that much time for folks to review.

-Justin

–

Justin Deoliveira
VP Engineering | Boundless
jdeolive@anonymised.com
@boundlessgeo

Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world’s largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds

Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

4

Phil perhaps it was not obvious, but this extension generates a normal GeoTools Style object (i.e. the same object we parse out of an SLD file). Thus there is no change needed to the GeoTools rendering code.

That said if you want to splice it a bit of custom rendering the GeoTools API provides a DirectLayer interface that can be used for scale bars and the like :slight_smile:

···

Jody Garnett

On Mon, Jul 14, 2014 at 4:45 PM, Phil Scadden <p.scadden@anonymised.com> wrote:

interesting. I like the idea of possible alternatives to SLD (preferably
something a lot less verbose - you can allow users to some interesting
map analysis which translates down into sending a different SLD.)

However, surely the reality is going to be a lot duplication on the
renderer code require for each style interpreter? I dont know the code
structure, but it seems to me that you would still want some
intermediate step to further decouple renderer from style. If you put in
an innovation in the renderer (eg things like the curved geometries), it
would not be good if you could only use them through a particular style
format.

Notice: This email and any attachments are confidential.
If received in error please destroy and immediately notify us.
Do not copy or disclose the contents.

Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world’s largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds

Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

5

Phil perhaps it was not obvious, but this extension generates a normal GeoTools Style object (i.e. the same object we parse out of an SLD file). Thus there is no change needed to the GeoTools rendering code.

Okay then. This was the abstraction I was looking for.

Notice: This email and any attachments are confidential.
If received in error please destroy and immediately notify us.
Do not copy or disclose the contents.

6

On Mon, Jul 14, 2014 at 5:45 PM, Jody Garnett <jody.garnett@anonymised.com>
wrote:

Quick feedback:

WMS and GetMapRequest: The STYLE_URL, STYLE_BODY, STYLE_FORMAT are all
clear ... do you need a STYLE_VERSION as par of this?

SLD_VERSION is part of the WMS 1.3 spec, so it's needed afaik, and because

of that STYLE_VERSION made sense given we are giving the rest of the
parameters generic version.

Feedback / Questions on implementation:

- Would it be more clear to have SLD 1.0 and SE 1.1 each with their
own StyleHandler?

I tried this originally and it came out much cleaner to have a single
handler, especially for the user interface.

- Recommend chaining:

void encode( StyleLayerDescriptor sld, Version version, boolean pretty,
OutputStream output)
StyledLayerDescriptor parse(Object input, Version version, ResourceLocator
resourceLocator, EntityResolver entityResolver)

To use GeoTools Style.

void encode( Style style, Version version, boolean pretty, OutputStream
output)
Style parse(Object input, Version version, ResourceLocator
resourceLocator, EntityResolver entityResolver)

Not sure I agree here. It seems safer to pass the root object around rather
than start stripping it off everywhere. I feel it is safer for client code
to make the decision whether they want to strip off the root object or not.

Also keep in mind here my goal is not to fix our style model and how it is
used, it is simply to make parsing and encoding styles pluggable. The code
that was there before worked in terms of StyledLayerDescriptor and I
followed suite.

As far as I know we often wrap a single Style in an SLD document for
output and then throw the wrapper away when we parse it back in. This
wrap/unwrap could take place inside the encode and parse methods. Allowing
SE 1.1 to output SE documents, and better matching our internal use of
Style.

Jody Garnett

On Mon, Jul 14, 2014 at 4:30 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

Hi all,

I updated the code based on feedback from last week and wrote up the
proposal.

  https://github.com/geoserver/geoserver/wiki/GSIP-117-Pluggable-Styles

It would be great to get this in by feature freeze but I realize I am not
leaving all that much time for folks to review.

-Justin

--
Justin Deoliveira
VP Engineering | Boundless <http://boundlessgeo.com/&gt;
jdeolive@anonymised.com
@boundlessgeo <http://twitter.com/boundlessgeo/&gt;

------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
Justin Deoliveira
VP Engineering | Boundless <http://boundlessgeo.com/&gt;
jdeolive@anonymised.com
@boundlessgeo <http://twitter.com/boundlessgeo/&gt;

7

WMS and GetMapRequest: The STYLE_URL, STYLE_BODY, STYLE_FORMAT are all

clear ... do you need a STYLE_VERSION as par of this?

SLD_VERSION is part of the WMS 1.3 spec, so it's needed afaik, and

because of that STYLE_VERSION made sense given we are giving the rest of
the parameters generic version.

Cool that is what I thought ... STYLE_VERSION was not listed on the
proposal page.

Not sure I agree here. It seems safer to pass the root object around

rather than start stripping it off everywhere. I feel it is safer for
client code to make the decision whether they want to strip off the root
object or not.

Also keep in mind here my goal is not to fix our style model and how it is
used, it is simply to make parsing and encoding styles pluggable. The code
that was there before worked in terms of StyledLayerDescriptor and I
followed suite.

I am not aware of any instance where we use the StyleLayerDescriptor object
in a useful fashion and would find it less risky to cut it out.
--
Jody

8

On Tue, Jul 15, 2014 at 1:30 AM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

Hi all,

I updated the code based on feedback from last week and wrote up the
proposal.

  https://github.com/geoserver/geoserver/wiki/GSIP-117-Pluggable-Styles

It would be great to get this in by feature freeze but I realize I am not
leaving all that much time for folks to review.

Works for me, +1

A question, you talk about streaming directly during a GET, can the
proposal say something about writes?
We still have people annoyed that we reformat styles posted to REST when
writing them out, will there
be any way to control that?

Cheers
Andrea

------------------------------

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

9

On Mon, Jul 14, 2014 at 6:17 PM, Jody Garnett <jody.garnett@anonymised.com>
wrote:

WMS and GetMapRequest: The STYLE_URL, STYLE_BODY, STYLE_FORMAT are all

clear ... do you need a STYLE_VERSION as par of this?

SLD_VERSION is part of the WMS 1.3 spec, so it's needed afaik, and

because of that STYLE_VERSION made sense given we are giving the rest of
the parameters generic version.

Cool that is what I thought ... STYLE_VERSION was not listed on the
proposal page.

Ahh, got it. Page updated.

Not sure I agree here. It seems safer to pass the root object around

rather than start stripping it off everywhere. I feel it is safer for
client code to make the decision whether they want to strip off the root
object or not.

Also keep in mind here my goal is not to fix our style model and how it
is used, it is simply to make parsing and encoding styles pluggable. The
code that was there before worked in terms of StyledLayerDescriptor and I
followed suite.

I am not aware of any instance where we use the StyleLayerDescriptor
object in a useful fashion and would find it less risky to cut it out.

Will mark this down as feedback in the proposal, but I wont be addressing
this right now.

--
Jody

--
Justin Deoliveira
VP Engineering | Boundless <http://boundlessgeo.com/&gt;
jdeolive@anonymised.com
@boundlessgeo <http://twitter.com/boundlessgeo/&gt;

10

On Tue, Jul 15, 2014 at 4:00 AM, Andrea Aime <andrea.aime@anonymised.com>
wrote:

On Tue, Jul 15, 2014 at 1:30 AM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

Hi all,

I updated the code based on feedback from last week and wrote up the
proposal.

  https://github.com/geoserver/geoserver/wiki/GSIP-117-Pluggable-Styles

It would be great to get this in by feature freeze but I realize I am not
leaving all that much time for folks to review.

Works for me, +1

A question, you talk about streaming directly during a GET, can the
proposal say something about writes?
We still have people annoyed that we reformat styles posted to REST when
writing them out, will there
be any way to control that?

I looked briefly at this and unfortunately I didn't think it was that

trivial. Basically we would need to side step the normal restconfig
"DataFormat" classes, but at the same time we probably still want to parse
the style to ensure it is valid. Which to do cleanly we would have to load
the entire request payload into a buffer (be it memory or file or both) so
we can read it twice. Which I guess isn't a big deal since styles are
generally not huge documents.

All in all I would prefer to defer on this for now. But I will try to find
some time in the next few days to try out a patch, and if it does turn out
to be straight forward perhaps we can consider including it.

Acceptable?

Cheers

Andrea

------------------------------

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

--
Justin Deoliveira
VP Engineering | Boundless <http://boundlessgeo.com/&gt;
jdeolive@anonymised.com
@boundlessgeo <http://twitter.com/boundlessgeo/&gt;

11

On Tue, Jul 15, 2014 at 4:26 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

I looked briefly at this and unfortunately I didn't think it was that
trivial. Basically we would need to side step the normal restconfig
"DataFormat" classes, but at the same time we probably still want to parse
the style to ensure it is valid. Which to do cleanly we would have to load
the entire request payload into a buffer (be it memory or file or both) so
we can read it twice. Which I guess isn't a big deal since styles are
generally not huge documents.

All in all I would prefer to defer on this for now. But I will try to find
some time in the next few days to try out a patch, and if it does turn out
to be straight forward perhaps we can consider including it.

Acceptable?

Sure. I was really just thinking about a "reformat" or "prettyprint" url
parameter in the request that
would be used to check if we want to just parse and check if the style is
well formed, or
if we want to take the parsed version and rewrite the style to pretty print
it while we save it
on disk (in POST/PUT requests, that is)

Cheers
Andrea
--

GeoServer Professional Services from the experts! Visit
http://goo.gl/NWWaa2 for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

12

I am not aware of any instance where we use the StyleLayerDescriptor

object in a useful fashion and would find it less risky to cut it out.

Will mark this down as feedback in the proposal, but I wont be addressing
this right now.

Thanks Justin, will mark myself down as +1 then.

I expect we will have to break this interface when SE 1.1 is made the
default, don't expect it will be a big deal (as all the code does the
unwrapping consistently right now).

Jody

13

On Tue, Jul 15, 2014 at 8:29 AM, Andrea Aime <andrea.aime@anonymised.com>
wrote:

On Tue, Jul 15, 2014 at 4:26 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

I looked briefly at this and unfortunately I didn't think it was that
trivial. Basically we would need to side step the normal restconfig
"DataFormat" classes, but at the same time we probably still want to parse
the style to ensure it is valid. Which to do cleanly we would have to load
the entire request payload into a buffer (be it memory or file or both) so
we can read it twice. Which I guess isn't a big deal since styles are
generally not huge documents.

All in all I would prefer to defer on this for now. But I will try to
find some time in the next few days to try out a patch, and if it does turn
out to be straight forward perhaps we can consider including it.

Acceptable?

Sure. I was really just thinking about a "reformat" or "prettyprint" url
parameter in the request that
would be used to check if we want to just parse and check if the style is
well formed, or
if we want to take the parsed version and rewrite the style to pretty
print it while we save it
on disk (in POST/PUT requests, that is)

OK, so what I ended up doing was adding a parameter named "raw" to

PUT/POST requests. Defaulting to false but when set to true simply streams
the style payload directly to disk:

https://github.com/jdeolive/geoserver/blob/style-plug/doc/en/user/source/rest/api/styles.rst#raw

Does that sound ok?

Cheers
Andrea
--

GeoServer Professional Services from the experts! Visit
http://goo.gl/NWWaa2 for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

--
Justin Deoliveira
VP Engineering | Boundless <http://boundlessgeo.com/&gt;
jdeolive@anonymised.com
@boundlessgeo <http://twitter.com/boundlessgeo/&gt;

14

OK, so what I ended up doing was adding a parameter named "raw" to
PUT/POST requests. Defaulting to false but when set to true simply streams
the style payload directly to disk:

https://github.com/jdeolive/geoserver/blob/style-plug/doc/en/user/source/rest/api/styles.rst#raw

Does that sound ok?

Thanks Justin, above and beyond the call of duty to fix a long standing
complaint.

If there is time I will try and update my importer patch to use pluggable
styles rather than recognise and parse an SLD sidecar file directly.

Jody

15

On Wed, Jul 16, 2014 at 5:47 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

Sure. I was really just thinking about a "reformat" or "prettyprint" url

parameter in the request that
would be used to check if we want to just parse and check if the style is
well formed, or
if we want to take the parsed version and rewrite the style to pretty
print it while we save it
on disk (in POST/PUT requests, that is)

OK, so what I ended up doing was adding a parameter named "raw" to

PUT/POST requests. Defaulting to false but when set to true simply streams
the style payload directly to disk:

https://github.com/jdeolive/geoserver/blob/style-plug/doc/en/user/source/rest/api/styles.rst#raw

Does that sound ok?

Not ok, excellent

Cheers
Andrea

--

GeoServer Professional Services from the experts! Visit
http://goo.gl/NWWaa2 for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

16

On Wed, Jul 16, 2014 at 10:24 PM, Andrea Aime <andrea.aime@anonymised.com>
wrote:

On Wed, Jul 16, 2014 at 5:47 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

Sure. I was really just thinking about a "reformat" or "prettyprint" url

parameter in the request that
would be used to check if we want to just parse and check if the style
is well formed, or
if we want to take the parsed version and rewrite the style to pretty
print it while we save it
on disk (in POST/PUT requests, that is)

OK, so what I ended up doing was adding a parameter named "raw" to

PUT/POST requests. Defaulting to false but when set to true simply streams
the style payload directly to disk:

https://github.com/jdeolive/geoserver/blob/style-plug/doc/en/user/source/rest/api/styles.rst#raw

Does that sound ok?

Not ok, excellent

Thanks!

So I'll assume that is a +1 on the proposal we have 3 +1's. Anyone else
care to vote?

Cheers
Andrea

--

GeoServer Professional Services from the experts! Visit
http://goo.gl/NWWaa2 for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

--
Justin Deoliveira
VP Engineering | Boundless <http://boundlessgeo.com/&gt;
jdeolive@anonymised.com
@boundlessgeo <http://twitter.com/boundlessgeo/&gt;

17

Yes, +1 from me. I will be very interested to see where this goes.

Notice: This email and any attachments are confidential.
If received in error please destroy and immediately notify us.
Do not copy or disclose the contents.

18

+1

-Jukka Rahkonen-
________________________________________
Phil Scadden wrote:

Yes, +1 from me. I will be very interested to see where this goes.

Notice: This email and any attachments are confidential.
If received in error please destroy and immediately notify us.
Do not copy or disclose the contents.

------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

19

+0

Regards,
Simone Giannecchini

GeoServer Professional Services from the experts! Visit
http://goo.gl/NWWaa2 for more information.

Ing. Simone Giannecchini
@simogeo
Founder/Director

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 333 8128928

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

On Fri, Jul 18, 2014 at 3:48 PM, Rahkonen Jukka (Tike)
<jukka.rahkonen@anonymised.com> wrote:

+1

-Jukka Rahkonen-
________________________________________
Phil Scadden wrote:

Yes, +1 from me. I will be very interested to see where this goes.

Notice: This email and any attachments are confidential.
If received in error please destroy and immediately notify us.
Do not copy or disclose the contents.

------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel