[Geoserver-devel] how we handle styles in configuration

[Justin_Deoliveira]

Hi all,

In working on the style editor in thew new UI I have once again run up against the issue of how we handle styles in configuration. I wanted to sum up my thoughts and get others peoples feedback about this.

So first off, if you look at StyleInfo you will see:

org.geotools.styling.Style getStyle();

The Style object corresponds to the UserStyle element in SLD. This is the first oddity. We store the full StyledLayerDescriptor on disk but only model a single UserStyle.

Where i am finding this problematic is in the style editor. Where we want to present the user will thefull SLD to edit. To do this we more or less have to bypass the style that is actually on the StyleInfo and load the underlying file from disk.

And saving out is problematic as well. With all other configuration objects we simply make the change on the configuration object and call save, and that gets transparently persisted for us. But this is a problem for styles for a couple of reasons:

1) just persisting what the StyleInfo.getStyle() represents loses information. Basically everything outside of the first UserStyle element.

2) It would change the structure of the actual XML/SLD. For instance anything that the style parser did not pick up and represent in memory would be lost. Comments, etc... Currently we preserve that, and for good reason.

So the workaround has always been to have the UI read and write styles directly from the data directory. With the old UI this was less of an issue. Because when the style page was submited and applied, every configuration object was reloaded, so the styles stayed in sync. Now the user interface has to keep the StyleInfo.getStyle() in sync manually... but said, there is no setStyle(Style) method.

Also, what about other code that wants to update styles, like say, the REST config stuff. It too has to know about these workarounds.

This represents a problem with how we model styles imho. There are other issues as well looking to the future. There has been talk of supporting other types of styles that are non SLD, like support for cascadnik. Our current model there would really be no way to handle this cleanly.

I am curious to hear other peoples thoughts on this. And if other people think that a retrofit of how we model styles is needed.

-Justin

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

[Andrea_Aime3]

Justin Deoliveira ha scritto:

Hi all,

In working on the style editor in thew new UI I have once again run up against the issue of how we handle styles in configuration. I wanted to sum up my thoughts and get others peoples feedback about this.

So first off, if you look at StyleInfo you will see:

org.geotools.styling.Style getStyle();

The Style object corresponds to the UserStyle element in SLD. This is the first oddity. We store the full StyledLayerDescriptor on disk but only model a single UserStyle.

Where i am finding this problematic is in the style editor. Where we want to present the user will thefull SLD to edit. To do this we more or less have to bypass the style that is actually on the StyleInfo and load the underlying file from disk.

And saving out is problematic as well. With all other configuration objects we simply make the change on the configuration object and call save, and that gets transparently persisted for us. But this is a problem for styles for a couple of reasons:

1) just persisting what the StyleInfo.getStyle() represents loses information. Basically everything outside of the first UserStyle element.

2) It would change the structure of the actual XML/SLD. For instance anything that the style parser did not pick up and represent in memory would be lost. Comments, etc... Currently we preserve that, and for good reason.

So the workaround has always been to have the UI read and write styles directly from the data directory. With the old UI this was less of an issue. Because when the style page was submited and applied, every configuration object was reloaded, so the styles stayed in sync. Now the user interface has to keep the StyleInfo.getStyle() in sync manually... but said, there is no setStyle(Style) method.

Also, what about other code that wants to update styles, like say, the REST config stuff. It too has to know about these workarounds.

This represents a problem with how we model styles imho. There are other issues as well looking to the future. There has been talk of supporting other types of styles that are non SLD, like support for cascadnik. Our current model there would really be no way to handle this cleanly.

I am curious to hear other peoples thoughts on this. And if other people think that a retrofit of how we model styles is needed.

I agree on the sentiment, however not sure how to provide a solution.
Let me start with two lower handing fruits that I have some more
definite ideas on.

