Skip to content

Commit c3479c4

Browse files
sjuddsjudd
committed
Make ListPreloader more tolerant of null values from its interfaces.
Progress towards bumptech#2379.
1 parent 4239a45 commit c3479c4

File tree

6 files changed

+80
-19
lines changed

6 files changed

+80
-19
lines changed

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

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.bumptech.glide;
22

3+
import android.support.annotation.NonNull;
34
import android.support.annotation.Nullable;
45
import android.widget.AbsListView;
56
import com.bumptech.glide.request.target.BaseTarget;
@@ -24,7 +25,6 @@
2425
* @param <T> The type of the model being displayed in the list.
2526
*/
2627
public class ListPreloader<T> implements AbsListView.OnScrollListener {
27-
2828
private final int maxPreload;
2929
private final PreloadTargetQueue preloadTargetQueue;
3030
private final RequestManager requestManager;
@@ -47,24 +47,44 @@ public class ListPreloader<T> implements AbsListView.OnScrollListener {
4747
public interface PreloadModelProvider<U> {
4848

4949
/**
50-
* Returns a non null list of all models that need to be loaded for the list to display adapter
51-
* items in positions between {@code start} and {@code end}.
50+
* Returns a {@link List} of models that need to be loaded for the list to display adapter items
51+
* in positions between {@code start} and {@code end}.
52+
*
53+
* <p>A list of any size can be returned so there can be multiple models per adapter position.
54+
*
55+
* <p>Every model returned by this method is expected to produce a valid {@link RequestBuilder}
56+
* in {@link #getPreloadRequestBuilder(Object)}. If that's not possible for any set of models,
57+
* avoid including them in the {@link List} returned by this method.
5258
*
53-
* <p> A list of any size can be returned so there can be multiple models per adapter position.
54-
* </p>
59+
* <p>Although it's acceptable for the returned {@link List} to contain {@code null} models,
60+
* it's best to filter them from the list instead of adding {@code null} to avoid unnecessary
61+
* logic and expanding the size of the {@link List}
5562
*
5663
* @param position The adapter position.
5764
*/
65+
@NonNull
5866
List<U> getPreloadItems(int position);
5967

6068
/**
61-
* Returns a non null {@link RequestBuilder} for a given item. Must exactly match the request
62-
* used to load the resource in the list.
69+
* Returns a {@link RequestBuilder} for a given item on which
70+
* {@link RequestBuilder#load(Object)}} has been called or {@code null} if no valid load can be
71+
* started.
72+
*
73+
* <p>For the preloader to be effective, the {@link RequestBuilder} returned here must use
74+
* exactly the same size and set of options as the {@link RequestBuilder} used when the ``View``
75+
* is bound. You may need to specify a size in both places to ensure that the width and height
76+
* match exactly. If so, you can use
77+
* {@link com.bumptech.glide.request.RequestOptions#override(int, int)} to do so.
78+
*
79+
* <p>The target and context will be provided by the preloader.
6380
*
64-
* <p> The target and context will be provided by the preloader. </p>
81+
* <p>If {@link RequestBuilder#load(Object)} is not called by this method, the preloader will
82+
* trigger a {@link RuntimeException}. If you don't want to load a particular item or position,
83+
* filter it from the list returned by {@link #getPreloadItems(int)}.
6584
*
6685
* @param item The model to load.
6786
*/
87+
@Nullable
6888
RequestBuilder getPreloadRequestBuilder(U item);
6989
}
7090

@@ -80,8 +100,9 @@ public interface PreloadSizeProvider<T> {
80100
* Returns the size of the view in the list where the resources will be displayed in pixels in
81101
* the format [x, y], or {@code null} if no size is currently available.
82102
*
83-
* <p> Note - The dimensions returned here must precisely match those of the view in the list.
84-
* </p>
103+
* <p>Note - The dimensions returned here must precisely match those of the view in the list.
104+
*
105+
* <p>If this method returns {@code null}, then no request will be started for the given item.
85106
*
86107
* @param item A model
87108
*/
@@ -175,13 +196,22 @@ private void preloadAdapterPosition(List<T> items, int position, boolean isIncre
175196
}
176197

177198
@SuppressWarnings("unchecked")
178-
private void preloadItem(T item, int position, int i) {
179-
final int[] dimensions = this.preloadDimensionProvider.getPreloadSize(item, position, i);
180-
if (dimensions != null) {
181-
RequestBuilder<Object> preloadRequestBuilder =
182-
this.preloadModelProvider.getPreloadRequestBuilder(item);
183-
preloadRequestBuilder.into(preloadTargetQueue.next(dimensions[0], dimensions[1]));
199+
private void preloadItem(@Nullable T item, int position, int perItemPosition) {
200+
if (item == null) {
201+
return;
202+
}
203+
int[] dimensions =
204+
preloadDimensionProvider.getPreloadSize(item, position, perItemPosition);
205+
if (dimensions == null) {
206+
return;
184207
}
208+
RequestBuilder<Object> preloadRequestBuilder =
209+
preloadModelProvider.getPreloadRequestBuilder(item);
210+
if (preloadRequestBuilder == null) {
211+
return;
212+
}
213+
214+
preloadRequestBuilder.into(preloadTargetQueue.next(dimensions[0], dimensions[1]));
185215
}
186216

187217
private void cancelAll() {
@@ -193,7 +223,7 @@ private void cancelAll() {
193223
private static final class PreloadTargetQueue {
194224
private final Queue<PreloadTarget> queue;
195225

196-
public PreloadTargetQueue(int size) {
226+
PreloadTargetQueue(int size) {
197227
queue = Util.createQueue(size);
198228

199229
for (int i = 0; i < size; i++) {
@@ -210,7 +240,7 @@ public PreloadTarget next(int width, int height) {
210240
}
211241
}
212242

213-
private static class PreloadTarget extends BaseTarget<Object> {
243+
private static final class PreloadTarget extends BaseTarget<Object> {
214244
@Synthetic int photoHeight;
215245
@Synthetic int photoWidth;
216246

‎library/src/test/java/com/bumptech/glide/ListPreloaderTest.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import static org.mockito.Mockito.times;
99
import static org.mockito.Mockito.verify;
1010

11+
import android.support.annotation.NonNull;
1112
import com.bumptech.glide.request.target.SizeReadyCallback;
1213
import com.bumptech.glide.request.target.Target;
1314
import java.util.ArrayList;
@@ -45,6 +46,7 @@ public void testGetItemsIsCalledIncreasing() {
4546
final AtomicInteger calledCount = new AtomicInteger();
4647

4748
ListPreloaderAdapter preloaderAdapter = new ListPreloaderAdapter() {
49+
@NonNull
4850
@Override
4951
public List<Object> getPreloadItems(int position) {
5052
called.set(true);
@@ -75,11 +77,13 @@ public int[] getPreloadSize(Object item, int adapterPosition, int itemPosition)
7577
return new int[] { 10, 10 };
7678
}
7779

80+
@NonNull
7881
@Override
7982
public List<Object> getPreloadItems(int position) {
8083
return objects.subList(position - 11, position + 1 - 11);
8184
}
8285

86+
@NonNull
8387
@Override
8488
@SuppressWarnings("unchecked")
8589
public RequestBuilder<Object> getPreloadRequestBuilder(Object item) {
@@ -98,6 +102,7 @@ public void testGetItemsIsCalledDecreasing() {
98102
final AtomicBoolean called = new AtomicBoolean(false);
99103
final AtomicInteger calledCount = new AtomicInteger();
100104
ListPreloaderAdapter preloaderAdapter = new ListPreloaderAdapter() {
105+
@NonNull
101106
@Override
102107
public List<Object> getPreloadItems(int position) {
103108
// Ignore the preload caused from us starting at the end
@@ -133,6 +138,7 @@ public int[] getPreloadSize(Object item, int adapterPosition, int itemPosition)
133138
return new int[] { 10, 10 };
134139
}
135140

141+
@NonNull
136142
@Override
137143
public List<Object> getPreloadItems(int position) {
138144
if (position == 40) {
@@ -141,6 +147,7 @@ public List<Object> getPreloadItems(int position) {
141147
return objects.subList(position, position + 1);
142148
}
143149

150+
@NonNull
144151
@Override
145152
@SuppressWarnings("unchecked")
146153
public RequestBuilder<Object> getPreloadRequestBuilder(Object item) {
@@ -160,6 +167,7 @@ public void testGetItemsIsNeverCalledWithEndGreaterThanTotalItems() {
160167
final AtomicBoolean called = new AtomicBoolean(false);
161168
final AtomicInteger calledCount = new AtomicInteger();
162169
ListPreloaderAdapter preloaderAdapter = new ListPreloaderAdapter() {
170+
@NonNull
163171
@Override
164172
public List<Object> getPreloadItems(int position) {
165173
called.set(true);
@@ -179,6 +187,7 @@ public void testGetItemsIsNeverCalledWithStartLessThanZero() {
179187
final AtomicBoolean called = new AtomicBoolean(false);
180188
final AtomicInteger calledCount = new AtomicInteger();
181189
ListPreloaderAdapter preloaderAdapter = new ListPreloaderAdapter() {
190+
@NonNull
182191
@Override
183192
public List<Object> getPreloadItems(int position) {
184193
if (position >= 17) {
@@ -202,6 +211,7 @@ public List<Object> getPreloadItems(int position) {
202211
public void testDontPreloadItemsRepeatedlyWhileIncreasing() {
203212
final AtomicInteger called = new AtomicInteger();
204213
ListPreloaderAdapter preloaderAdapter = new ListPreloaderAdapter() {
214+
@NonNull
205215
@Override
206216
public List<Object> getPreloadItems(int position) {
207217
final int current = called.getAndIncrement();
@@ -222,6 +232,7 @@ public List<Object> getPreloadItems(int position) {
222232
public void testDontPreloadItemsRepeatedlyWhileDecreasing() {
223233
final AtomicInteger called = new AtomicInteger();
224234
ListPreloaderAdapter preloaderAdapter = new ListPreloaderAdapter() {
235+
@NonNull
225236
@Override
226237
public List<Object> getPreloadItems(int position) {
227238
if (position >= 20) {
@@ -249,6 +260,7 @@ public void testMultipleItemsForPositionIncreasing() throws NoSuchFieldException
249260
ListPreloaderAdapter preloaderAdapter = new ListPreloaderAdapter() {
250261
private int expectedPosition = (1 + 10) * 2;
251262

263+
@NonNull
252264
@Override
253265
public List<Object> getPreloadItems(int position) {
254266
return objects;
@@ -262,6 +274,7 @@ public int[] getPreloadSize(Object item, int adapterPosition, int itemPosition)
262274
return itemPosition == 0 ? new int[] { 10, 11 } : new int[] { 20, 21 };
263275
}
264276

277+
@NonNull
265278
@Override
266279
public RequestBuilder<Object> getPreloadRequestBuilder(Object item) {
267280
return request;
@@ -285,6 +298,7 @@ public void testMultipleItemsForPositionDecreasing() throws NoSuchFieldException
285298
ListPreloaderAdapter preloaderAdapter = new ListPreloaderAdapter() {
286299
private int expectedPosition = objects.size() * 2 - 1;
287300

301+
@NonNull
288302
@Override
289303
public List<Object> getPreloadItems(int position) {
290304
return objects;
@@ -298,6 +312,7 @@ public int[] getPreloadSize(Object item, int adapterPosition, int itemPosition)
298312
return itemPosition == 0 ? new int[] { 10, 11 } : new int[] { 20, 21 };
299313
}
300314

315+
@NonNull
301316
@Override
302317
public RequestBuilder<Object> getPreloadRequestBuilder(Object item) {
303318
return request;
@@ -334,11 +349,13 @@ public void testItemsArePreloadedWithGlide() {
334349
objects.add(new Object());
335350
final HashSet<Object> loadedObjects = new HashSet<>();
336351
ListPreloaderAdapter preloaderAdapter = new ListPreloaderAdapter() {
352+
@NonNull
337353
@Override
338354
public List<Object> getPreloadItems(int position) {
339355
return objects.subList(position - 11, position - 10);
340356
}
341357

358+
@NonNull
342359
@Override
343360
public RequestBuilder<Object> getPreloadRequestBuilder(Object item) {
344361
loadedObjects.add(item);
@@ -358,13 +375,15 @@ private static class ListPreloaderAdapter implements ListPreloader.PreloadModelP
358375
public ListPreloaderAdapter() {
359376
}
360377

378+
@NonNull
361379
@Override
362380
public List<Object> getPreloadItems(int position) {
363381
ArrayList<Object> result = new ArrayList<>(1);
364382
Collections.fill(result, new Object());
365383
return result;
366384
}
367385

386+
@NonNull
368387
@Override
369388
@SuppressWarnings("unchecked")
370389
public RequestBuilder<Object> getPreloadRequestBuilder(Object item) {

‎samples/flickr/src/main/java/com/bumptech/glide/samples/flickr/FlickrPhotoGrid.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import android.graphics.Rect;
55
import android.graphics.drawable.Drawable;
66
import android.os.Bundle;
7+
import android.support.annotation.NonNull;
78
import android.support.v4.app.Fragment;
89
import android.support.v7.widget.GridLayoutManager;
910
import android.support.v7.widget.RecyclerView;
@@ -191,14 +192,16 @@ public int getItemCount() {
191192
return photos.size();
192193
}
193194

195+
@NonNull
194196
@Override
195197
public List<Photo> getPreloadItems(int position) {
196198
return photos.subList(position, position + 1);
197199
}
198200

201+
@NonNull
199202
@Override
200203
public RequestBuilder<Drawable> getPreloadRequestBuilder(Photo item) {
201-
return preloadRequest.load(item);
204+
return fullRequest.clone().thumbnail(thumbnailRequest.clone().load(item)).load(item);
202205
}
203206
}
204207

‎samples/flickr/src/main/java/com/bumptech/glide/samples/flickr/FlickrPhotoList.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import android.graphics.drawable.ColorDrawable;
88
import android.graphics.drawable.Drawable;
99
import android.os.Bundle;
10+
import android.support.annotation.NonNull;
1011
import android.support.v4.app.Fragment;
1112
import android.support.v7.widget.LinearLayoutManager;
1213
import android.support.v7.widget.RecyclerView;
@@ -165,11 +166,13 @@ public int getItemCount() {
165166
return photos.size();
166167
}
167168

169+
@NonNull
168170
@Override
169171
public List<Photo> getPreloadItems(int position) {
170172
return photos.subList(position, position + 1);
171173
}
172174

175+
@NonNull
173176
@Override
174177
public RequestBuilder<Drawable> getPreloadRequestBuilder(Photo item) {
175178
return fullRequest.thumbnail(thumbRequest.load(item)).load(item);

‎samples/gallery/src/main/java/com/bumptech/glide/samples/gallery/RecyclerAdapter.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import android.content.Context;
44
import android.graphics.Point;
55
import android.graphics.drawable.Drawable;
6+
import android.support.annotation.NonNull;
67
import android.support.v7.widget.RecyclerView;
78
import android.view.Display;
89
import android.view.LayoutInflater;
@@ -91,11 +92,13 @@ public int getItemViewType(int position) {
9192
return 0;
9293
}
9394

95+
@NonNull
9496
@Override
9597
public List<MediaStoreData> getPreloadItems(int position) {
9698
return Collections.singletonList(data.get(position));
9799
}
98100

101+
@NonNull
99102
@Override
100103
public RequestBuilder<Drawable> getPreloadRequestBuilder(MediaStoreData item) {
101104
MediaStoreSignature signature =

‎samples/giphy/src/main/java/com/bumptech/glide/samples/giphy/MainActivity.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import android.content.Intent;
88
import android.graphics.drawable.Drawable;
99
import android.os.Bundle;
10+
import android.support.annotation.NonNull;
1011
import android.support.v7.widget.LinearLayoutManager;
1112
import android.support.v7.widget.RecyclerView;
1213
import android.view.View;
@@ -137,11 +138,13 @@ public int getItemCount() {
137138
return results.length;
138139
}
139140

141+
@NonNull
140142
@Override
141143
public List<Api.GifResult> getPreloadItems(int position) {
142144
return Collections.singletonList(results[position]);
143145
}
144146

147+
@NonNull
145148
@Override
146149
public RequestBuilder<Drawable> getPreloadRequestBuilder(Api.GifResult item) {
147150
return requestBuilder.load(item);

0 commit comments

Comments
 (0)