Make the dependency on Apache HTTP optional

[jpd236]

Volley currently depends on Apache HTTP. We can mostly remove the dependency except that HttpStack#performRequest returns an org.apache.http.HttpResponse, which means it'd be an API-breaking change to remove.

It might be worth biting the bullet and just doing a major version bump that breaks the API. There are other known issues that we could potentially address alongside this. However, one avenue worth exploring would be to see if we could treat Apache HTTP as a "provided" dependency - that is, one which is available at compile time, but not runtime - and then deprecate the old performRequest and add a new one that is implemented by all built-in clients (except HttpClientStack, which should be deprecated as well).

I think the result would compile and let apps build against Volley without pulling in the Apache HTTP dependency, but existing users could continue to use the Apache stack as long as they pulled in the dependency at runtime (and it should be there on an actual Android device).

Apart from the ugliness, my main concern is how proguard would respond to the missing reference - we could add a consumer proguard rule to not warn on org.apache stuff though that might be a bit overbearing.

[xp-vit]

In my humble opinion, doing a major version bump worth it. Also I think that this change is really important, since HttpClient has been deprecated 2 APIs ago and in some future it may become unavailable at all.

[ngocchung]

@jpd236 can it be a workaround? https://github.com/ngocchung/VolleyNoApache/blob/master/app/src/main/java/com/android/volley/toolbox/HurlStack.java#119

[jpd236]

I'd need more details on what you're suggesting with the above, but it looks like the above code just changes the signature of HttpStack#performRequest, which is the problem noted in 1 (an API-breaking change). Doing that is the extreme option which will break every caller of Volley so like I said, I'd like to explore a more gentle approach before taking such a hard line.

[ngocchung]

@jpd236 sorry for my bad English, can you explain more about "API-breaking change"? In the code i mentioned, not only the signatures changed, and you can see dependency removed.

[jpd236]

Sure - what I mean is that there are existing apps which depend on Volley with their own subclasses of HttpStack and/or Request. If we change those classes in an incompatible way, e.g. by changing the return type of HttpStack#performRequest from HttpResponse to URLConnection, those apps will no longer compile when they upgrade.

This is painful to developers, so it's something we generally like to avoid if at all possible. At the very least, we like to provide a transition period for apps to move off the broken APIs before we remove them.

So I'm hoping that instead of just changing the library's public method signatures, we can keep the old ones, but deprecate them and add new ones to replace them.

[ngocchung]

@jpd236 I got it, many thanks :)

[xp-vit]

I think this is a right time to have 2.0 version released, which will drop HttpClient suppport and break API. It's already 17th month passed since official HttpClient deprecation. I don't think it's necessary to wait longer...

[jpd236]