SLD storage wise, I too believe the file should only contain a Style
element, and not an SLD one. A full SLD is meant to operate in the
context of a WMS request, providing the definition of a map (in terms
or layers and styles), or acting as a style library.
Having people deal with anything other than just Style is indeed
only source of confusion.
So I would be very much +1 changing the parsing to deal with Style
only (but keep on reading StyledLayerDescriptor for backwards
compatibility) and change our demo material to just use a Style.

CSS wise (a nice SOC topic btw, just added it to the page), I'd
say we'd just have to parse the css definition in to a Style
object, so API wise using a GT2 Style would not change.

Yet, what would change is the way we parse, store, and present
the style to the user. Adding pluggable parser is not that
hard. The UI can be made pluggable as well.
To support UI to disk and back I guess we'll need a style
store extension point?

For sure we cannot use the parsed version in neither case,
as you have noticed, keeping the formatting and comments
the user made in the source is very important.

So, let's try to sum up a possible action plan:
- add a style type field to StyleInfo
- add methods in StyleInfo to get and set the raw
   style form (as i/o streams?)
- have a storage facility that recognizes the type
   and knows what file to look for
- have a pluggable parser that knows how to parse
   the contents coming from the storage into a GT2
   Style object.

Hmm... actually the storage could be really dumb,
just know what the file name is and decouple
the parser from disk (database?) access.

Opinions?
Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

[Justin_Deoliveira]

<snip>

So, let's try to sum up a possible action plan:
- add a style type field to StyleInfo
- add methods in StyleInfo to get and set the raw
  style form (as i/o streams?)
- have a storage facility that recognizes the type
  and knows what file to look for
- have a pluggable parser that knows how to parse
  the contents coming from the storage into a GT2
  Style object.

I am a little hesitant to start putting methods for doing I/O directly on the StyleInfo interface. I would like to try to keep config objects a s java bean like as possible.

But I think its a good idea to add those methods. What about putting them on ResourcePool? StyleInfo could still be a call through to ResourcePool, but client code wanting to change a style would have to go through RP.

Hmm... actually the storage could be really dumb,
just know what the file name is and decouple
the parser from disk (database?) access.

Opinions?
Cheers
Andrea

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

[Gabriel_Roldan2]

Hi,

I just went quickly over the topic so I guess I understand the problem but may not be that insightful about a proposal for a solution. Yet, let me just chip in before I forget.

I guess you said we may want to leverage other sorts of styles definitions than ogc's ones, which seems clever to me.
To do that my first reaction would be to use some way the IAdaptable pattern in StyleInfo. Something on the lines of
StyleInfo.canAdapt(Class styleClass) and StyleInfo.getStyle(Class<T> styleClass):T

Implementation wise we can allow the factory that creates a styleinfo receive a String and held it internally. It may well be the SLD contents or any other thing and we can store it in the database verbatim if needed or as a file. So no matter if we store an SLD or a single SE style or some other sort of style definition, it could evolve to support other formats easily?

ok, it may be an off topic or simple stupid idea but wanted to tell what my though was when reading through the topic.

Gabriel

Andrea Aime wrote:

Justin Deoliveira ha scritto:
  

Hi all,

In working on the style editor in thew new UI I have once again run up against the issue of how we handle styles in configuration. I wanted to sum up my thoughts and get others peoples feedback about this.

So first off, if you look at StyleInfo you will see:

org.geotools.styling.Style getStyle();

The Style object corresponds to the UserStyle element in SLD. This is the first oddity. We store the full StyledLayerDescriptor on disk but only model a single UserStyle.

Where i am finding this problematic is in the style editor. Where we want to present the user will thefull SLD to edit. To do this we more or less have to bypass the style that is actually on the StyleInfo and load the underlying file from disk.

And saving out is problematic as well. With all other configuration objects we simply make the change on the configuration object and call save, and that gets transparently persisted for us. But this is a problem for styles for a couple of reasons:

1) just persisting what the StyleInfo.getStyle() represents loses information. Basically everything outside of the first UserStyle element.

2) It would change the structure of the actual XML/SLD. For instance anything that the style parser did not pick up and represent in memory would be lost. Comments, etc... Currently we preserve that, and for good reason.

