jira.codehaus.org

  • Log In Access more options
    • Online Help
    • Keyboard Shortcuts
    • About JIRA
    • JIRA Credits
    • What?s New
  • Dashboards Access more options (Alt+d)
  • Projects Access more options (Alt+p)
  • Issues Access more options (Alt+i)
Signup
GeoTools
  • GeoTools
  • GEOT-3565 Use MapContent in GTRenderer and Stre...
  • GEOT-3658

Layer to support methods expected by Renderers

  • Log In
  • Views
    • XML
    • Word
    • Printable

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 8.0-M1
  • Component/s: None
  • Labels:
    None

Description

Hey guys - I am going to try something. I am worried that I have been too pig headed about this. The real motivation for Layer was to make the constructors make sense - not to make Andrea's life difficult. I think with my object-oriented hat on, wanting each class to have a minimal responsibility, I have made things too difficult / risky.

I am going to look at pulling the getStyle() and getFeatureSource() and getQuer() methods up into Layer (and having them return "empty" content). I can explain in the javadocs that these methods are used by feature based renderers; and the default implementation only needs to be overridden if your Layer has something to say.

I would hope this would avoid the amazing amount of casts witnessed in Andrea's and Micheal's patches. I don't think it impacts on the clarity of the interfaces; the constructors are still clear; we still have concrete implementations each of which has their own specific "workarounds" for providing a FeatureSource - and since it is not tired up in conditional code I hope it would be readable.

  • Options
    • Sort By Name
    • Sort By Date
    • Ascending
    • Descending
    • Download All

Attachments

  1. Text File
    geot-3658.patch
    18/Jun/11 1:42 AM
    17 kB
    Jody Garnett
  2. Text File
    layer_defaults.patch
    18/Jun/11 10:32 PM
    7 kB
    Jody Garnett

Issue Links

supercedes

Sub-task - The sub-task of the issue GEOT-3626 Introduce StyleLayer to allow Renderer to grab SLD

  • Major - Major loss of function.
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Sub-task - The sub-task of the issue GEOT-3627 Introduce RasterLayer abstract class

  • Major - Major loss of function.
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Activity

Ascending order - Click to sort in descending order
  • All
  • Comments
  • History
  • Activity
Hide
Permalink
Jody Garnett added a comment - 18/Jun/11 1:24 AM
Okay the result was clean; and allowed me to clean up the conditional code in MapLayer (so double win). I will attach a patch as soon as I figure out how to stop arguing with git.

Andrea I hope this goes a long ways towards cutting down on the risk associated with this transition.
Show
Jody Garnett added a comment - 18/Jun/11 1:24 AM Okay the result was clean; and allowed me to clean up the conditional code in MapLayer (so double win). I will attach a patch as soon as I figure out how to stop arguing with git. Andrea I hope this goes a long ways towards cutting down on the risk associated with this transition.
Hide
Permalink
Jody Garnett added a comment - 18/Jun/11 1:38 AM
We may be able to remove the StyleLayer and RasterLayer interfaces after this change (as they were both half measures to address this issue).
Show
Jody Garnett added a comment - 18/Jun/11 1:38 AM We may be able to remove the StyleLayer and RasterLayer interfaces after this change (as they were both half measures to address this issue).
Hide
Permalink
Jody Garnett added a comment - 18/Jun/11 1:42 AM
Patch adds support for:

* Layer.getStyle()
* Layer.getQuery()
* Layer.getFeatureSource()

With associated implementation in the subclasses; and cleanup in MapLayer to avoid duplication.
Show
Jody Garnett added a comment - 18/Jun/11 1:42 AM Patch adds support for: * Layer.getStyle() * Layer.getQuery() * Layer.getFeatureSource() With associated implementation in the subclasses; and cleanup in MapLayer to avoid duplication.
Hide
Permalink
Andrea Aime added a comment - 18/Jun/11 3:30 AM - edited
Scratch scratch... this basically means I have to ditch that patch and start over, as most of the work was actually the casts, and I guess replace it with null checks instead...
Show
Andrea Aime added a comment - 18/Jun/11 3:30 AM - edited Scratch scratch... this basically means I have to ditch that patch and start over, as most of the work was actually the casts, and I guess replace it with null checks instead...
Hide
Permalink
Jody Garnett added a comment - 18/Jun/11 9:42 AM
Note the old MapLayer code did return null (the patch here preserves exactly the MapLayer functionality).