The above commit should make it possible to depend on Volley without having to declare a dependency on the Apache HTTP client via the approach I outlined above as long as you're using HurlStack or another stack which doesn't depend on the Apache HTTP library (in which case you'll need to make a small tweak to extend from BaseHttpStack instead of implementing HttpStack). In my testing, the Android build tools seem to call proguard with the Apache JAR in the classpath even though the app never asked for it, so we may not have to do any proguard hacks at this point. That being said, it'd be very helpful for other app developers to try this approach to see if it meets their needs and works with their build.

If you'd like to test this, you can either patch in this change and build from source, or try this compiled aar library by unzipping the .aar file inside, putting it in your tree (say aar/) and adding this to build.gradle in place of any existing Volley dependency:

repositories {
    flatDir {
        dirs 'aar'
    }
}

dependencies {
    compile 'com.android.volley:volley-release:1.0@aar'
}

If this works well for folks, I'll move forward with getting this merged and released.

[Ericliu001]

Good to know that the dependency on Apache HttpClient is going to go.

[ihasanov]

I have editted HurlStack class , you just need to paste it to your project and you will give totally new perspective to your users:I just added between of //************************-------------- this lines

/*
 * Copyright (C) 2011 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.volley.toolbox;

import com.android.volley.AuthFailureError;
import com.android.volley.Request;
import com.android.volley.Request.Method;

import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.ProtocolVersion;
import org.apache.http.StatusLine;
import org.apache.http.entity.BasicHttpEntity;
import org.apache.http.message.BasicHeader;
import org.apache.http.message.BasicHttpResponse;
import org.apache.http.message.BasicStatusLine;

import java.io.DataOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.URL;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;

import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLSocketFactory;

/**
 * An {@link HttpStack} based on {@link HttpURLConnection}.
 */
public class HurlStack implements HttpStack {

    private static final String HEADER_CONTENT_TYPE = "Content-Type";
    //************************--------------
    //added variables and functions
    HttpURLConnection httpURLConnection=null;
    //below methods can be extended
    public HttpURLConnection getConnectionPreExecute(HttpURLConnection connection){
        return connection;
    }

    public HttpURLConnection getConnectionPostExecute(HttpURLConnection connection){
        return connection;
    }
    //************************--------------
    /**
     * An interface for transforming URLs before use.
     */
    public interface UrlRewriter {
        /**
         * Returns a URL to use instead of the provided one, or null to indicate
         * this URL should not be used at all.
         */
        String rewriteUrl(String originalUrl);
    }

    private final UrlRewriter mUrlRewriter;
    private final SSLSocketFactory mSslSocketFactory;

    public HurlStack() {
        this(null);
    }

    /**
     * @param urlRewriter Rewriter to use for request URLs
     */
    public HurlStack(UrlRewriter urlRewriter) {
        this(urlRewriter, null);
    }

    /**
     * @param urlRewriter Rewriter to use for request URLs
     * @param sslSocketFactory SSL factory to use for HTTPS connections
     */
    public HurlStack(UrlRewriter urlRewriter, SSLSocketFactory sslSocketFactory) {
        mUrlRewriter = urlRewriter;
        mSslSocketFactory = sslSocketFactory;
    }

    @Override
    public HttpResponse performRequest(Request<?> request, Map<String, String> additionalHeaders)
            throws IOException, AuthFailureError {
        String url = request.getUrl();
        HashMap<String, String> map = new HashMap<String, String>();
        map.putAll(request.getHeaders());
        map.putAll(additionalHeaders);
        if (mUrlRewriter != null) {
            String rewritten = mUrlRewriter.rewriteUrl(url);
            if (rewritten == null) {
                throw new IOException("URL blocked by rewriter: " + url);
            }
            url = rewritten;
        }
        URL parsedUrl = new URL(url);
        HttpURLConnection connection = openConnection(parsedUrl, request);
        //**********************-------
        //added
        connection = getConnectionPreExecute(connection);
        //**********************-------
        httpURLConnection = connection;
        for (String headerName : map.keySet()) {
            connection.addRequestProperty(headerName, map.get(headerName));
        }
        setConnectionParametersForRequest(connection, request);
        // Initialize HttpResponse with data from the HttpURLConnection.
        ProtocolVersion protocolVersion = new ProtocolVersion("HTTP", 1, 1);
        int responseCode = connection.getResponseCode();
        if (responseCode == -1) {
            // -1 is returned by getResponseCode() if the response code could not be retrieved.
            // Signal to the caller that something was wrong with the connection.
            throw new IOException("Could not retrieve response code from HttpUrlConnection.");
        }
        StatusLine responseStatus = new BasicStatusLine(protocolVersion,
                connection.getResponseCode(), connection.getResponseMessage());
        BasicHttpResponse response = new BasicHttpResponse(responseStatus);
        if (hasResponseBody(request.getMethod(), responseStatus.getStatusCode())) {
            response.setEntity(entityFromConnection(connection));
        }
        //**********************-------
        //added
        connection = getConnectionPostExecute(connection);
        //**********************-------
        for (Entry<String, List<String>> header : connection.getHeaderFields().entrySet()) {
            if (header.getKey() != null) {
                Header h = new BasicHeader(header.getKey(), header.getValue().get(0));
                response.addHeader(h);
            }
        }
        return response;
    }

    /**
     * Checks if a response message contains a body.
     * @see <a href="https://tools.ietf.org/html/rfc7230#section-3.3">RFC 7230 section 3.3</a>
     * @param requestMethod request method
     * @param responseCode response status code
     * @return whether the response has a body
     */
    private static boolean hasResponseBody(int requestMethod, int responseCode) {
        return requestMethod != Request.Method.HEAD
            && !(HttpStatus.SC_CONTINUE <= responseCode && responseCode < HttpStatus.SC_OK)
            && responseCode != HttpStatus.SC_NO_CONTENT
            && responseCode != HttpStatus.SC_NOT_MODIFIED;
    }

    /**
     * Initializes an {@link HttpEntity} from the given {@link HttpURLConnection}.
     * @param connection
     * @return an HttpEntity populated with data from <code>connection</code>.
     */
    private static HttpEntity entityFromConnection(HttpURLConnection connection) {
        BasicHttpEntity entity = new BasicHttpEntity();
        InputStream inputStream;
        try {
            inputStream = connection.getInputStream();
        } catch (IOException ioe) {
            inputStream = connection.getErrorStream();
        }
        entity.setContent(inputStream);
        entity.setContentLength(connection.getContentLength());
        entity.setContentEncoding(connection.getContentEncoding());
        entity.setContentType(connection.getContentType());
        return entity;
    }

    /**
     * Create an {@link HttpURLConnection} for the specified {@code url}.
     */
    protected HttpURLConnection createConnection(URL url) throws IOException {
        HttpURLConnection connection = (HttpURLConnection) url.openConnection();

        // Workaround for the M release HttpURLConnection not observing the
        // HttpURLConnection.setFollowRedirects() property.
        // https://code.google.com/p/android/issues/detail?id=194495
        connection.setInstanceFollowRedirects(HttpURLConnection.getFollowRedirects());

        return connection;
    }

    /**
     * Opens an {@link HttpURLConnection} with parameters.
     * @param url
     * @return an open connection
     * @throws IOException
     */
    private HttpURLConnection openConnection(URL url, Request<?> request) throws IOException {
        HttpURLConnection connection = createConnection(url);

        int timeoutMs = request.getTimeoutMs();
        connection.setConnectTimeout(timeoutMs);
        connection.setReadTimeout(timeoutMs);
        connection.setUseCaches(false);
        connection.setDoInput(true);

        // use caller-provided custom SslSocketFactory, if any, for HTTPS
        if ("https".equals(url.getProtocol()) && mSslSocketFactory != null) {
            ((HttpsURLConnection)connection).setSSLSocketFactory(mSslSocketFactory);
        }

        return connection;
    }

    @SuppressWarnings("deprecation")
    /* package */ static void setConnectionParametersForRequest(HttpURLConnection connection,
            Request<?> request) throws IOException, AuthFailureError {
        switch (request.getMethod()) {
            case Method.DEPRECATED_GET_OR_POST:
                // This is the deprecated way that needs to be handled for backwards compatibility.
                // If the request's post body is null, then the assumption is that the request is
                // GET.  Otherwise, it is assumed that the request is a POST.
                byte[] postBody = request.getPostBody();
                if (postBody != null) {
                    connection.setRequestMethod("POST");
                    addBody(connection, request, postBody);
                }
                break;
            case Method.GET:
                // Not necessary to set the request method because connection defaults to GET but
                // being explicit here.
                connection.setRequestMethod("GET");
                break;
            case Method.DELETE:
                connection.setRequestMethod("DELETE");
                break;
            case Method.POST:
                connection.setRequestMethod("POST");
                addBodyIfExists(connection, request);
                break;
            case Method.PUT:
                connection.setRequestMethod("PUT");
                addBodyIfExists(connection, request);
                break;
            case Method.HEAD:
                connection.setRequestMethod("HEAD");
                break;
            case Method.OPTIONS:
                connection.setRequestMethod("OPTIONS");
                break;
            case Method.TRACE:
                connection.setRequestMethod("TRACE");
                break;
            case Method.PATCH:
                connection.setRequestMethod("PATCH");
                addBodyIfExists(connection, request);
                break;
            default:
                throw new IllegalStateException("Unknown method type.");
        }
    }

    private static void addBodyIfExists(HttpURLConnection connection, Request<?> request)
            throws IOException, AuthFailureError {
        byte[] body = request.getBody();
        if (body != null) {
            addBody(connection, request, body);
        }
    }

    private static void addBody(HttpURLConnection connection, Request<?> request, byte[] body)
            throws IOException, AuthFailureError {
        // Prepare output. There is no need to set Content-Length explicitly,
        // since this is handled by HttpURLConnection using the size of the prepared
        // output stream.
        connection.setDoOutput(true);
        connection.addRequestProperty(HEADER_CONTENT_TYPE, request.getBodyContentType());
        DataOutputStream out = new DataOutputStream(connection.getOutputStream());
        out.write(body);
        out.close();
    }
}
`
```

[xp-vit]

@NewTimeBox I don't really know how your class is related to the issue discussed here. You are just offering to add Pre and Post hooks to connection. What are you offering to do in those and how this may solve issue of Apache HttpClient dependency?

[jpd236]

Initial migration guide is here: https://github.com/google/volley/wiki/Migrating-from-Apache-HTTP

This should become available in an official SNAPSHOT build shortly, though the interface is subject to change until the next official release.