So the workaround has always been to have the UI read and write styles directly from the data directory. With the old UI this was less of an issue. Because when the style page was submited and applied, every configuration object was reloaded, so the styles stayed in sync. Now the user interface has to keep the StyleInfo.getStyle() in sync manually... but said, there is no setStyle(Style) method.

Also, what about other code that wants to update styles, like say, the REST config stuff. It too has to know about these workarounds.

This represents a problem with how we model styles imho. There are other issues as well looking to the future. There has been talk of supporting other types of styles that are non SLD, like support for cascadnik. Our current model there would really be no way to handle this cleanly.
    
I am curious to hear other peoples thoughts on this. And if other people think that a retrofit of how we model styles is needed.
    
I agree on the sentiment, however not sure how to provide a solution.
Let me start with two lower handing fruits that I have some more
definite ideas on.

SLD storage wise, I too believe the file should only contain a Style
element, and not an SLD one. A full SLD is meant to operate in the
context of a WMS request, providing the definition of a map (in terms
or layers and styles), or acting as a style library.
Having people deal with anything other than just Style is indeed
only source of confusion.
So I would be very much +1 changing the parsing to deal with Style
only (but keep on reading StyledLayerDescriptor for backwards
compatibility) and change our demo material to just use a Style.

CSS wise (a nice SOC topic btw, just added it to the page), I'd
say we'd just have to parse the css definition in to a Style
object, so API wise using a GT2 Style would not change.

Yet, what would change is the way we parse, store, and present
the style to the user. Adding pluggable parser is not that
hard. The UI can be made pluggable as well.
To support UI to disk and back I guess we'll need a style
store extension point?

For sure we cannot use the parsed version in neither case,
as you have noticed, keeping the formatting and comments
the user made in the source is very important.

So, let's try to sum up a possible action plan:
- add a style type field to StyleInfo
- add methods in StyleInfo to get and set the raw
   style form (as i/o streams?)
- have a storage facility that recognizes the type
   and knows what file to look for
- have a pluggable parser that knows how to parse
   the contents coming from the storage into a GT2
   Style object.

Hmm... actually the storage could be really dumb,
just know what the file name is and decouple
the parser from disk (database?) access.

Opinions?
Cheers
Andrea

[Justin_Deoliveira]

Gabriel Roldan wrote:

Hi,

I just went quickly over the topic so I guess I understand the problem but may not be that insightful about a proposal for a solution. Yet, let me just chip in before I forget.

I guess you said we may want to leverage other sorts of styles definitions than ogc's ones, which seems clever to me.
To do that my first reaction would be to use some way the IAdaptable pattern in StyleInfo. Something on the lines of
StyleInfo.canAdapt(Class styleClass) and StyleInfo.getStyle(Class<T> styleClass):T

Implementation wise we can allow the factory that creates a styleinfo receive a String and held it internally. It may well be the SLD contents or any other thing and we can store it in the database verbatim if needed or as a file. So no matter if we store an SLD or a single SE style or some other sort of style definition, it could evolve to support other formats easily?

ok, it may be an off topic or simple stupid idea but wanted to tell what my though was when reading through the topic.

Gabriel

Interesting thought Gabriel. I agree that this would be a good application of IAdaptable. I know there has been push back to this "eclipsism" because of the complexity but I think this circumstance it does indeed make sense.

With a bit of variation I think it would also solve another issue. One of the problems we have with how we store stuff is afaik we only just read the first style in an SLD. Which means if someone is using an SLD as a "library", they are kind of stuck. One thing that would be cool would be:

FeatureTypeInfo ftinfo = ...;
StyleInfo style = ...;

//load the style for the feature type
Style se = style.adapt( Style.class, ftinfo );

Basically parameterizing Iadapable so that in this case only styles which will apply to the feature type would be loaded.

2c.

-Justin

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

[Gabriel_Roldan2]

Justin Deoliveira wrote:

Gabriel Roldan wrote:

Hi,

I just went quickly over the topic so I guess I understand the problem but may not be that insightful about a proposal for a solution. Yet, let me just chip in before I forget.

I guess you said we may want to leverage other sorts of styles definitions than ogc's ones, which seems clever to me.
To do that my first reaction would be to use some way the IAdaptable pattern in StyleInfo. Something on the lines of
StyleInfo.canAdapt(Class styleClass) and StyleInfo.getStyle(Class<T> styleClass):T

Implementation wise we can allow the factory that creates a styleinfo receive a String and held it internally. It may well be the SLD contents or any other thing and we can store it in the database verbatim if needed or as a file. So no matter if we store an SLD or a single SE style or some other sort of style definition, it could evolve to support other formats easily?

ok, it may be an off topic or simple stupid idea but wanted to tell what my though was when reading through the topic.

Gabriel

Interesting thought Gabriel. I agree that this would be a good application of IAdaptable. I know there has been push back to this "eclipsism" because of the complexity but I think this circumstance it does indeed make sense.

With a bit of variation I think it would also solve another issue. One of the problems we have with how we store stuff is afaik we only just read the first style in an SLD. Which means if someone is using an SLD as a "library", they are kind of stuck. One thing that would be cool would be:

FeatureTypeInfo ftinfo = ...;
StyleInfo style = ...;

//load the style for the feature type
Style se = style.adapt( Style.class, ftinfo );

Basically parameterizing Iadapable so that in this case only styles which will apply to the feature type would be loaded.

I very much like it.
Do you think we can push for it and we have the energy to do that right now or should go for some sort of compromise solution meanwhile?

Cheers,

Gabriel

2c.

-Justin

[Andrea_Aime3]

Justin Deoliveira ha scritto:

Gabriel Roldan wrote:

Hi,

I just went quickly over the topic so I guess I understand the problem but may not be that insightful about a proposal for a solution. Yet, let me just chip in before I forget.

I guess you said we may want to leverage other sorts of styles definitions than ogc's ones, which seems clever to me.
To do that my first reaction would be to use some way the IAdaptable pattern in StyleInfo. Something on the lines of
StyleInfo.canAdapt(Class styleClass) and StyleInfo.getStyle(Class<T> styleClass):T

Implementation wise we can allow the factory that creates a styleinfo receive a String and held it internally. It may well be the SLD contents or any other thing and we can store it in the database verbatim if needed or as a file. So no matter if we store an SLD or a single SE style or some other sort of style definition, it could evolve to support other formats easily?

ok, it may be an off topic or simple stupid idea but wanted to tell what my though was when reading through the topic.

Gabriel

Interesting thought Gabriel. I agree that this would be a good application of IAdaptable. I know there has been push back to this "eclipsism" because of the complexity but I think this circumstance it does indeed make sense.

With a bit of variation I think it would also solve another issue. One of the problems we have with how we store stuff is afaik we only just read the first style in an SLD. Which means if someone is using an SLD as a "library", they are kind of stuck. One thing that would be cool would be:

FeatureTypeInfo ftinfo = ...;
StyleInfo style = ...;

//load the style for the feature type
Style se = style.adapt( Style.class, ftinfo );

Basically parameterizing Iadapable so that in this case only styles which will apply to the feature type would be loaded.

A few random notes:
- do we really want/need to support style libraries? I thought the
   idea was to simplify style documents so that they contained only
   one Style root element. Most non trivial styles are very long,
   not sure I see the advantage of putting togheter many of them
   in a single file, it would also make the user chioce less direct.
   You associated a feature type to a style library, you get back
   the first style that applies?
   What if there are various of them and you did not realize it?
   How do you debug that as a user?
- I like the idea of storing just a string representation, whatever
   it is. I'm not sure I understand the need to IAdapt it thought.
   What choices do we have today? We want the style as a String, or
   the style as a GeoTools Style. IAdaptable fits when the conversions
   set is open ended, I'm failing to see the other potential cases.
