Cleanup GlideUrl/Headers with better equals() imll

library/src/main/java/com/bumptech/glide/load/model/GlideUrl.java

@@ -25,8 +25,9 @@ public class GlideUrl {
 
     private final URL url;
     private final Headers headers;
-    private String stringUrl;
+    private final String stringUrl;
 
+    private String safeStringUrl;
     private URL safeUrl;
 
     public GlideUrl(URL url) {
@@ -41,6 +42,9 @@ public class GlideUrl {
         if (url == null) {
             throw new IllegalArgumentException("URL must not be null!");
         }
+        if (headers == null) {
+            throw new IllegalArgumentException("Headers must not be null");
+        }
         this.url = url;
         stringUrl = null;
         this.headers = headers;
@@ -50,6 +54,9 @@ public class GlideUrl {
         if (TextUtils.isEmpty(url)) {
             throw new IllegalArgumentException("String url must not be empty or null: " + url);
         }
+        if (headers == null) {
+            throw new IllegalArgumentException("Headers must not be null");
+        }
         this.stringUrl = url;
         this.url = null;
         this.headers = headers;
@@ -63,11 +70,9 @@ public class GlideUrl {
     // using it would require both decoding and encoding each string which is more complicated, slower and generates
     // more objects than the solution below. See also issue #133.
     private URL getSafeUrl() throws MalformedURLException {
-        if (safeUrl != null) {
-            return safeUrl;
+        if (safeUrl == null) {
+            safeUrl = new URL(getSafeStringUrl());
         }
-
-        safeUrl = new URL(getSafeStringUrl());
         return safeUrl;
     }
 
@@ -76,10 +81,14 @@ public class GlideUrl {
     }
 
     private String getSafeStringUrl() {
-        if (TextUtils.isEmpty(stringUrl)) {
-            stringUrl = url.toString();
+        if (TextUtils.isEmpty(safeStringUrl)) {
+            String unsafeStringUrl = stringUrl;
+            if (TextUtils.isEmpty(unsafeStringUrl)) {
+                unsafeStringUrl = url.toString();
+            }
+            safeStringUrl = Uri.encode(unsafeStringUrl, ALLOWED_URI_CHARS);
         }
-        return Uri.encode(stringUrl, ALLOWED_URI_CHARS);
+        return safeStringUrl;
     }
 
     public Map<String, String> getHeaders() {
@@ -91,29 +100,29 @@ public class GlideUrl {
         String urlString = getSafeStringUrl();
         StringBuilder stringBuilder = new StringBuilder(urlString);
         Map<String, String> headerMap = headers.getHeaders();
-        for (String key : headerMap.keySet()) {
-            stringBuilder.append("\n")
-                .append(key)
+        for (Map.Entry<String, String> entry : headerMap.entrySet()) {
+            stringBuilder.append('\n')
+                .append(entry.getKey())
                 .append(": ")
-                .append(headerMap.get(key));
+                .append(entry.getValue());
         }
         return stringBuilder.toString();
     }
 
     @Override
     public boolean equals(Object o) {
-        if (this == o) {
-            return true;
+        if (o instanceof GlideUrl) {
+          GlideUrl other = (GlideUrl) o;
+          return getSafeStringUrl().equals(other.getSafeStringUrl())
+              && headers.equals(other.headers);
         }
-        if (o == null || getClass() != o.getClass()) {
-            return false;
-        }
-
-        return toString().equals(o.toString());
+        return false;
     }
 
     @Override
     public int hashCode() {
-        return toString().hashCode();
+        int hashCode = getSafeStringUrl().hashCode();
+        hashCode = 31 * hashCode + headers.hashCode();
+        return hashCode;
     }
 }

library/src/main/java/com/bumptech/glide/load/model/Headers.java

 package com.bumptech.glide.load.model;
 
+import android.text.TextUtils;
+
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -9,14 +11,14 @@ import java.util.Set;
 /**
  * A wrapper class for a set of headers to be included in a Glide request.
  */
-public class Headers {
+public final class Headers {
 
     public static final Headers NONE = new Builder().build();
 
     private final Map<String, Set<String>> headers;
     private volatile Map<String, String> combinedHeaders;
 
-    private Headers(Map<String, Set<String>> headers) {
+    Headers(Map<String, Set<String>> headers) {
         this.headers = Collections.unmodifiableMap(headers);
     }
 
@@ -24,20 +26,7 @@ public class Headers {
         if (combinedHeaders == null) {
             synchronized (this) {
                 if (combinedHeaders == null) {
-                    Map<String, String> combinedHeaders = new HashMap<>();
-
-                    for (String key : headers.keySet()) {
-                        StringBuilder stringBuilder = new StringBuilder();
-                        for (String value : headers.get(key)) {
-                            stringBuilder.append(",").append(value);
-                        }
-                        if (stringBuilder.length() > 0) {
-                            stringBuilder.deleteCharAt(0);
-                            combinedHeaders.put(key, stringBuilder.toString());
-                        }
-                    }
-
-                    this.combinedHeaders = Collections.unmodifiableMap(combinedHeaders);
+                    this.combinedHeaders = generateCombinedHeaders();
                 }
             }
         }
@@ -45,17 +34,25 @@ public class Headers {
         return combinedHeaders;
     }
 
+    private Map<String, String> generateCombinedHeaders() {
+        Map<String, String> combinedHeaders = new HashMap<String, String>();
+        for (Map.Entry<String, Set<String>> entry : headers.entrySet()) {
+            combinedHeaders.put(entry.getKey(), TextUtils.join(",", entry.getValue()));
+        }
+        return Collections.unmodifiableMap(combinedHeaders);
+    }
+
   /**
    * Builder class for {@link Headers}.
    */
-    public static class Builder {
-        private final Map<String, Set<String>> headers = new HashMap<>();
+    public static final class Builder {
+        private final Map<String, Set<String>> headers = new HashMap<String, Set<String>>();
 
         public void addHeader(String key, String value) {
             if (headers.containsKey(key)) {
                 headers.get(key).add(value);
             } else {
-                Set<String> values = new HashSet<>();
+                Set<String> values = new HashSet<String>();
                 values.add(value);
                 headers.put(key, values);
             }
@@ -65,4 +62,18 @@ public class Headers {
             return new Headers(headers);
         }
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (o instanceof Headers) {
+            Headers other = (Headers) o;
+            return headers.equals(other.headers);
+        }
+        return false;
+    }
+
+    @Override
+    public int hashCode() {
+        return headers.hashCode();
+    }
 }

library/src/main/java/com/bumptech/glide/load/model/stream/BaseGlideUrlLoader.java

@@ -80,7 +80,7 @@ public abstract class BaseGlideUrlLoader<T> implements StreamModelLoader<T> {
      * @param height The height in pixels of the view/target the image will be loaded into.
      * @return The Headers object containing the headers, or null if no headers should be added.
      */
-    protected Headers getHeaders(T model, int width, int height){
+    protected Headers getHeaders(T model, int width, int height) {
         return Headers.NONE;
     }
 }