Hi all. I've done some work on adding support for the optional
Attribution field in the WMS capabilities document. I'd like a review
on the patch attached to http://jira.codehaus.org/browse/GEOS-3181 .
The patch uses resource metadata to store title, link, and logo image
info for attribution on each layer, and provides a panel on the
Publishing tab for configuring them. If a layer has these properties,
then they are encoded during a capabilities request (since each field is
optional, my code will render a partial Attribution field if some of the
information is missing, eg, just a title and link if no logo is
provided.)
Thanks.
--
David Winslow
OpenGeo - http://opengeo.org/
All in all patch looks good David. And thanks for including test cases with it :).
One question I noted on the issue. Is ResourceInfo.getTitle() not suitable for the title? Or is it a different intention?
Also, just a point of discussion (not a issue with the patch), I wonder if we should break out an actual class for this type of metadata?
David Winslow wrote:
Hi all. I've done some work on adding support for the optional
Attribution field in the WMS capabilities document. I'd like a review
on the patch attached to http://jira.codehaus.org/browse/GEOS-3181 .
The patch uses resource metadata to store title, link, and logo image
info for attribution on each layer, and provides a panel on the
Publishing tab for configuring them. If a layer has these properties,
then they are encoded during a capabilities request (since each field is
optional, my code will render a partial Attribution field if some of the
information is missing, eg, just a title and link if no logo is
provided.)
Thanks.
--
David Winslow OpenGeo - http://opengeo.org/
------------------------------------------------------------------------------
Are you an open source citizen? Join us for the Open Source Bridge conference!
Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250.
Need another reason to go? 24-hour hacker lounge. Register today!
http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
On Wed, 2009-06-24 at 10:03 +0800, Justin Deoliveira wrote:
All in all patch looks good David. And thanks for including test cases
with it :).
One question I noted on the issue. Is ResourceInfo.getTitle() not
suitable for the title? Or is it a different intention?
The Attribution data describes the data provider, not the dataset
itself, so ResourceInfo.getTitle() is indeed different from the
Attribution Title.
Also, just a point of discussion (not a issue with the patch), I wonder
if we should break out an actual class for this type of metadata?
I would be +1 on this. I should note that when I tried to store an
AttributionInfo object in the metadata map, it was persisted using just
AttributionInfo.toString(). Is this the intended behavior? (and if so,
does this mean I need to implement a GeoTools Converter to store custom
objects in the metadata map? This page on the wiki says anything
Serializable is okay:
http://geoserver.org/display/GEOS/Adding+a+Configuration+Option-GS2 )
--
David Winslow
OpenGeo - http://opengeo.org/
David Winslow wrote:
> Hi all. I've done some work on adding support for the optional
> Attribution field in the WMS capabilities document. I'd like a review
> on the patch attached to http://jira.codehaus.org/browse/GEOS-3181 .
>
> The patch uses resource metadata to store title, link, and logo image
> info for attribution on each layer, and provides a panel on the
> Publishing tab for configuring them. If a layer has these properties,
> then they are encoded during a capabilities request (since each field is
> optional, my code will render a partial Attribution field if some of the
> information is missing, eg, just a title and link if no logo is
> provided.)
>
> Thanks.
>
> --
> David Winslow
> OpenGeo - http://opengeo.org/
>
>
> ------------------------------------------------------------------------------
> Are you an open source citizen? Join us for the Open Source Bridge conference!
> Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250.
> Need another reason to go? 24-hour hacker lounge. Register today!
> http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
> _______________________________________________
> Geoserver-devel mailing list
> Geoserver-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
David Winslow wrote:
On Wed, 2009-06-24 at 10:03 +0800, Justin Deoliveira wrote:
All in all patch looks good David. And thanks for including test cases with it :).
One question I noted on the issue. Is ResourceInfo.getTitle() not suitable for the title? Or is it a different intention?
The Attribution data describes the data provider, not the dataset
itself, so ResourceInfo.getTitle() is indeed different from the
Attribution Title.
Fair enough.
Also, just a point of discussion (not a issue with the patch), I wonder if we should break out an actual class for this type of metadata?
I would be +1 on this. I should note that when I tried to store an
AttributionInfo object in the metadata map, it was persisted using just
AttributionInfo.toString(). Is this the intended behavior? (and if so,
does this mean I need to implement a GeoTools Converter to store custom
objects in the metadata map? This page on the wiki says anything
Serializable is okay:
http://geoserver.org/display/GEOS/Adding+a+Configuration+Option-GS2 )
Yeah... this is a issue. The metadata map says it can store anything serializable, but the xtream persister assumes it is just a string. To the persister should be patched. Do you want to open a seperate jira issue for this?
I would also be +1 for a AttributionInfo class, and adding LayerInfo.getAttribution(). What do others think?
--
David Winslow
OpenGeo - http://opengeo.org/
David Winslow wrote:
Hi all. I've done some work on adding support for the optional
Attribution field in the WMS capabilities document. I'd like a review
on the patch attached to http://jira.codehaus.org/browse/GEOS-3181 .
The patch uses resource metadata to store title, link, and logo image
info for attribution on each layer, and provides a panel on the
Publishing tab for configuring them. If a layer has these properties,
then they are encoded during a capabilities request (since each field is
optional, my code will render a partial Attribution field if some of the
information is missing, eg, just a title and link if no logo is
provided.)
Thanks.
--
David Winslow OpenGeo - http://opengeo.org/
------------------------------------------------------------------------------
Are you an open source citizen? Join us for the Open Source Bridge conference!
Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250.
Need another reason to go? 24-hour hacker lounge. Register today!
http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
Justin Deoliveira wrote:
David Winslow wrote:
On Wed, 2009-06-24 at 10:03 +0800, Justin Deoliveira wrote:
All in all patch looks good David. And thanks for including test cases with it :).
One question I noted on the issue. Is ResourceInfo.getTitle() not suitable for the title? Or is it a different intention?
The Attribution data describes the data provider, not the dataset
itself, so ResourceInfo.getTitle() is indeed different from the
Attribution Title.
Fair enough.
Also, just a point of discussion (not a issue with the patch), I wonder if we should break out an actual class for this type of metadata?
I would be +1 on this. I should note that when I tried to store an
AttributionInfo object in the metadata map, it was persisted using just
AttributionInfo.toString(). Is this the intended behavior? (and if so,
does this mean I need to implement a GeoTools Converter to store custom
objects in the metadata map? This page on the wiki says anything
Serializable is okay:
http://geoserver.org/display/GEOS/Adding+a+Configuration+Option-GS2 )
Yeah... this is a issue. The metadata map says it can store anything serializable, but the xtream persister assumes it is just a string. To the persister should be patched. Do you want to open a seperate jira issue for this?
I would also be +1 for a AttributionInfo class, and adding LayerInfo.getAttribution(). What do others think?
seems like the cleaner path to me, so +1
Gabriel
--
David Winslow
OpenGeo - http://opengeo.org/
David Winslow wrote:
Hi all. I've done some work on adding support for the optional
Attribution field in the WMS capabilities document. I'd like a review
on the patch attached to http://jira.codehaus.org/browse/GEOS-3181 .
The patch uses resource metadata to store title, link, and logo image
info for attribution on each layer, and provides a panel on the
Publishing tab for configuring them. If a layer has these properties,
then they are encoded during a capabilities request (since each field is
optional, my code will render a partial Attribution field if some of the
information is missing, eg, just a title and link if no logo is
provided.)
Thanks.
--
David Winslow OpenGeo - http://opengeo.org/
------------------------------------------------------------------------------
Are you an open source citizen? Join us for the Open Source Bridge conference!
Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250.
Need another reason to go? 24-hour hacker lounge. Register today!
http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.
David,
Based on the commit logs I see you went ahead with this. A couple of things about the commit.
* Registering AttributionInfoImpl as the default implementation of the interface AttributionInfo was missed in the xstream persister. This is done to make the persisted XML more human readable.
* The AttributionInfo interface has some minor inconsistencies with regard to the rest of the interfaces:
- All the methods on the interface have the public modifier which is redundant
- There are no javadocs on any of the methods, i realize they are simple getters and setters, but a simple description of what the property means should be there
- getLogoUrl() --> getLogoURL(). This is just a convention I have been using for url and uri, using uppercase.
I know this is minor stuff and probably a bit too anal but a lot of time and effort have gone into keeping the catalog and configuration interfaces clean in terms of javadocs and consistency. I would appreciate if we could keep that up.
-Justin
Gabriel Roldan wrote:
Justin Deoliveira wrote:
David Winslow wrote:
On Wed, 2009-06-24 at 10:03 +0800, Justin Deoliveira wrote:
All in all patch looks good David. And thanks for including test cases with it :).
One question I noted on the issue. Is ResourceInfo.getTitle() not suitable for the title? Or is it a different intention?
The Attribution data describes the data provider, not the dataset
itself, so ResourceInfo.getTitle() is indeed different from the
Attribution Title.
Fair enough.
Also, just a point of discussion (not a issue with the patch), I wonder if we should break out an actual class for this type of metadata?
I would be +1 on this. I should note that when I tried to store an
AttributionInfo object in the metadata map, it was persisted using just
AttributionInfo.toString(). Is this the intended behavior? (and if so,
does this mean I need to implement a GeoTools Converter to store custom
objects in the metadata map? This page on the wiki says anything
Serializable is okay:
http://geoserver.org/display/GEOS/Adding+a+Configuration+Option-GS2 )
Yeah... this is a issue. The metadata map says it can store anything serializable, but the xtream persister assumes it is just a string. To the persister should be patched. Do you want to open a seperate jira issue for this?
I would also be +1 for a AttributionInfo class, and adding LayerInfo.getAttribution(). What do others think?
seems like the cleaner path to me, so +1
Gabriel
--
David Winslow
OpenGeo - http://opengeo.org/
David Winslow wrote:
Hi all. I've done some work on adding support for the optional
Attribution field in the WMS capabilities document. I'd like a review
on the patch attached to http://jira.codehaus.org/browse/GEOS-3181 . The patch uses resource metadata to store title, link, and logo image
info for attribution on each layer, and provides a panel on the
Publishing tab for configuring them. If a layer has these properties,
then they are encoded during a capabilities request (since each field is
optional, my code will render a partial Attribution field if some of the
information is missing, eg, just a title and link if no logo is
provided.)
Thanks.
--
David Winslow OpenGeo - http://opengeo.org/
------------------------------------------------------------------------------
Are you an open source citizen? Join us for the Open Source Bridge conference!
Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250.
Need another reason to go? 24-hour hacker lounge. Register today!
http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.