- Parsing the style takes time, so for sure we don't want
   do perform the parsing for each WMS request, meaning we also need
   to create a cache of parsed styles somewhere, the cache should
   also be smart enough to avoid doing string equalities to check
   if the style was updated, but rely on getters/setters of StyleInfo
   instead. So I guess it would be better implemented inside the
   StyleInfo itself.

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

[Justin_Deoliveira]

Andrea Aime wrote:

Justin Deoliveira ha scritto:

Gabriel Roldan wrote:

Hi,

I just went quickly over the topic so I guess I understand the problem but may not be that insightful about a proposal for a solution. Yet, let me just chip in before I forget.

I guess you said we may want to leverage other sorts of styles definitions than ogc's ones, which seems clever to me.
To do that my first reaction would be to use some way the IAdaptable pattern in StyleInfo. Something on the lines of
StyleInfo.canAdapt(Class styleClass) and StyleInfo.getStyle(Class<T> styleClass):T

Implementation wise we can allow the factory that creates a styleinfo receive a String and held it internally. It may well be the SLD contents or any other thing and we can store it in the database verbatim if needed or as a file. So no matter if we store an SLD or a single SE style or some other sort of style definition, it could evolve to support other formats easily?

ok, it may be an off topic or simple stupid idea but wanted to tell what my though was when reading through the topic.

Gabriel

Interesting thought Gabriel. I agree that this would be a good application of IAdaptable. I know there has been push back to this "eclipsism" because of the complexity but I think this circumstance it does indeed make sense.

With a bit of variation I think it would also solve another issue. One of the problems we have with how we store stuff is afaik we only just read the first style in an SLD. Which means if someone is using an SLD as a "library", they are kind of stuck. One thing that would be cool would be:

FeatureTypeInfo ftinfo = ...;
StyleInfo style = ...;

//load the style for the feature type
Style se = style.adapt( Style.class, ftinfo );

Basically parameterizing Iadapable so that in this case only styles which will apply to the feature type would be loaded.

A few random notes:
- do we really want/need to support style libraries? I thought the
  idea was to simplify style documents so that they contained only
  one Style root element. Most non trivial styles are very long,
  not sure I see the advantage of putting togheter many of them
  in a single file, it would also make the user chioce less direct.
  You associated a feature type to a style library, you get back
  the first style that applies?
  What if there are various of them and you did not realize it?
  How do you debug that as a user?

Well I did not mean the example to be 100% correct, but change to a list of array and the point holds. I don't know if we want to support SLD libraries per se. But the use case does not seem to extreme to me. I am sure if we look hard enough we can find someone that uses a single SLD document for all there feature type styles.

- I like the idea of storing just a string representation, whatever
  it is. I'm not sure I understand the need to IAdapt it thought.
  What choices do we have today? We want the style as a String, or
  the style as a GeoTools Style. IAdaptable fits when the conversions
  set is open ended, I'm failing to see the other potential cases.

Well I think it comes in to get back the information you want. Do you want the full SLD, or do you want just a single feature type style, or maybe you want the raw string as it sits on disk, all could fit into the context of iadaptable.

That said, I did think about this, just storing the raw style as a string. It has the benefit of keeping around the raw structure around. Seemed a bit clumsy though...

- Parsing the style takes time, so for sure we don't want
  do perform the parsing for each WMS request, meaning we also need
  to create a cache of parsed styles somewhere, the cache should
  also be smart enough to avoid doing string equalities to check
  if the style was updated, but rely on getters/setters of StyleInfo
  instead. So I guess it would be better implemented inside the
  StyleInfo itself.

Yeah... we definitely want to cache, which is why i like ResourcePool as the home for this stuff. While I think iadaptable *could* work in this context I am still hesitant to use StyleInfo for things other then just a dumb java bean.

Cheers
Andrea

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

[Andrea_Aime3]

Justin Deoliveira ha scritto:

Andrea Aime wrote:

Justin Deoliveira ha scritto:

Gabriel Roldan wrote:

Hi,

