turbomanage / basic-http-client

Automatically exported from code.google.com/p/basic-http-client
73 stars 25 forks source link

No way to determine the HTTP response code without reading the the stream when using a RequestHandler #10

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The current implementation of AbstractHttpClient uses 
readInputStream(HttpURLConnection) to read the input stream. This method can 
delegate the steam to a RequestHandler in order to avoid creating a byte[] or a 
String and directly read the stream :

    protected HttpResponse readInputStream(HttpURLConnection urlConnection) throws Exception {
        InputStream in = null;
        byte[] responseBody = null;
        try {
            in = urlConnection.getInputStream();
            if (in != null) {
                responseBody = requestHandler.readStream(in);
            }
            return new HttpResponse(urlConnection, responseBody);
        } finally {
            if (in != null) {
                try {
                    in.close();
                } catch (Exception e) {
                    // Swallow to avoid dups
                }
            }
        }
    }

In case the status code is not okay, there may be no need to read the stream. 
Unfortunately there is no way to get the status/HTTP response code prior 
starting reading the stream. 

Original issue reported on code.google.com by cyrilmot...@gmail.com on 11 Jan 2013 at 1:41

GoogleCodeExporter commented 9 years ago
Good catch. I seem to remember that in some cases, the response code is not 
available until after opening the input stream, but I can't remember which. At 
any rate, for 200 and 404, it is available, so it should be possible to decide 
beforehand to ignore the input stream in these cases. Would it be helpful to 
move the whole readInputStream() method into the RequestHandler? Or is it a 
good enough workaround to override readInputStream() in AbstractHttpClient?

I distinctly remember that I wanted AbstractHttpClient to be responsible for 
all the exception handling around opening and reading an input stream, which is 
why I structured the RequestHandler interface as it is; however, I think I 
could be convinced to move readInputStream() and writeOutputStream() into 
BasicRequestHandler instead.

Original comment by turboman...@gmail.com on 11 Jan 2013 at 4:15

GoogleCodeExporter commented 9 years ago
I think letting clients overriding AbstractHttpClient to change the behavior of 
readInputStream(HttpURLConnection) would be a bad idea because it would 
basically involve copy-paste the implementation of the method and it would also 
force the developer to recreate AndroidHttpClient, AsyncHttpClient, etc.

I like the idea of delegating this behavior to a RequestHandler interface. 
Moving readInputStream and writeOutputStream there would be an option but it 
doesn't change the fact developers will do the work on their own.

Would it be possible to have a new readStream(HttpResponse, InputStream) method 
that is called with all the necessary information already filled in 
HttpResponse (except the body). After all, when using a custom RequestHandler 
you usually don't want to return a byte[] and end up returning null ...

Original comment by cyrilmot...@gmail.com on 11 Jan 2013 at 4:26

GoogleCodeExporter commented 9 years ago
I added openInput() and openOutput() methods to RequestHandler and checked 
these into trunk. Will update the jar to 0.87 also.

Original comment by turboman...@gmail.com on 11 Jan 2013 at 8:26