Hi,
I'm trying to use CatalogBuilder.buildFeatureType() and I'm
having some troubles with its behaviour, so I'm looking
for ways to fix it.
(see full source code here: http://svn.codehaus.org/geoserver/trunk/src/main/src/main/java/org/geoserver/catalog/CatalogBuilder.java)
The basic issue I'm having is with the method specification. That is,
is the method required to build a fully usable FeatureTypeInfo, and
thus throw exceptions in the case it's not possible to do so, or
try a best effort to provide something ready that the user can customized later, so the focus is to fill as much fields as possible
automatically?
Both use cases are legitimate. The REST configuration may need the
first, the Wicket UI the latter. The current implementation is a bit
of both, but neither fully.
For example, buildFeatureType() may set a null SRS in case the
lookup fails, meaning the layer won't be usable in a valid capabilities
response.
Yet, if the transformation from native to lat/lon fails, an exception
will be thrown back. But, if the native CRS was missing, then the
lat/long bbox will be simply skipped and the feature type info returned.
I think the method should be modified to just fill as much as possible
without throwing exceptions, and leave the checks to whether a feature
type is usable to the catalog validation routines instead, or to the
UI validation, in case the feature type built like this is used
to fill a new layer configuration page.
Oh, UI wise we don't want to have the native bbox computed before
hand as that may take various minutes when the data sets are huge
(this is one of the things we had to fix for TIKE in 1.7.x).
Should we add a param in that direction, or just have the bbox filling
be a separate method (initBboxes(FEatureTypeInfo)?
Opinions?
Cheers
Andrea
--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.
Andrea Aime wrote:
Hi,
I'm trying to use CatalogBuilder.buildFeatureType() and I'm
having some troubles with its behaviour, so I'm looking
for ways to fix it.
(see full source code here: http://svn.codehaus.org/geoserver/trunk/src/main/src/main/java/org/geoserver/catalog/CatalogBuilder.java)
The basic issue I'm having is with the method specification. That is,
is the method required to build a fully usable FeatureTypeInfo, and
thus throw exceptions in the case it's not possible to do so, or
try a best effort to provide something ready that the user can customized later, so the focus is to fill as much fields as possible
automatically?
Both use cases are legitimate. The REST configuration may need the
first, the Wicket UI the latter. The current implementation is a bit
of both, but neither fully.
For example, buildFeatureType() may set a null SRS in case the
lookup fails, meaning the layer won't be usable in a valid capabilities
response.
Yet, if the transformation from native to lat/lon fails, an exception
will be thrown back. But, if the native CRS was missing, then the
lat/long bbox will be simply skipped and the feature type info returned.
I guess the latter. I agree that its better design to just have the builder do what it can, and not throw exceptions, seems more inline with what a builder should do. The downside is that this logic will have to replicated elsewhere. And there is possibility of duplication, doing it in the UI and the catalog. And of course removing any thing will most likely result in a regression somewhere.
That class was a place to put "stuff", so its a bit messy at this point. If we think it is "slowing down", we should refactor it, and now is the time to place some rules on the builder, I would be for that. But it would be nice to do it consistently. Or at least come up with the plan of how to do it consistently.
I think the method should be modified to just fill as much as possible
without throwing exceptions, and leave the checks to whether a feature
type is usable to the catalog validation routines instead, or to the
UI validation, in case the feature type built like this is used
to fill a new layer configuration page.
Oh, UI wise we don't want to have the native bbox computed before
hand as that may take various minutes when the data sets are huge
(this is one of the things we had to fix for TIKE in 1.7.x).
Should we add a param in that direction, or just have the bbox filling
be a separate method (initBboxes(FEatureTypeInfo)?
Are you talking about the code that calls featureSource.getBounds()? I am ok with breaking out expensive operations into a separate methods.
Opinions?
Cheers
Andrea
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
Justin Deoliveira ha scritto:
> I guess the latter. I agree that its better design to just have the
builder do what it can, and not throw exceptions, seems more inline with what a builder should do. The downside is that this logic will have to replicated elsewhere. And there is possibility of duplication, doing it in the UI and the catalog. And of course removing any thing will most likely result in a regression somewhere.
Either that or I have to copy that code fully for the work I'm doing
and fix it in my module. At the moment the only user of that method
on trunk seems to be the wicket UI, which would enjoy the same fixes.
That class was a place to put "stuff", so its a bit messy at this point. If we think it is "slowing down", we should refactor it, and now is the time to place some rules on the builder, I would be for that. But it would be nice to do it consistently. Or at least come up with the plan of how to do it consistently.
Sure, let's talk about it (I can make a short lived copy of that
code for the time being).
So, all the methods should try to act as data fillers and avoid
failing when they cannot fill one parameter.
Long operations should be optional.
Any other principle?
I think the method should be modified to just fill as much as possible
without throwing exceptions, and leave the checks to whether a feature
type is usable to the catalog validation routines instead, or to the
UI validation, in case the feature type built like this is used
to fill a new layer configuration page.
Oh, UI wise we don't want to have the native bbox computed before
hand as that may take various minutes when the data sets are huge
(this is one of the things we had to fix for TIKE in 1.7.x).
Should we add a param in that direction, or just have the bbox filling
be a separate method (initBboxes(FEatureTypeInfo)?
Are you talking about the code that calls featureSource.getBounds()? I am ok with breaking out expensive operations into a separate methods.
Yep, that's the one.
Cheers
Andrea
--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.
Andrea Aime ha scritto:
Are you talking about the code that calls featureSource.getBounds()? I am ok with breaking out expensive operations into a separate methods.
Yep, that's the one.
Another long-ish operation is looking up the SRS from the native CRS
using:
ftinfo.setSRS( CRS.lookupIdentifier( crs, true ) );
If the native CRS does not have an identifier that might take around 5
seconds. Too much? Depends, if you're trying to do that for 100 layers
in a row that might get annoying 
Alternatively, we could have a boolean parameter in the building
methods on whether to try out the expensive operations or not.
Separate methods (initSRS, initBBoxes) would provide a more
"a la carte" api thought.
Cheers
Andrea
--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.
Andrea Aime wrote:
Andrea Aime ha scritto:
Are you talking about the code that calls featureSource.getBounds()? I am ok with breaking out expensive operations into a separate methods.
Yep, that's the one.
Another long-ish operation is looking up the SRS from the native CRS
using:
ftinfo.setSRS( CRS.lookupIdentifier( crs, true ) );
If the native CRS does not have an identifier that might take around 5
seconds. Too much? Depends, if you're trying to do that for 100 layers
in a row that might get annoying
Alternatively, we could have a boolean parameter in the building
methods on whether to try out the expensive operations or not.
Separate methods (initSRS, initBBoxes) would provide a more
"a la carte" api thought.
Sure, let's break out separate methods, and add srs lookups to the list of expensive operations.
Cheers
Andrea
--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.