I just went quickly over the topic so I guess I understand the problem but may not be that insightful about a proposal for a solution. Yet, let me just chip in before I forget.

I guess you said we may want to leverage other sorts of styles definitions than ogc's ones, which seems clever to me.
To do that my first reaction would be to use some way the IAdaptable pattern in StyleInfo. Something on the lines of
StyleInfo.canAdapt(Class styleClass) and StyleInfo.getStyle(Class<T> styleClass):T

Implementation wise we can allow the factory that creates a styleinfo receive a String and held it internally. It may well be the SLD contents or any other thing and we can store it in the database verbatim if needed or as a file. So no matter if we store an SLD or a single SE style or some other sort of style definition, it could evolve to support other formats easily?

ok, it may be an off topic or simple stupid idea but wanted to tell what my though was when reading through the topic.

Gabriel

Interesting thought Gabriel. I agree that this would be a good application of IAdaptable. I know there has been push back to this "eclipsism" because of the complexity but I think this circumstance it does indeed make sense.

With a bit of variation I think it would also solve another issue. One of the problems we have with how we store stuff is afaik we only just read the first style in an SLD. Which means if someone is using an SLD as a "library", they are kind of stuck. One thing that would be cool would be:

FeatureTypeInfo ftinfo = ...;
StyleInfo style = ...;

//load the style for the feature type
Style se = style.adapt( Style.class, ftinfo );

Basically parameterizing Iadapable so that in this case only styles which will apply to the feature type would be loaded.

A few random notes:
- do we really want/need to support style libraries? I thought the
  idea was to simplify style documents so that they contained only
  one Style root element. Most non trivial styles are very long,
  not sure I see the advantage of putting togheter many of them
  in a single file, it would also make the user chioce less direct.
  You associated a feature type to a style library, you get back
  the first style that applies?
  What if there are various of them and you did not realize it?
  How do you debug that as a user?

Well I did not mean the example to be 100% correct, but change to a list of array and the point holds. I don't know if we want to support SLD libraries per se. But the use case does not seem to extreme to me. I am sure if we look hard enough we can find someone that uses a single SLD document for all there feature type styles.

Sure, but that sounds like theory, and I reason in practical terms
instead. So I guess I can rephrase the question as, how do we make
sure this does not become confusing for the user?
Do we log somewhere which style has been actually picked up out
of the library? (possibly, in the UI)?
Do we have a plan to offset the cost of parsing tens or hundreds
of styles when we're actually just looking for one? (once we go
down that path we relinquish control on how many styles the sld
document contains).

- I like the idea of storing just a string representation, whatever
  it is. I'm not sure I understand the need to IAdapt it thought.
  What choices do we have today? We want the style as a String, or
  the style as a GeoTools Style. IAdaptable fits when the conversions
  set is open ended, I'm failing to see the other potential cases.

Well I think it comes in to get back the information you want. Do you want the full SLD, or do you want just a single feature type style, or maybe you want the raw string as it sits on disk, all could fit into the context of iadaptable.

That said, I did think about this, just storing the raw style as a string. It has the benefit of keeping around the raw structure around. Seemed a bit clumsy though...

If we want to avoid loosing comments and so on, I guess there is
no other way. Somewhere the raw form must be available. Keeping
it in memory is a possible way, otherwise we need some kind of direct
reference to the file on disk (or the record in the database...
but we can decide that is a implementation detail of StyleImpl,
and that a Hibernate based implementation will keep something other
than a path to allow the raw form lookup).

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

[Gabriel_Roldan2]

Andrea Aime wrote:

Justin Deoliveira ha scritto:

Andrea Aime wrote:

Justin Deoliveira ha scritto:

Gabriel Roldan wrote:

Hi,

I just went quickly over the topic so I guess I understand the problem but may not be that insightful about a proposal for a solution. Yet, let me just chip in before I forget.

I guess you said we may want to leverage other sorts of styles definitions than ogc's ones, which seems clever to me.
To do that my first reaction would be to use some way the IAdaptable pattern in StyleInfo. Something on the lines of
StyleInfo.canAdapt(Class styleClass) and StyleInfo.getStyle(Class<T> styleClass):T

