From: <dz...@re...> - 2004-12-29 22:27:02
|
Heya Chris, thanks for your thoughts ... I've included thoughts/reasons inline. Quoting Chris Holmes <ch...@op...>: > Ok, so I'm finally playing on trunk! > > GeoServer seems to be working, more or less. Had some fun with the > datastore selection, as the old getDescription call lead to a super long > WFSDataStore description, screwing up struts a bit. But then I was able > to replace it with getDisplayName(), and all is fine. Will have to figure > out how to integrate the getDescription stuff in some way though. Much of this matches the capabilities of the WFS or hard-coded strings (yes, I realize this is bad) ... but we can figure out smaller texts. > > I find the WFSDataStore factory params a bit odd though. The whole > filling in one of two mutually exclusive parameters strikes me as a bit > odd. And the descriptions as to how to use each can't even fully show up > on my mouse over help (a problem I should fix, but it does speak to how > long the descriptions are). There are two params here ... one is to allow for a capabilities document that is not a server cgi base to be included, while not forcing the encoding of the get params on the url for simple servers. (ie, you may have a local personal capabilities doc). > But if two booleans are mutually exclusive, > then can't we just use one? I'm thinking of use_post and use_get here. > It seems to me we only need one of them. If use_post is false then > use_get is implied, so we don't need users to specifically set it to true. > It also should have a default, right now there is none, for either use > post or use get. So yes, how would we feel about just one of the use_get > and use_post? I don't think we really lose much, and these fields are > just confusing anyways. These fields are for testing or optimization ... and they actually each have three valid values (Boolean.TRUE, Boolean.FALSE, NULL) ... with two pairs of values not allowed (both Boolean.TRUE or both Boolean.FALSE). Yes I did think about using an int mask here, and would be open to this. > > Second, the whole mutually exclusive thing doesn't actually work out with > the server_url and capabilities_url, since server_url is required, yet > capabilities_url is not. I also see no reason not to just restrict users > to one or the other - the win of not having to specify exactly where the > capabilities document at all, instead just appending > request=getcapabilities, seems not nearly worth the confusion caused by > two weird mutually exclusive fields. Though looking at uDIG it looks like > you guys already took care of this? I am doing magic there ... attempting the getCapabilities param, grabbing the exception, and then trying the server url ... we could include this magic with an optional flag as to how to treat the url (some capabilities urls do not include all the params ... and appending them cause errors, I can't recall the server, but found one, and it appeared to follow the spec). > I haven't been able to update my > geotools due to slow svn, but there seems to just be one url. Did you > guys do this in GeoTools? If not could you roll the fix into GeoTools? Nope, it's a wrapper in udig. > > As for some nit-picky complaints the display name for WFSDataStore is the > only one that has the words DataStore - seems to me like it should just > say WFS or Web Feature Server, to be in line with the rest of the display > names. Not a problem, can fix this. > It is also the only one to that has its params prefixed with > WFSDataStore: type things. Seems to me a bit redundant, but if people > actually want it, at the least we should make it consistent. GeoServer > does not special case any datastores, with nice wizards like uDIG, which > is why I'm a bit more nitpicky with this stuff. But I think we should > have it as nice as possible in the library. Just did that to ensure that the name was unique ... think I included the whole package name too. > > I also wasn't quite able to get features - due to an > ArrayIndexOutOfBoundsException: 3 at > org.geotools.xml.wfs.WFSBasicComplexTypes$GetFeatureType.encode(WFSBasicComlexTypes.java:185). > I looked at the code and could not figure out where this elem array came > from, and what the hardcoded 3 meant. Might be nice to use some static > finals to name the 0,1,2,3 and whatnot, with what they represent, since I > could not make heads or tails of it. This may have been fixed, I need to > update my jars. But you should be able to replicate it fairly easily - > all I did was had a geoserver running with the defaults, and then made > another one (from cvs) with the WFSDataStore. Was able to add the states > as a WFS layer, but then when I tried to actually get features it would > choke with the array out of bound. And I did this with a couple different > feature Types. This could well have been fixed ... it is in the reverse-engineering of the gt featuretype from a gml schema declaration. I did some work here not too long ago. > > But yeah, overall it looks like great work, I'm excited to offer cascading > WFS with GeoServer. Thanks, me too. David > > best regards, > > Chris > > -- > > > > ------------------------------------------------------- > SF email is sponsored by - The IT Product Guide > Read honest & candid reviews on hundreds of IT Products from real users. > Discover which products truly live up to the hype. Start reading now. > http://productguide.itmanagersjournal.com/ > _______________________________________________ > Geotools-devel mailing list > Geo...@li... > https://lists.sourceforge.net/lists/listinfo/geotools-devel > |