Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 4.7, 6.0
    • Fix Version/s: 4.7, 6.0
    • Component/s: modules/spatial
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      A user reported a fairly serious issue affecting the latest version of Spatial4j 0.4

      https://github.com/spatial4j/spatial4j/issues/72 "JtsSpatialContextFactory and geo=false with worldBounds fails"

      I've already fixed this locally and I'll push out a bug-fix Spatial4j 0.4.1. Upgrading will be a complete drop-in replacement. Heck I could do that now but as I work with the user who found the bug I want to see if there's any other problem.

        Activity

        Hide
        David Smiley added a comment -

        I'd just like to take a moment to clarify some things for Simon Willnauer – we were chatting in IRC but needed to sign off for the night. Others may be curious.

        Firstly, Spatial4j has its own tests using the same extensive randomization methodology & actual libraries (JUnit & RandomizedTesting) that Lucene uses, and it's run by CI – TravisCI. I fixed the aforementioned bug and it has a test now to catch potential regressions. Even if Lucene could test this particular bug (it can't due to needing JTS; see next paragraph), I don't think that it's needed since it's already tested somewhere (Spatial4j's tests itself). That said, I'd love to incorporate some JTS using tests in Lucene that exercise various things at once (sort of an integration test) including definitely what this bug is related to.

        Until JTS is relicensed to Eclipse Public License (which is pending to occur sometime this year) Lucene can't use it, and AFAIK we're not even permitted to put it on the test classpath (right?). It wouldn't be a test compile-dependency; it needs to be available at runtime so Spatial4j can use it. Perhaps Lucene's Jenkins can put it on the classpath when it runs tests? Uwe Schindler what do you think? There does exist one test which uses assumeTrue(...check for JTS...);JtsPolygonTest and in practice they never get run.

        Show
        David Smiley added a comment - I'd just like to take a moment to clarify some things for Simon Willnauer – we were chatting in IRC but needed to sign off for the night. Others may be curious. Firstly, Spatial4j has its own tests using the same extensive randomization methodology & actual libraries (JUnit & RandomizedTesting) that Lucene uses, and it's run by CI – TravisCI. I fixed the aforementioned bug and it has a test now to catch potential regressions. Even if Lucene could test this particular bug (it can't due to needing JTS; see next paragraph), I don't think that it's needed since it's already tested somewhere (Spatial4j's tests itself). That said, I'd love to incorporate some JTS using tests in Lucene that exercise various things at once (sort of an integration test) including definitely what this bug is related to. Until JTS is relicensed to Eclipse Public License (which is pending to occur sometime this year) Lucene can't use it, and AFAIK we're not even permitted to put it on the test classpath (right?). It wouldn't be a test compile-dependency; it needs to be available at runtime so Spatial4j can use it. Perhaps Lucene's Jenkins can put it on the classpath when it runs tests? Uwe Schindler what do you think? There does exist one test which uses assumeTrue(...check for JTS...); – JtsPolygonTest and in practice they never get run.
        Show
        David Smiley added a comment - https://svn.apache.org/repos/asf/lucene/dev/trunk@1569650 https://svn.apache.org/repos/asf/lucene/dev/branches/branch_4x@1569652 https://svn.apache.org/repos/asf/lucene/dev/branches/lucene_solr_4_7@1569653

          People

          • Assignee:
            David Smiley
            Reporter:
            David Smiley
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Due:
              Created:
              Updated:
              Resolved:

              Development