Implementation wise we can allow the factory that creates a styleinfo receive a String and held it internally. It may well be the SLD contents or any other thing and we can store it in the database verbatim if needed or as a file. So no matter if we store an SLD or a single SE style or some other sort of style definition, it could evolve to support other formats easily?

ok, it may be an off topic or simple stupid idea but wanted to tell what my though was when reading through the topic.

Gabriel

Interesting thought Gabriel. I agree that this would be a good application of IAdaptable. I know there has been push back to this "eclipsism" because of the complexity but I think this circumstance it does indeed make sense.

With a bit of variation I think it would also solve another issue. One of the problems we have with how we store stuff is afaik we only just read the first style in an SLD. Which means if someone is using an SLD as a "library", they are kind of stuck. One thing that would be cool would be:

FeatureTypeInfo ftinfo = ...;
StyleInfo style = ...;

//load the style for the feature type
Style se = style.adapt( Style.class, ftinfo );

Basically parameterizing Iadapable so that in this case only styles which will apply to the feature type would be loaded.

A few random notes:
- do we really want/need to support style libraries? I thought the
  idea was to simplify style documents so that they contained only
  one Style root element. Most non trivial styles are very long,
  not sure I see the advantage of putting togheter many of them
  in a single file, it would also make the user chioce less direct.
  You associated a feature type to a style library, you get back
  the first style that applies?
  What if there are various of them and you did not realize it?
  How do you debug that as a user?

Well I did not mean the example to be 100% correct, but change to a list of array and the point holds. I don't know if we want to support SLD libraries per se. But the use case does not seem to extreme to me. I am sure if we look hard enough we can find someone that uses a single SLD document for all there feature type styles.

Sure, but that sounds like theory, and I reason in practical terms
instead. So I guess I can rephrase the question as, how do we make
sure this does not become confusing for the user?
Do we log somewhere which style has been actually picked up out
of the library? (possibly, in the UI)?
Do we have a plan to offset the cost of parsing tens or hundreds
of styles when we're actually just looking for one? (once we go
down that path we relinquish control on how many styles the sld
document contains).

I was thinking, instead of sld library mode, that we wanted to support other sort of styles than SLD ones. And if you ask me I reckon I don't know yet what those are gonna be, so this may well be overarchitecting and I just got wrong the intention of supporting other kinds of style definitions?

- I like the idea of storing just a string representation, whatever
  it is. I'm not sure I understand the need to IAdapt it thought.
  What choices do we have today? We want the style as a String, or
  the style as a GeoTools Style. IAdaptable fits when the conversions
  set is open ended, I'm failing to see the other potential cases.

Well I think it comes in to get back the information you want. Do you want the full SLD, or do you want just a single feature type style, or maybe you want the raw string as it sits on disk, all could fit into the context of iadaptable.

That said, I did think about this, just storing the raw style as a string. It has the benefit of keeping around the raw structure around. Seemed a bit clumsy though...

If we want to avoid loosing comments and so on, I guess there is
no other way. Somewhere the raw form must be available. Keeping
it in memory is a possible way, otherwise we need some kind of direct
reference to the file on disk (or the record in the database...
but we can decide that is a implementation detail of StyleImpl,
and that a Hibernate based implementation will keep something other
than a path to allow the raw form lookup).

Cheers
Andrea

[Andrea_Aime3]

Gabriel Roldan ha scritto:

I was thinking, instead of sld library mode, that we wanted to support other sort of styles than SLD ones. And if you ask me I reckon I don't know yet what those are gonna be, so this may well be overarchitecting and I just got wrong the intention of supporting other kinds of style definitions?

Yeah, I see the use case as a plausible one. Connecting
it to the existing code is harder thought. The renderer, the svg
generator, the kml generator all play with geotools Style objects
directly.
In order to make any use of different ways to specify styling we'd
need some sort o generalization at that level.

