Feature/wcs-client #89
Conversation
…, add tests for that
| * Check if the coverage appears in the GetCapabilities response | ||
| */ | ||
| public boolean servesCoverage(String coverageId) { | ||
| if (coverageId == null || coverageId.length() == 0) { |
There was a problem hiding this comment.
String.isEmpty() is used in other places of Oskari. No biggie - just a heads up.
| * Check if the format appears in the GetCapabilities response | ||
| */ | ||
| public boolean supportsFormat(String format) { | ||
| if (format == null || format.length() == 0) { |
There was a problem hiding this comment.
Same here with length() == 0 -> isEmpty(). I'll stop mentioning for the rest of these :)
| return new Envelope(srsName, srsDimension, axisLabels, uomLabels, lowerCorner, upperCorner); | ||
| } | ||
|
|
||
| public static Optional<double[]> parseDoubleArray(String str, char c) { |
There was a problem hiding this comment.
These parse methods (parseDoubleArray, parseIntArray, parseInt) are very similar to what ConversionHelper already has, but using Optionals. I would suggest that these should be in ConversionHelper, but that would mean a load of dependencies that you don't otherwise need.
Should we add a new utility-module that has no dependencies and could hold classes with these kinds of functions?
| return CommonParser.parsePoint(pointE, dimension); | ||
| } | ||
|
|
||
| private static String[] parseAxisLables(Element gridEnvelopeE, int dimension) { |
There was a problem hiding this comment.
Typo here parseAxisLables() -> parseAxisLabels()
| * @param get true if you want the GET endpoint, false if POST | ||
| * @return the endpoint which might not exist for your binding | ||
| */ | ||
| public static Optional<String> getDescribeCoverageEndPoint(Capabilities wcs, boolean get) { |
There was a problem hiding this comment.
Not really sure how I feel about the name "get". How would you feel about "useGet" or having two methods: one for get and another for post?
|
|
||
| public class CapabilitiesParser { | ||
|
|
||
| public static final String ALLOWED_VERSION = "2.0.1"; |
There was a problem hiding this comment.
Should this be used in all the code for WCS here having the "2.0.1" version as string to make it easier to find where version references are?
| return Maps.of( | ||
| "service", "WCS", | ||
| "version", "2.0.1", | ||
| "request", "DescribeCoverage", |
There was a problem hiding this comment.
Should DescribeCoverage, GetCapabilities etc be declared as an enum and used here (and other similar places)?
| return join(values, ','); | ||
| } | ||
|
|
||
| private static String join(String[] a, char c) { |
There was a problem hiding this comment.
Might as well be a public util-method in another class?
|
|
||
| public static Document readDocument(InputStream in) throws ParserConfigurationException, | ||
| SAXException, IOException { | ||
| DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); |
There was a problem hiding this comment.
Consider configuring dbf with some XEE prevention:
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Prevention_Cheat_Sheet
| import org.w3c.dom.Node; | ||
| import org.xml.sax.SAXException; | ||
|
|
||
| public class XML { |
There was a problem hiding this comment.
This class feels like it should be in that no-dependencies util-module that I mentioned in a comment above.
|
Nice work! Check the comments, but looks like this is good for merging. |
…one for each binding (GET and POST)
…eference to that where it makes sense
…module, optimize logic and add unit tests for those
Groundwork for WCS client library module
Support for creating WCS GetCapabilities, DescribeCoverage, GetCoverage requests
Support for parsing WCS GetCapabilities and DescribeCoverage responses