Thanks for the proposal Roar:
The GeoTools wiki is not restricted, we can consider setting that up as
needed.
If I understand correctly the proposal is:
- boolean *isOK()*: returns true for HTTP status 200
- not quite sure if you want to consider some of the other success
codes such as 201 here also?
- Can/should this handle redirects?
- InputStream *getErrorStream()*: provides the error response content
when isOK() false
- Unclear what this does if isOK() true? Do you wish to throw an
exception to indicate error stream is not available?
- InputStream *getResponseStream()*: provides the response when isOK()
is true, or throws an IOException (presumably with the contents of the
error response?)
Glad your proposal includes examples:
- Where we would parse a ServiceException: Before and after show different
logic (the first assumes failure)
- Where we can't handle the error response.: Does not show use
of getErrorStream()
Let me try and write an example that uses both getErrorStream() and
getResponseStream() to see if I understand the proposal:
try {
if (httpResponse.isOK()) {
return
ImageIOExt.readBufferedImage(httpResponse.getResponseStream());
}
else {
if
("application/vnd.ogc.se_xml".equalsIgnoreCase(httpResponse.getContentType()))
{
throw parseXMLException(httpResponse.getErrorStream());
}
else if
(httpResponse.getContentType().toLowerCase().startsWith("text/")){
throw parseException(httpResponse.getErrorStream());
}
else {
throw new IOException("Server returned error on request");
}
} finally {
httpResponse.dispose();
}
Feedback:
- This is an HTTP Request, can we just see the response code? If we had a
status code isOK() becomes making a check against
Arrays.asList(200,201,203);
- Your ticket examples with HTTP 400 and 415 requests do not seem to be
addressed here?
- Tempting to document httpResponse.getContentType() as returning lowercase
to avoid requiring equalsIgnoreCase() and toLowerCase() be used by all
client code ever
- Having two method names is difficult, and it maybe easier for users of
the API if you added httpResponse.getResponseStream(boolean
allowsErrorResponse)
- Even better would be httpResponse.getResponseStream(int httpStatusCode )
providing you make the status code available
Putting these ideas together:
try {
if (httpResponse.getStatus() == 200) {
return
ImageIOExt.readBufferedImage(httpResponse.getResponseStream());
}
if (httpResponse.getStatus() == 400) {
if
("application/vnd.ogc.se_xml".equals(httpResponse.getContentType())) {
String message =
parseXmlException(httpResponse.getErrorStream(400));
throw new HttpException(400,message);
}
else if (httpResponse.getContentType().startsWith("text/")){
String message = parseTextException(
httpResponse.getErrorStream(400));
throw new HttpException(400,message);
}
}
throw new HttpException(httpResponse.getStatus(),"Couldn't get image of
tile: " + tile.getId());
} finally {
httpResponse.dispose();
}
Food for thought.
--
Jody Garnett
On Wed, 25 May 2022 at 00:34, Roar Brænden <[email protected]>
wrote:
> Hi,
>
> I have a Proposal for the gt-http library. More specific the HTTPResponse
> interface. I followed the instructions at the Wiki page, and created a New
> Page, but when I clicked Save Page I didn't get a Pull Request as the
> instructions indicated. It seems like the Page was created and freely
> visible.
>
>
> https://github.com/geotools/geotools/wiki/HTTPResponse-handling-error-responses
>
> Could you please have a look, and maybe give some feedback?
>
> Best regards,
> Roar Brænden
>
>
> _______________________________________________
> GeoTools-Devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>
_______________________________________________
GeoTools-Devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