I guess we'd need to some kind of pluggable factory that given a feature
and a generic style object (whose nature is unknown) returns
the actual geometry that needs rendering, and some directions
on how to render it (fill, symbols, labels).

This role is played, partly, by the SLDStyleFactory, but not
for the geometries for example. We could generalize this further,
introduce a style factory interface and an extension point, I guess
we'd first need a solid use case (and resources) to do that first.
So far the two examples we have at hand (SLD and CSS) seem to something
that we should be able to coax into a single SLD object.

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

[Gabriel_Roldan2]

Andrea Aime wrote:

Gabriel Roldan ha scritto:

I was thinking, instead of sld library mode, that we wanted to support other sort of styles than SLD ones. And if you ask me I reckon I don't know yet what those are gonna be, so this may well be overarchitecting and I just got wrong the intention of supporting other kinds of style definitions?

Yeah, I see the use case as a plausible one. Connecting
it to the existing code is harder thought. The renderer, the svg
generator, the kml generator all play with geotools Style objects
directly.
In order to make any use of different ways to specify styling we'd
need some sort o generalization at that level.

I guess we'd need to some kind of pluggable factory that given a feature
and a generic style object (whose nature is unknown) returns
the actual geometry that needs rendering, and some directions
on how to render it (fill, symbols, labels).

On the other side, we could keep our internal object model for styling being SLD and provide adaptations from anything else (css as you mentioned) to SLD so not all the rest of the code needs to migrate to something abstract and we can still keep the original style definition in its native form, whatever it is... as long as our SLD implementation with our custom extensions is powerful enough.
but you're right this may be too an abstract though without a mandate/resources/strong use case to go for it.

Gabriel

This role is played, partly, by the SLDStyleFactory, but not
for the geometries for example. We could generalize this further,
introduce a style factory interface and an extension point, I guess
we'd first need a solid use case (and resources) to do that first.
So far the two examples we have at hand (SLD and CSS) seem to something
that we should be able to coax into a single SLD object.

Cheers
Andrea

[Justin_Deoliveira]

Soo... for now what I have done is added two methods to ResourcePool:

Reader readStyle(StyleInfo);
void writeStyle(StyleInfo,InputStream);

for reading and writing a style directly. When it comes to the point where we do support other types of styles, it will be low effort to change these methods to load a specific parser or factory, etc...

But for now this at least gives us all the logic for loading/persisting of styles centralized to a single place. The downside is that styles are sort of a a special case.

-Justin

Gabriel Roldan wrote:

Andrea Aime wrote:

Gabriel Roldan ha scritto:

I was thinking, instead of sld library mode, that we wanted to support other sort of styles than SLD ones. And if you ask me I reckon I don't know yet what those are gonna be, so this may well be overarchitecting and I just got wrong the intention of supporting other kinds of style definitions?

Yeah, I see the use case as a plausible one. Connecting
it to the existing code is harder thought. The renderer, the svg
generator, the kml generator all play with geotools Style objects
directly.
In order to make any use of different ways to specify styling we'd
need some sort o generalization at that level.

I guess we'd need to some kind of pluggable factory that given a feature
and a generic style object (whose nature is unknown) returns
the actual geometry that needs rendering, and some directions
on how to render it (fill, symbols, labels).

On the other side, we could keep our internal object model for styling being SLD and provide adaptations from anything else (css as you mentioned) to SLD so not all the rest of the code needs to migrate to something abstract and we can still keep the original style definition in its native form, whatever it is... as long as our SLD implementation with our custom extensions is powerful enough.
but you're right this may be too an abstract though without a mandate/resources/strong use case to go for it.

Gabriel

This role is played, partly, by the SLDStyleFactory, but not
for the geometries for example. We could generalize this further,
introduce a style factory interface and an extension point, I guess
we'd first need a solid use case (and resources) to do that first.
So far the two examples we have at hand (SLD and CSS) seem to something
that we should be able to coax into a single SLD object.

Cheers
Andrea

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.