Hey Ho! I'm happy to create a patch for the (1). As far as (2) is concerned, it doesn't make any sense to use my solution if it uses a "deprecated style". I will look into the code of the WFS datastore (I discovered the bugs by using this datastore) and see if I can find a solution supporting the "new style".
Unfortunately, I have heaps of things to do in the next couple of days. So I don't think, I will be able to finish any of the things I just promised to do before the end of next week. I hope you can wait that long. Otherwise someone else has to volunteer. cheers, stefan Saul Farber wrote: > Ahh, the curse of the "willing to do it". > > Stefan, I've looked at your suggestions, and here are my thoughts: > > 1) I'm not sure I understand the changes well enough on this one, but > if you could provide a patch using "svn diff" on the PPPFSV.java file, > then I can review and give you the ok on this one. > > > 2) You've hit a recurring problem with PPPFSV. FilterCapabilities have > two 'styles'. One is the "old style" with > FilterCapabilities.INTEGER_CONSTANT as the definition of the capability. > The other is the "new style" with Capability.class as the definition of > the capability. Your patch which includes BOTH checks looks good. > Leaving one off will work for you, but break other datastores that use > the new filter capabilities style exclusively. Also, as andrea pointed > out, you should have your FilterCapabilities declared in the "new style" > as well (if not exclusively). > > > For both patches, you'll need to do the following: > > a. Make your changes to your local subversion working copy. > b. Run '$ mvn clean install to make sure all tests are still passing. > c. create an "svn diff" of your patch and send it to the list for > review (attaching it to the JIRA issue would be just as good, if not > better) > d. Port your patch FORWARD to geotools-trunk. the PPPFSV class hasn't > changed much between the two branches, so probably just 'svn diff' and > 'patch -p0 < your-patch-file.patch' should do it. If not, eclipse and > 'compare with each other...' will do you wonders. > e. Make sure that the patched geotools-trunk also is passing all tests > (or at least those tests which you probably didn't break) > f. Commit! > > > If you don't have the time/interest in doing steps a-e, please don't do > step f...but please *do* create patches and then I can do all 6 steps > just to make sure 2.4.x and trunk get the benefit of the patches (and > stay stable). > > If you're really ambition, before step a make your own test-case that > tests exactly the bug you're fixing, and add it to the appropriate > testing classes. That way this bug won't ever come back (as long as > people do all 6 of the above steps!) > > > Thanks much for finding these bugs and taking the time to report them in > such detail to the list! We really appreciate it. > > Go Sox, > --saul > > > On Sun, 2007-10-28 at 22:59 -0700, Justin Deoliveira wrote: > >> Also it is always nice to have the patch in a diff format that can be >> readily applied. As for the patches they look reasonable to me. I >> believe Saul has become the defacto maintainer for the filter >> capabilities stuff on main so you may want to get his review before >> committing. >> >> -Justin >> >> Cameron Shorter wrote: >> >>> Stefan, >>> For these 2 issues you have found, I suggest you create an issue in the >>> Geoserver issue tracker. >>> I understand that you have fixed these issues in your branch. I suggest >>> you reference the svn version you committed which makes it easier to review. >>> Be specific about what you would like, and how you can help. In this >>> case, ask if it is ok if you can commit this patch back to the trunk, or >>> if someone else can do it for you. >>> >>> Geotools, >>> I suggest requesting codehaus track geotools with >>> http://fisheye.codehaus.org/ >>> It provides lots of useful metics and web based svn diffs. >>> >>> Stefan Hansen wrote: >>> >>>> Dear list! >>>> >>>> I discovered a small problem in the function visit(GeometryFilter >>>> filter) of >>>> org.geotools.filter.visitor.PostPreProcessFilterSplittingVisitor.java. >>>> >>>> If the given filter is a FilterType.GEOMETRY_BBOX containing a BBOX that >>>> lies completely outside the maxbox, the BBOX in the filter gets altered. >>>> Because of this, sometimes features are returned, that are not in the >>>> requested BBOX. >>>> >>>> Here is the code snippet of which I think causes the problem: >>>> >>>> if(bbox!=null){ >>>> boolean changed = false; >>>> double minx,miny,maxx,maxy; >>>> minx = bbox.getMinX(); >>>> miny = bbox.getMinY(); >>>> maxx = bbox.getMaxX(); >>>> maxy = bbox.getMaxY(); >>>> if(minx < maxbbox.getMinX()){ >>>> minx = maxbbox.getMinX(); >>>> changed = true; >>>> } >>>> if(maxx > maxbbox.getMaxX()){ >>>> maxx = maxbbox.getMaxX(); >>>> changed = true; >>>> } >>>> if(miny < maxbbox.getMinY()){ >>>> miny = maxbbox.getMinY(); >>>> changed = true; >>>> } >>>> if(maxy > maxbbox.getMaxY()){ >>>> maxy = maxbbox.getMaxY(); >>>> changed = true; >>>> } >>>> if(changed){ >>>> Envelope tmp = new >>>> Envelope(minx,maxx,miny,maxy); >>>> try { >>>> le.setLiteral((new >>>> GeometryFactory()).toGeometry(tmp)); >>>> } catch (IllegalFilterException e) >>>> { >>>> logger.warning(e.toString()); >>>> } >>>> } >>>> >>>> And here is my solution, which is supposed to alter the BBOX only if it >>>> overlaps the maxbox: >>>> >>>> if(bbox!=null){ >>>> logger.warning("sh: GeometryFilter: >>>> BBOX!=null"); >>>> boolean changed = false; >>>> double minx,miny,maxx,maxy; >>>> minx = bbox.getMinX(); >>>> miny = bbox.getMinY(); >>>> maxx = bbox.getMaxX(); >>>> maxy = bbox.getMaxY(); >>>> if(minx < maxbbox.getMinX() && maxx >>>> > maxbbox.getMinX()){ >>>> minx = maxbbox.getMinX(); >>>> changed = true; >>>> } >>>> if(maxx > maxbbox.getMaxX() && minx >>>> < maxbbox.getMaxX()){ >>>> maxx = maxbbox.getMaxX(); >>>> changed = true; >>>> } >>>> if(miny < maxbbox.getMinY() && maxy >>>> > maxbbox.getMinY()){ >>>> miny = maxbbox.getMinY(); >>>> changed = true; >>>> } >>>> if(maxy > maxbbox.getMaxY() && miny >>>> > maxbbox.getMaxY()){ >>>> maxy = maxbbox.getMaxY(); >>>> changed = true; >>>> } >>>> if(changed){ >>>> logger.warning("sh: >>>> GeometryFilter: Changed true"); >>>> Envelope tmp = new >>>> Envelope(minx,maxx,miny,maxy); >>>> try { >>>> le.setLiteral((new >>>> GeometryFactory()).toGeometry(tmp)); >>>> } catch (IllegalFilterException e) >>>> { >>>> logger.warning(e.toString()); >>>> } >>>> } >>>> >>>> cheers, >>>> stefan >>>> >>>> ------------------------------------------------------------------------- >>>> This SF.net email is sponsored by: Splunk Inc. >>>> Still grepping through log files to find problems? Stop. >>>> Now Search log events and configuration files using AJAX and a browser. >>>> Download your FREE copy of Splunk now >> http://get.splunk.com/ >>>> _______________________________________________ >>>> Geotools-devel mailing list >>>> [email protected] >>>> https://lists.sourceforge.net/lists/listinfo/geotools-devel >>>> >>>> >>>> >>> >> > > > ------------------------------------------------------------------------- > This SF.net email is sponsored by: Splunk Inc. > Still grepping through log files to find problems? Stop. > Now Search log events and configuration files using AJAX and a browser. > Download your FREE copy of Splunk now >> http://get.splunk.com/ > _______________________________________________ > Geotools-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/geotools-devel > > > ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
