From 34474ac18b45ec5baba1e1e9e29a330ee921845b Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Wed, 25 Feb 2015 13:06:44 -0800 Subject: [PATCH] Cleanup GlideUrl/Headers with better equals() imll --- .../bumptech/glide/load/model/GlideUrl.java | 49 +++++++++++-------- .../bumptech/glide/load/model/Headers.java | 49 ++++++++++++------- .../load/model/stream/BaseGlideUrlLoader.java | 2 +- 3 files changed, 60 insertions(+), 40 deletions(-) diff --git a/library/src/main/java/com/bumptech/glide/load/model/GlideUrl.java b/library/src/main/java/com/bumptech/glide/load/model/GlideUrl.java index a82e47ae1..58f73b8e5 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/GlideUrl.java +++ b/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 getHeaders() { @@ -91,29 +100,29 @@ public class GlideUrl { String urlString = getSafeStringUrl(); StringBuilder stringBuilder = new StringBuilder(urlString); Map headerMap = headers.getHeaders(); - for (String key : headerMap.keySet()) { - stringBuilder.append("\n") - .append(key) + for (Map.Entry 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; } } diff --git a/library/src/main/java/com/bumptech/glide/load/model/Headers.java b/library/src/main/java/com/bumptech/glide/load/model/Headers.java index 63c86c3f7..165e8d9e9 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/Headers.java +++ b/library/src/main/java/com/bumptech/glide/load/model/Headers.java @@ -1,5 +1,7 @@ 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> headers; private volatile Map combinedHeaders; - private Headers(Map> headers) { + Headers(Map> headers) { this.headers = Collections.unmodifiableMap(headers); } @@ -24,20 +26,7 @@ public class Headers { if (combinedHeaders == null) { synchronized (this) { if (combinedHeaders == null) { - Map 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 generateCombinedHeaders() { + Map combinedHeaders = new HashMap(); + for (Map.Entry> 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> headers = new HashMap<>(); + public static final class Builder { + private final Map> headers = new HashMap>(); public void addHeader(String key, String value) { if (headers.containsKey(key)) { headers.get(key).add(value); } else { - Set values = new HashSet<>(); + Set values = new HashSet(); 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(); + } } diff --git a/library/src/main/java/com/bumptech/glide/load/model/stream/BaseGlideUrlLoader.java b/library/src/main/java/com/bumptech/glide/load/model/stream/BaseGlideUrlLoader.java index 86117d894..94e5dd509 100644 --- a/library/src/main/java/com/bumptech/glide/load/model/stream/BaseGlideUrlLoader.java +++ b/library/src/main/java/com/bumptech/glide/load/model/stream/BaseGlideUrlLoader.java @@ -80,7 +80,7 @@ public abstract class BaseGlideUrlLoader implements StreamModelLoader { * @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; } } -- GitLab