Caught up with Andrea on IRC:

1) I am going to try and return null objects so that null checks are not needed; the only one I am worried about is an "Empty" FeatureSource - I will need to make an place holder feature type of some sort.
2) I am going to look into Andrea's GeoServer patch and see if it is faster to apply it; and then remove the casts (as they can at least be searched for easily).
Show
Jody Garnett added a comment - 18/Jun/11 9:42 AM Note the old MapLayer code did return null (the patch here preserves exactly the MapLayer functionality). Caught up with Andrea on IRC: 1) I am going to try and return null objects so that null checks are not needed; the only one I am worried about is an "Empty" FeatureSource - I will need to make an place holder feature type of some sort. 2) I am going to look into Andrea's GeoServer patch and see if it is faster to apply it; and then remove the casts (as they can at least be searched for easily).
Hide
Permalink
Jody Garnett added a comment - 18/Jun/11 10:32 PM
The above patch updates layer so it does not return null for anything; while geotools builds I have not been able to get geoserver tests to pass using this patch.
Show
Jody Garnett added a comment - 18/Jun/11 10:32 PM The above patch updates layer so it does not return null for anything; while geotools builds I have not been able to get geoserver tests to pass using this patch.
Hide
Permalink
Jody Garnett added a comment - 18/Jun/11 10:34 PM - edited
Scratch that the geoserver tests are failing due to :

java.lang.AbstractMethodError: org.apache.xerces.dom.DocumentImpl.getXmlStandalone()Z
at com.sun.org.apache.xalan.internal.xsltc.trax.DOM2TO.setDocumentInfo(DOM2TO.java:373)
at com.sun.org.apache.xalan.internal.xsltc.trax.DOM2TO.parse(DOM2TO.java:127)
at com.sun.org.apache.xalan.internal.xsltc.trax.DOM2TO.parse(DOM2TO.java:94)
at com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.transformIdentity(TransformerImpl.java:661)
at com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.transform(TransformerImpl.java:707)
at com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.transform(TransformerImpl.java:313)
at org.geoserver.data.CatalogWriter.write(CatalogWriter.java:253)
Show
Jody Garnett added a comment - 18/Jun/11 10:34 PM - edited Scratch that the geoserver tests are failing due to : java.lang.AbstractMethodError: org.apache.xerces.dom.DocumentImpl.getXmlStandalone()Z at com.sun.org.apache.xalan.internal.xsltc.trax.DOM2TO.setDocumentInfo(DOM2TO.java:373) at com.sun.org.apache.xalan.internal.xsltc.trax.DOM2TO.parse(DOM2TO.java:127) at com.sun.org.apache.xalan.internal.xsltc.trax.DOM2TO.parse(DOM2TO.java:94) at com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.transformIdentity(TransformerImpl.java:661) at com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.transform(TransformerImpl.java:707) at com.sun.org.apache.xalan.internal.xsltc.trax.TransformerImpl.transform(TransformerImpl.java:313) at org.geoserver.data.CatalogWriter.write(CatalogWriter.java:253)
Hide
Permalink
Andrea Aime added a comment - 19/Jun/11 1:40 AM
Jody, you should not be building with java 6
Show
Andrea Aime added a comment - 19/Jun/11 1:40 AM Jody, you should not be building with java 6
Hide
Permalink
Andrea Aime added a comment - 19/Jun/11 1:41 AM
Oh btw, to avoid that error xerces has to be removed from the geoserver classpath
Show
Andrea Aime added a comment - 19/Jun/11 1:41 AM Oh btw, to avoid that error xerces has to be removed from the geoserver classpath

People

  • Assignee:
    Jody Garnett
    Reporter:
    Jody Garnett
Vote (0)
Watch (2)

Dates

  • Created:
    18/Jun/11 12:26 AM
    Updated:
    02/Jul/11 8:50 PM
    Resolved:
    02/Jul/11 8:50 PM
  • Atlassian JIRA (v5.2.7#850-sha1:b2af0c8)
  • Report a problem
  • Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.