GeoServer
  1. GeoServer
  2. GEOS-4753

RESTConfig doesn't calculate Coverage grid/dimensions

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.1
    • Fix Version/s: 2.1.3, 2.2.x
    • Component/s: Configuration, REST
    • Labels:
      None
    • Testcase included:
      yes
    • Patch Submitted:
      Yes
    • Number of attachments :
      9

      Description

      Expanding on GEOS-4596, 'grid' and 'dimensions' are also not calculated for Coverages created via the REST API. And WCS DescribeCoverage/etc all explode when grid/dimensions/etc aren't set.

      Collections like supportedFormats/dimensions/etc are set to null rather than the default empty List from CoverageInfoImpl if they're not specified in the POST data. This happens because XStreamPersister doesn't run the constructor when unmarshalling - the object gets magicked up with no attributes setup.

      The attached patch:

      • makes ResourceInfoConverter, FeatureTypeInfoConverter, and CoverageInfoConverter setup empty lists on unmarshalling the same way the default implementation constructors do. So not specifying eg. supportedFormats will leave your coverageInfo object with an empty list rather than null - the same as if you instantiated a CoverageInfoImpl object directly.
      • move most of the logic from CatalogBuiler.buildCoverage() (called via the Web UI) to CatalogBuilder.initCoverage() (was called by REST after GEOS-4596, now by both). Wrap the calculations/derivations so they only happen if the values aren't set already. This allows you to override via REST, but the defaults will now be sensible - the same as if you created the coverage via the Web UI.
      • enable CoverageTest.testPostAsJSON() again, and set it up to use the same data as testPostAsXML() - but auto-specify nothing so it's all calculated automatically, then assert against the calculated values.

      rest_coverageinfo.contentOnly.patch has no whitespace changes included for easier reviewing, and rest_coverageinfo.patch has everything formatted with the GeoTools style.

      1. geos-4753-coverageinfo.patch
        27 kB
        Tony Young
      2. geos-4753-coverageinfo-v2.patch
        9 kB
        Tony Young
      3. geos-4753-coverageinfo-v3.patch
        11 kB
        Tony Young
      4. geos-4753-coverageinfo-v3.patch
        11 kB
        Tony Young
      5. geos-4753-coverageinfo-v4.patch
        3 kB
        Tony Young
      6. geos-4753-coverageinfo-v5.patch
        2 kB
        Tony Young
      7. geos-4753-coverageinfo-v5.patch
        2 kB
        Tony Young
      8. rest_coverageinfo.contentOnly.patch
        24 kB
        Robert Coup
      9. rest_coverageinfo.patch
        29 kB
        Robert Coup

        Activity

        Hide
        Justin Deoliveira added a comment -

        Hi Tony,

        No problem, thanks for the continued effort. Looking at the new patch I notice that essentially the call to CoverageInfo.setGrid() has been moved from buildCoverage() to initCoverage(). This seems like a potential regression since buildCoverage() does not call initCoverage(), so any call to buildCoverage() will now result in a null grid. So I am thinking we should just leave that line in there. Let me know if I have missed something. Other than that the patch looks good to me. Thanks!

        -Justin

        Show
        Justin Deoliveira added a comment - Hi Tony, No problem, thanks for the continued effort. Looking at the new patch I notice that essentially the call to CoverageInfo.setGrid() has been moved from buildCoverage() to initCoverage(). This seems like a potential regression since buildCoverage() does not call initCoverage(), so any call to buildCoverage() will now result in a null grid. So I am thinking we should just leave that line in there. Let me know if I have missed something. Other than that the patch looks good to me. Thanks! -Justin
        Hide
        Tony Young added a comment -

        Add setGrid to CatalogBuilder in case it is null.

        Show
        Tony Young added a comment - Add setGrid to CatalogBuilder in case it is null .
        Hide
        Tony Young added a comment -

        Add setGrid to CatalogBuilder in case it is null.

        Show
        Tony Young added a comment - Add setGrid to CatalogBuilder in case it is null .
        Hide
        Justin Deoliveira added a comment -

        Patch applied. Thanks again!

        Show
        Justin Deoliveira added a comment - Patch applied. Thanks again!
        Hide
        Andrea Aime added a comment -

        Mass transition all resolved issue that did not see any further comment in the last month to closed status

        Show
        Andrea Aime added a comment - Mass transition all resolved issue that did not see any further comment in the last month to closed status

          People

          • Assignee:
            Justin Deoliveira
            Reporter:
            Robert Coup
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: