Skip to content

Commit a912e0f

Browse files
committedDec 15, 2022
Support parsing package local resource uris to ids
This lets us load resource Uris with the same theming benefits that we do normal resource ids. This is really only for backwards compatibility so that we can more readily remove the old ResourceLoader and support for resource Uris in UriLoader.
1 parent 5816903 commit a912e0f

File tree

4 files changed

+329
-14
lines changed

4 files changed

+329
-14
lines changed
 

‎instrumentation/src/androidTest/java/com/bumptech/glide/DarkModeTest.java

+96-1
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@
44
import static com.bumptech.glide.testutil.BitmapSubject.assertThat;
55
import static org.junit.Assume.assumeTrue;
66

7+
import android.content.ContentResolver;
78
import android.content.Context;
9+
import android.content.res.Resources;
810
import android.graphics.Bitmap;
911
import android.graphics.drawable.BitmapDrawable;
1012
import android.graphics.drawable.Drawable;
1113
import android.os.Build.VERSION;
1214
import android.os.Build.VERSION_CODES;
15+
import android.net.Uri;
1316
import android.os.Bundle;
1417
import android.view.LayoutInflater;
1518
import android.view.View;
@@ -47,7 +50,7 @@ public void before() {
4750
assumeTrue(VERSION.SDK_INT >= VERSION_CODES.Q);
4851
}
4952

50-
// TODO(judds): The way we handle data loads in the background for resoures is not Theme
53+
// TODO(judds): The way we handle data loads in the background for resources is not Theme
5154
// compatible. In particular, the theme gets lost when we convert the resource id to a Uri and
5255
// we don't use the user provided theme. While ResourceBitmapDecoder and ResourceDrawableDecoder
5356
// will use the theme, they're not called for most resource ids because those instead go through
@@ -105,6 +108,50 @@ public void load_withDarkModeActivity_darkModeTheme_usesDarkModeDrawable() {
105108
.theme(activity.getTheme()));
106109
}
107110

111+
@Test
112+
public void loadResourceNameUri_withDarkModeActivity_darkModeTheme_usesDarkModeDrawable() {
113+
runActivityTest(
114+
darkModeActivity(),
115+
R.raw.dog_dark,
116+
activity ->
117+
Glide.with(activity)
118+
.load(newResourceNameUri(activity, R.drawable.dog))
119+
.override(Target.SIZE_ORIGINAL)
120+
.theme(activity.getTheme()));
121+
}
122+
123+
@Test
124+
public void loadResourceIdUri_withDarkModeActivity_darkModeTheme_usesDarkModeDrawable() {
125+
runActivityTest(
126+
darkModeActivity(),
127+
R.raw.dog_dark,
128+
activity ->
129+
Glide.with(activity)
130+
.load(newResourceIdUri(activity, R.drawable.dog))
131+
.override(Target.SIZE_ORIGINAL)
132+
.theme(activity.getTheme()));
133+
}
134+
135+
private static Uri newResourceNameUri(Context context, int resourceId) {
136+
Resources resources = context.getResources();
137+
return newResourceUriBuilder(context)
138+
.appendPath(resources.getResourceTypeName(resourceId))
139+
.appendPath(resources.getResourceEntryName(resourceId))
140+
.build();
141+
}
142+
143+
private static Uri newResourceIdUri(Context context, int resourceId) {
144+
return newResourceUriBuilder(context)
145+
.appendPath(String.valueOf(resourceId))
146+
.build();
147+
}
148+
149+
private static Uri.Builder newResourceUriBuilder(Context context) {
150+
return new Uri.Builder()
151+
.scheme(ContentResolver.SCHEME_ANDROID_RESOURCE)
152+
.authority(context.getPackageName());
153+
}
154+
108155
@Test
109156
public void load_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() {
110157
runFragmentTest(
@@ -117,6 +164,30 @@ public void load_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() {
117164
.theme(fragment.requireActivity().getTheme()));
118165
}
119166

167+
@Test
168+
public void loadResourceNameUri_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() {
169+
runFragmentTest(
170+
darkModeActivity(),
171+
R.raw.dog_dark,
172+
fragment ->
173+
Glide.with(fragment)
174+
.load(newResourceNameUri(fragment.requireContext(), R.drawable.dog))
175+
.override(Target.SIZE_ORIGINAL)
176+
.theme(fragment.requireActivity().getTheme()));
177+
}
178+
179+
@Test
180+
public void loadResourceIdUri_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() {
181+
runFragmentTest(
182+
darkModeActivity(),
183+
R.raw.dog_dark,
184+
fragment ->
185+
Glide.with(fragment)
186+
.load(newResourceIdUri(fragment.requireContext(), R.drawable.dog))
187+
.override(Target.SIZE_ORIGINAL)
188+
.theme(fragment.requireActivity().getTheme()));
189+
}
190+
120191
@Test
121192
public void load_withApplicationContext_darkTheme_usesDarkModeDrawable() {
122193
runActivityTest(
@@ -129,6 +200,30 @@ public void load_withApplicationContext_darkTheme_usesDarkModeDrawable() {
129200
.theme(input.getTheme()));
130201
}
131202

203+
@Test
204+
public void loadResourceNameUri_withApplicationContext_darkTheme_usesDarkModeDrawable() {
205+
runActivityTest(
206+
darkModeActivity(),
207+
R.raw.dog_dark,
208+
input ->
209+
Glide.with(input.getApplicationContext())
210+
.load(newResourceNameUri(input.getApplicationContext(), R.drawable.dog))
211+
.override(Target.SIZE_ORIGINAL)
212+
.theme(input.getTheme()));
213+
}
214+
215+
@Test
216+
public void loadResourceIdUri_withApplicationContext_darkTheme_usesDarkModeDrawable() {
217+
runActivityTest(
218+
darkModeActivity(),
219+
R.raw.dog_dark,
220+
input ->
221+
Glide.with(input.getApplicationContext())
222+
.load(newResourceIdUri(input.getApplicationContext(), R.drawable.dog))
223+
.override(Target.SIZE_ORIGINAL)
224+
.theme(input.getTheme()));
225+
}
226+
132227
@Test
133228
public void load_withApplicationContext_lightTheme_usesLightModeDrawable() {
134229
runActivityTest(

‎library/src/main/java/com/bumptech/glide/RegistryFactory.java

+11-4
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.bumptech.glide.load.model.MediaStoreFileLoader;
3333
import com.bumptech.glide.load.model.ModelLoaderFactory;
3434
import com.bumptech.glide.load.model.ResourceLoader;
35+
import com.bumptech.glide.load.model.ResourceUriLoader;
3536
import com.bumptech.glide.load.model.StreamEncoder;
3637
import com.bumptech.glide.load.model.StringLoader;
3738
import com.bumptech.glide.load.model.UnitModelLoader;
@@ -280,7 +281,12 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm
280281
.append(int.class, InputStream.class, inputStreamFactory)
281282
.append(Integer.class, InputStream.class, inputStreamFactory)
282283
.append(int.class, AssetFileDescriptor.class, assetFileDescriptorFactory)
283-
.append(Integer.class, AssetFileDescriptor.class, assetFileDescriptorFactory);
284+
.append(Integer.class, AssetFileDescriptor.class, assetFileDescriptorFactory)
285+
.append(Uri.class, InputStream.class, ResourceUriLoader.newStreamFactory(context))
286+
.append(
287+
Uri.class,
288+
AssetFileDescriptor.class,
289+
ResourceUriLoader.newAssetFileDescriptorFactory(context));
284290
} else {
285291
ResourceLoader.StreamFactory resourceLoaderStreamFactory =
286292
new ResourceLoader.StreamFactory(resources);
@@ -325,15 +331,16 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm
325331
new QMediaStoreUriLoader.FileDescriptorFactory(context));
326332
}
327333
registry
328-
.append(Uri.class, InputStream.class, new UriLoader.StreamFactory(contentResolver))
334+
.append(
335+
Uri.class, InputStream.class, new UriLoader.StreamFactory(contentResolver, experiments))
329336
.append(
330337
Uri.class,
331338
ParcelFileDescriptor.class,
332-
new UriLoader.FileDescriptorFactory(contentResolver))
339+
new UriLoader.FileDescriptorFactory(contentResolver, experiments))
333340
.append(
334341
Uri.class,
335342
AssetFileDescriptor.class,
336-
new UriLoader.AssetFileDescriptorFactory(contentResolver))
343+
new UriLoader.AssetFileDescriptorFactory(contentResolver, experiments))
337344
.append(Uri.class, InputStream.class, new UrlUriLoader.StreamFactory())
338345
.append(URL.class, InputStream.class, new UrlLoader.StreamFactory())
339346
.append(Uri.class, File.class, new MediaStoreFileLoader.Factory(context))
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
package com.bumptech.glide.load.model;
2+
3+
import android.annotation.SuppressLint;
4+
import android.content.ContentResolver;
5+
import android.content.Context;
6+
import android.content.res.AssetFileDescriptor;
7+
import android.net.Uri;
8+
import android.util.Log;
9+
import androidx.annotation.NonNull;
10+
import androidx.annotation.Nullable;
11+
import com.bumptech.glide.load.Options;
12+
import java.io.InputStream;
13+
import java.util.List;
14+
15+
/**
16+
* Converts Resource Uris to resource ids if the resource Uri points to a resource in this package.
17+
*
18+
* <p>This class really shouldn't need to exist. If you need to load resources, just pass
19+
* in the integer resource id directly using {@link com.bumptech.glide.RequestManager#load(Integer)}
20+
* instead. It'll be more correct in terms of caching and more efficient to load. The only reason
21+
* we're supporting this case is for backwards compatibility.
22+
*
23+
* @param <DataT> The type of data produced, e.g. {@link InputStream} or
24+
* {@link AssetFileDescriptor}.
25+
*/
26+
public final class ResourceUriLoader<DataT> implements ModelLoader<Uri, DataT> {
27+
/**
28+
* See the javadoc on {@link
29+
* android.content.res.Resources#getIdentifier(java.lang.String, java.lang.String, java.lang.String)}.
30+
*/
31+
private static final int INVALID_RESOURCE_ID = 0;
32+
private static final String TAG = "ResourceUriLoader";
33+
34+
private final Context context;
35+
private final ModelLoader<Integer, DataT> delegate;
36+
37+
public static ModelLoaderFactory<Uri, InputStream> newStreamFactory(Context context) {
38+
return new InputStreamFactory(context);
39+
}
40+
41+
public static ModelLoaderFactory<Uri, AssetFileDescriptor> newAssetFileDescriptorFactory(
42+
Context context) {
43+
return new AssetFileDescriptorFactory(context);
44+
}
45+
46+
ResourceUriLoader(Context context, ModelLoader<Integer, DataT> delegate) {
47+
this.context = context.getApplicationContext();
48+
this.delegate = delegate;
49+
}
50+
51+
@Nullable
52+
@Override
53+
public LoadData<DataT> buildLoadData(@NonNull Uri uri, int width, int height, @NonNull Options options) {
54+
List<String> pathSegments = uri.getPathSegments();
55+
// android.resource//<package_name>/<resource_id>
56+
if (pathSegments.size() == 1) {
57+
return parseResourceIdUri(uri, width, height, options);
58+
}
59+
// android.resource//<package_name>/<drawable>/<resource_name>
60+
if (pathSegments.size() == 2) {
61+
return parseResourceNameUri(uri, width, height, options);
62+
}
63+
if (Log.isLoggable(TAG, Log.WARN)) {
64+
Log.w(TAG, "Failed to parse resource uri: " + uri);
65+
}
66+
return null;
67+
}
68+
69+
@Nullable
70+
private LoadData<DataT> parseResourceNameUri(
71+
@NonNull Uri uri, int width, int height, @NonNull Options options) {
72+
List<String> pathSegments = uri.getPathSegments();
73+
String resourceType = pathSegments.get(0);
74+
String resourceName = pathSegments.get(1);
75+
76+
// Yes it's bad, but the caller has chosen to give us a resource uri...
77+
@SuppressLint("DiscouragedApi") int identifier =
78+
context.getResources().getIdentifier(resourceName, resourceType, context.getPackageName());
79+
if (identifier == INVALID_RESOURCE_ID) {
80+
if (Log.isLoggable(TAG, Log.WARN)) {
81+
Log.w(TAG, "Failed to find resource id for: " + uri);
82+
}
83+
return null;
84+
}
85+
86+
return delegate.buildLoadData(identifier, width, height, options);
87+
}
88+
89+
@Nullable
90+
private LoadData<DataT> parseResourceIdUri(
91+
@NonNull Uri uri, int width, int height, @NonNull Options options) {
92+
try {
93+
int resourceId = Integer.parseInt(uri.getPathSegments().get(0));
94+
if (resourceId == INVALID_RESOURCE_ID) {
95+
if (Log.isLoggable(TAG, Log.WARN)) {
96+
Log.w(TAG, "Failed to parse a valid non-0 resource id from: " + uri);
97+
}
98+
return null;
99+
}
100+
return delegate.buildLoadData(resourceId, width, height, options);
101+
} catch (NumberFormatException e) {
102+
if (Log.isLoggable(TAG, Log.WARN)) {
103+
Log.w(TAG, "Failed to parse resource id from: " + uri ,e);
104+
}
105+
}
106+
return null;
107+
}
108+
109+
@Override
110+
public boolean handles(@NonNull Uri uri) {
111+
return ContentResolver.SCHEME_ANDROID_RESOURCE.equals(uri.getScheme())
112+
&& context.getPackageName().equals(uri.getAuthority());
113+
}
114+
115+
private static final class InputStreamFactory implements ModelLoaderFactory<Uri, InputStream> {
116+
117+
private final Context context;
118+
119+
InputStreamFactory(Context context) {
120+
this.context = context;
121+
}
122+
123+
@NonNull
124+
@Override
125+
public ModelLoader<Uri, InputStream> build(@NonNull MultiModelLoaderFactory multiFactory) {
126+
return new ResourceUriLoader<>(context, multiFactory.build(Integer.class, InputStream.class));
127+
}
128+
129+
@Override public void teardown() {}
130+
}
131+
132+
private static final class AssetFileDescriptorFactory implements ModelLoaderFactory<Uri, AssetFileDescriptor> {
133+
134+
private final Context context;
135+
136+
AssetFileDescriptorFactory(Context context) {
137+
this.context = context;
138+
}
139+
140+
@NonNull
141+
@Override
142+
public ModelLoader<Uri, AssetFileDescriptor> build(
143+
@NonNull MultiModelLoaderFactory multiFactory) {
144+
return new ResourceUriLoader<>(
145+
context, multiFactory.build(Integer.class, AssetFileDescriptor.class));
146+
}
147+
148+
@Override public void teardown() {}
149+
}
150+
}

‎library/src/main/java/com/bumptech/glide/load/model/UriLoader.java

+72-9
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
import android.net.Uri;
66
import android.os.ParcelFileDescriptor;
77
import androidx.annotation.NonNull;
8+
import androidx.annotation.Nullable;
9+
import com.bumptech.glide.GlideBuilder.UseDirectResourceLoader;
10+
import com.bumptech.glide.GlideExperiments;
811
import com.bumptech.glide.load.Options;
912
import com.bumptech.glide.load.data.AssetFileDescriptorLocalUriFetcher;
1013
import com.bumptech.glide.load.data.DataFetcher;
@@ -26,20 +29,36 @@
2629
* @param <Data> The type of data that will be retrieved for {@link android.net.Uri}s.
2730
*/
2831
public class UriLoader<Data> implements ModelLoader<Uri, Data> {
29-
private static final Set<String> SCHEMES =
32+
33+
private static final Set<String> SCHEMES_WITH_RESOURCE =
3034
Collections.unmodifiableSet(
3135
new HashSet<>(
3236
Arrays.asList(
3337
ContentResolver.SCHEME_FILE,
3438
ContentResolver.SCHEME_ANDROID_RESOURCE,
3539
ContentResolver.SCHEME_CONTENT)));
3640

41+
42+
private static final Set<String> SCHEMES =
43+
Collections.unmodifiableSet(
44+
new HashSet<>(
45+
Arrays.asList(
46+
ContentResolver.SCHEME_FILE,
47+
ContentResolver.SCHEME_CONTENT)));
48+
3749
private final LocalUriFetcherFactory<Data> factory;
50+
@Nullable private final GlideExperiments glideExperiments;
51+
52+
UriLoader(
53+
LocalUriFetcherFactory<Data> factory, @Nullable GlideExperiments glideExperiments) {
54+
this.factory = factory;
55+
this.glideExperiments = glideExperiments;
56+
}
3857

3958
// Public API.
4059
@SuppressWarnings("WeakerAccess")
4160
public UriLoader(LocalUriFetcherFactory<Data> factory) {
42-
this.factory = factory;
61+
this(factory, /* glideExperiments= */ null);
4362
}
4463

4564
@Override
@@ -50,7 +69,13 @@ public LoadData<Data> buildLoadData(
5069

5170
@Override
5271
public boolean handles(@NonNull Uri model) {
53-
return SCHEMES.contains(model.getScheme());
72+
Set<String> schemesToCheck =
73+
isUseDirectResourceLoaderEnabled() ? SCHEMES : SCHEMES_WITH_RESOURCE;
74+
return schemesToCheck.contains(model.getScheme());
75+
}
76+
77+
private boolean isUseDirectResourceLoaderEnabled() {
78+
return glideExperiments != null && glideExperiments.isEnabled(UseDirectResourceLoader.class);
5479
}
5580

5681
/**
@@ -67,9 +92,22 @@ public static class StreamFactory
6792
implements ModelLoaderFactory<Uri, InputStream>, LocalUriFetcherFactory<InputStream> {
6893

6994
private final ContentResolver contentResolver;
95+
@Nullable
96+
private final GlideExperiments glideExperiments;
97+
98+
/**
99+
* @deprecated This method is experimental and will be removed in a future version without
100+
* warning.
101+
*/
102+
@Deprecated
103+
public StreamFactory(
104+
ContentResolver contentResolver, @Nullable GlideExperiments glideExperiments) {
105+
this.contentResolver = contentResolver;
106+
this.glideExperiments = glideExperiments;
107+
}
70108

71109
public StreamFactory(ContentResolver contentResolver) {
72-
this.contentResolver = contentResolver;
110+
this(contentResolver, /* glideExperiments= */ null);
73111
}
74112

75113
@Override
@@ -80,7 +118,7 @@ public DataFetcher<InputStream> build(Uri uri) {
80118
@NonNull
81119
@Override
82120
public ModelLoader<Uri, InputStream> build(MultiModelLoaderFactory multiFactory) {
83-
return new UriLoader<>(this);
121+
return new UriLoader<>(this, glideExperiments);
84122
}
85123

86124
@Override
@@ -95,9 +133,21 @@ public static class FileDescriptorFactory
95133
LocalUriFetcherFactory<ParcelFileDescriptor> {
96134

97135
private final ContentResolver contentResolver;
136+
@Nullable private final GlideExperiments glideExperiments;
137+
138+
/**
139+
* @deprecated This method is experimental and will be removed in a future version without
140+
* warning.
141+
*/
142+
@Deprecated
143+
public FileDescriptorFactory(
144+
ContentResolver contentResolver, @Nullable GlideExperiments glideExperiments) {
145+
this.contentResolver = contentResolver;
146+
this.glideExperiments = glideExperiments;
147+
}
98148

99149
public FileDescriptorFactory(ContentResolver contentResolver) {
100-
this.contentResolver = contentResolver;
150+
this(contentResolver, /* glideExperiments= */ null);
101151
}
102152

103153
@Override
@@ -108,7 +158,7 @@ public DataFetcher<ParcelFileDescriptor> build(Uri uri) {
108158
@NonNull
109159
@Override
110160
public ModelLoader<Uri, ParcelFileDescriptor> build(MultiModelLoaderFactory multiFactory) {
111-
return new UriLoader<>(this);
161+
return new UriLoader<>(this, glideExperiments);
112162
}
113163

114164
@Override
@@ -123,14 +173,27 @@ public static final class AssetFileDescriptorFactory
123173
LocalUriFetcherFactory<AssetFileDescriptor> {
124174

125175
private final ContentResolver contentResolver;
176+
@Nullable
177+
private final GlideExperiments glideExperiments;
178+
179+
/**
180+
* @deprecated This method is experimental and will be removed in a future version without
181+
* warning.
182+
*/
183+
@Deprecated
184+
public AssetFileDescriptorFactory(
185+
ContentResolver contentResolver, @Nullable GlideExperiments glideExperiments) {
186+
this.contentResolver = contentResolver;
187+
this.glideExperiments = glideExperiments;
188+
}
126189

127190
public AssetFileDescriptorFactory(ContentResolver contentResolver) {
128-
this.contentResolver = contentResolver;
191+
this(contentResolver, /* glideExperiments= */ null);
129192
}
130193

131194
@Override
132195
public ModelLoader<Uri, AssetFileDescriptor> build(MultiModelLoaderFactory multiFactory) {
133-
return new UriLoader<>(this);
196+
return new UriLoader<>(this, glideExperiments);
134197
}
135198

136199
@Override

0 commit comments

Comments
 (0)
Please sign in to comment.