Skip to content

Commit d4c677a

Browse files
johnlcoxjohnlcox
authored
Teach B2ListFilesIterableBase to handle empty pages when nextFileNam is not null (#180)
1 parent c2a2332 commit d4c677a

File tree

4 files changed

+278
-14
lines changed

4 files changed

+278
-14
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
* Return a subtype of `B2Exception` on errors when the response body does not conform to `B2ErrorStructure`.
66
Returning a `B2Exception` subtype enables the `B2Retryer` to retry exceptions that may succeed on retry.
77

8+
### Fixed
9+
* Fixed B2ListFilesIterableBase assuming a response with 0 results was the end. It now looks for
10+
`nextFileName` being null to indicate the end.
11+
812
## [6.1.0] - 2022-09-19
913
### Added
1014
* Added support for Java 8's `-parameters` option so constructor parameters do not need to be reiterated in `B2Json.constructor#params`

‎core/src/main/java/com/backblaze/b2/client/B2ListFilesIterableBase.java‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,20 @@ protected abstract class IterBase implements Iterator<B2FileVersion> {
2424

2525
@Override
2626
public boolean hasNext() {
27-
return (currentIndex < getCurrentResponseSize());
27+
return hasNextOnCurrentPage() || hasMorePages();
28+
}
29+
30+
private boolean hasNextOnCurrentPage() {
31+
return currentIndex < getCurrentResponseSize();
32+
}
33+
34+
/**
35+
* Returns true if we haven't gotten the first response page yet, or the response
36+
* says we're not at the end.
37+
*/
38+
private boolean hasMorePages() {
39+
final B2ListFilesResponse response = getCurrentResponseOrNull();
40+
return response == null || !response.atEnd();
2841
}
2942

3043
@Override
@@ -45,21 +58,20 @@ public B2FileVersion next() {
4558
}
4659

4760
private void advanceIfNeeded() throws B2Exception {
48-
if (hasNext()) {
49-
// no need to advance.
50-
return;
51-
}
52-
53-
if (getCurrentResponseOrNull() != null) {
54-
// we've gotten at least one page of results. was it the last page?
55-
if (getCurrentResponseOrNull().atEnd()) {
56-
// no more pages to fetch.
57-
return;
61+
// Advance until we find a non-empty page, or the response says we're at the end
62+
while (!hasNextOnCurrentPage()) {
63+
64+
if (getCurrentResponseOrNull() != null) {
65+
// we've gotten at least one page of results. was it the last page?
66+
if (getCurrentResponseOrNull().atEnd()) {
67+
// no more pages to fetch.
68+
return;
69+
}
5870
}
59-
}
6071

61-
advance();
62-
currentIndex = 0;
72+
advance();
73+
currentIndex = 0;
74+
}
6375
}
6476

6577
// may be called when there's no current response yet.

‎core/src/test/java/com/backblaze/b2/client/B2ListFileNamesIterableTest.java‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
import static com.backblaze.b2.client.B2TestHelpers.fileName;
2424
import static com.backblaze.b2.client.B2TestHelpers.makeVersion;
2525
import static org.junit.Assert.assertEquals;
26+
import static org.junit.Assert.assertFalse;
2627
import static org.junit.Assert.assertNull;
28+
import static org.junit.Assert.assertSame;
2729
import static org.junit.Assert.assertTrue;
2830
import static org.mockito.Matchers.anyObject;
2931
import static org.mockito.Mockito.mock;
@@ -69,6 +71,49 @@ public void testOnePage() throws B2Exception {
6971
assertTrue(!iter.hasNext());
7072
}
7173

74+
@Test
75+
public void testEmptyPageWithNextFileName() throws B2Exception {
76+
final B2ListFileNamesRequest request = B2ListFileNamesRequest
77+
.builder(BUCKET_ID)
78+
.setStartFileName("file/0000")
79+
.setMaxFileCount(106)
80+
.setPrefix("file/")
81+
.setDelimiter("/")
82+
.build();
83+
84+
// First page is empty, but has a nextFileName
85+
final B2ListFileNamesResponse emptyResponse = new B2ListFileNamesResponse(B2Collections.listOf(), fileName(3000000));
86+
when(client.listFileNames(request)).thenReturn(emptyResponse);
87+
88+
// Second page has actual results
89+
final B2ListFileNamesRequest pageTwoRequest = B2ListFileNamesRequest
90+
.builder(BUCKET_ID)
91+
.setStartFileName(fileName(3000000))
92+
.setMaxFileCount(106)
93+
.setPrefix("file/")
94+
.setDelimiter("/")
95+
.build();
96+
final List<B2FileVersion> pageTwoNames = B2Collections.listOf(
97+
makeVersion(3,3000014),
98+
makeVersion(4, 3000015),
99+
makeVersion(5, 3000015));
100+
final B2ListFileNamesResponse pageTwoResponse = new B2ListFileNamesResponse(pageTwoNames, null);
101+
when(client.listFileNames(pageTwoRequest)).thenReturn(pageTwoResponse);
102+
103+
final Iterator<B2FileVersion> iter = new B2ListFileNamesIterable(client, request).iterator();
104+
105+
// first page.
106+
assertTrue(iter.hasNext());
107+
108+
// second page
109+
assertSame(pageTwoNames.get(0), iter.next());
110+
assertTrue(iter.hasNext());
111+
assertSame(pageTwoNames.get(1), iter.next());
112+
assertTrue(iter.hasNext());
113+
assertSame(pageTwoNames.get(2), iter.next());
114+
assertFalse(iter.hasNext());
115+
}
116+
72117
@Test
73118
public void testMultiplePages() throws B2Exception {
74119
// using some arguments here to make sure they're used in first request and that most are propagated.
@@ -130,6 +175,81 @@ public void testMultiplePages() throws B2Exception {
130175
assertTrue(!iter.hasNext());
131176
}
132177

178+
@Test
179+
public void testMultiplePagesWithEmptyPageInMiddle() throws B2Exception {
180+
// using some arguments here to make sure they're used in first request and that most are propagated.
181+
final B2ListFileNamesRequest request = B2ListFileNamesRequest
182+
.builder(BUCKET_ID)
183+
.setStartFileName("file/0000")
184+
.setMaxFileCount(106)
185+
.setPrefix("file/")
186+
.setDelimiter("/")
187+
.build();
188+
189+
// when asked, return one answer with a few names and some nexts.
190+
final List<B2FileVersion> pageOneNames = B2Collections.listOf(makeVersion(1,1), makeVersion(2, 1));
191+
final B2ListFileNamesResponse pageOneResponse = new B2ListFileNamesResponse(pageOneNames, fileName(3));
192+
when(client.listFileNames(request)).thenReturn(pageOneResponse);
193+
194+
// Second page is empty, but has a nextFileName (Maybe millions were checked, but
195+
// didn't match; an empty page is returned to prevent the client from timing out)
196+
final B2ListFileNamesRequest pageTwoRequest = B2ListFileNamesRequest
197+
.builder(BUCKET_ID)
198+
.setStartFileName(fileName(3))
199+
.setMaxFileCount(106)
200+
.setPrefix("file/")
201+
.setDelimiter("/")
202+
.build();
203+
final B2ListFileNamesResponse emptyResponse = new B2ListFileNamesResponse(B2Collections.listOf(), fileName(3000000));
204+
when(client.listFileNames(pageTwoRequest)).thenReturn(emptyResponse);
205+
206+
// Third page has actual results and a nextFileName
207+
final B2ListFileNamesRequest pageThreeRequest = B2ListFileNamesRequest
208+
.builder(BUCKET_ID)
209+
.setStartFileName(fileName(3000000))
210+
.setMaxFileCount(106)
211+
.setPrefix("file/")
212+
.setDelimiter("/")
213+
.build();
214+
final List<B2FileVersion> pageThreeNames = B2Collections.listOf(
215+
makeVersion(3,30000014),
216+
makeVersion(4, 30000015),
217+
makeVersion(5, 30000015));
218+
final B2ListFileNamesResponse pageThreeResponse = new B2ListFileNamesResponse(pageThreeNames, fileName(30000016));
219+
when(client.listFileNames(pageThreeRequest)).thenReturn(pageThreeResponse);
220+
221+
// note that we expected to have more cuz pageThreeResponse had 'next's, but it turned out we didn't.
222+
final B2ListFileNamesRequest pageFourRequest = B2ListFileNamesRequest
223+
.builder(BUCKET_ID)
224+
.setStartFileName(fileName(30000016))
225+
.setMaxFileCount(106)
226+
.setPrefix("file/")
227+
.setDelimiter("/")
228+
.build();
229+
final B2ListFileNamesResponse pageFourResponse = new B2ListFileNamesResponse(B2Collections.listOf(), null);
230+
when(client.listFileNames(pageFourRequest)).thenReturn(pageFourResponse);
231+
232+
// iter should have two pageOneNames.
233+
final Iterator<B2FileVersion> iter = new B2ListFileNamesIterable(client, request).iterator();
234+
235+
// first page.
236+
assertTrue(iter.hasNext());
237+
assertSame(pageOneNames.get(0), iter.next());
238+
assertTrue(iter.hasNext());
239+
assertSame(pageOneNames.get(1), iter.next());
240+
assertTrue(iter.hasNext());
241+
242+
// third page
243+
//noinspection ConstantConditions
244+
assertTrue(iter.hasNext());
245+
assertSame(pageThreeNames.get(0), iter.next());
246+
assertTrue(iter.hasNext());
247+
assertSame(pageThreeNames.get(1), iter.next());
248+
assertTrue(iter.hasNext());
249+
assertSame(pageThreeNames.get(2), iter.next());
250+
assertFalse(iter.hasNext());
251+
}
252+
133253
@Test
134254
public void testBuilder() {
135255
B2ListFileNamesRequest request = B2ListFileNamesRequest

‎core/src/test/java/com/backblaze/b2/client/B2ListFileVersionsIterableTest.java‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static com.backblaze.b2.client.B2TestHelpers.fileName;
2121
import static com.backblaze.b2.client.B2TestHelpers.makeVersion;
2222
import static org.junit.Assert.assertEquals;
23+
import static org.junit.Assert.assertFalse;
2324
import static org.junit.Assert.assertNull;
2425
import static org.junit.Assert.assertSame;
2526
import static org.junit.Assert.assertTrue;
@@ -67,6 +68,49 @@ public void testOnePage() throws B2Exception {
6768
assertTrue(!iter.hasNext());
6869
}
6970

71+
@Test
72+
public void testEmptyPageWithNextFileNameAndVersion() throws B2Exception {
73+
final B2ListFileVersionsRequest request = B2ListFileVersionsRequest
74+
.builder(BUCKET_ID)
75+
.setStart("file/0000", fileId(0))
76+
.setMaxFileCount(106)
77+
.setPrefix("file/")
78+
.setDelimiter("/")
79+
.build();
80+
81+
// First page is empty, but has a nextFileName
82+
final B2ListFileVersionsResponse pageOneResponse = new B2ListFileVersionsResponse(B2Collections.listOf(), "file/0000", fileId(3000013));
83+
when(client.listFileVersions(request)).thenReturn(pageOneResponse);
84+
85+
// Second page has actual results
86+
final B2ListFileVersionsRequest pageTwoRequest = B2ListFileVersionsRequest
87+
.builder(BUCKET_ID)
88+
.setStart("file/0000", fileId(3000013))
89+
.setMaxFileCount(106)
90+
.setPrefix("file/")
91+
.setDelimiter("/")
92+
.build();
93+
final List<B2FileVersion> pageTwoVersions = B2Collections.listOf(
94+
makeVersion(3000000,3000014),
95+
makeVersion(4000000, 3000015),
96+
makeVersion(5000000, 3000015));
97+
final B2ListFileVersionsResponse pageTwoResponse = new B2ListFileVersionsResponse(pageTwoVersions, null, null);
98+
when(client.listFileVersions(pageTwoRequest)).thenReturn(pageTwoResponse);
99+
100+
final Iterator<B2FileVersion> iter = new B2ListFileVersionsIterable(client, request).iterator();
101+
102+
// first page.
103+
assertTrue(iter.hasNext());
104+
105+
// second page
106+
assertSame(pageTwoVersions.get(0), iter.next());
107+
assertTrue(iter.hasNext());
108+
assertSame(pageTwoVersions.get(1), iter.next());
109+
assertTrue(iter.hasNext());
110+
assertSame(pageTwoVersions.get(2), iter.next());
111+
assertFalse(iter.hasNext());
112+
}
113+
70114
@Test
71115
public void testMultiplePages() throws B2Exception {
72116
// using some arguments here to make sure they're used in first request and that most are propagated.
@@ -130,6 +174,81 @@ public void testMultiplePages() throws B2Exception {
130174
assertTrue(!iter.hasNext());
131175
}
132176

177+
@Test
178+
public void testMultiplePagesWithEmptyPageInMiddle() throws B2Exception {
179+
// using some arguments here to make sure they're used in first request and that most are propagated.
180+
final B2ListFileVersionsRequest request = B2ListFileVersionsRequest
181+
.builder(BUCKET_ID)
182+
.setStart("file/0000", fileId(0))
183+
.setMaxFileCount(106)
184+
.setPrefix("file/")
185+
.setDelimiter("/")
186+
.build();
187+
188+
// when asked, return one answer with a few versions and some nexts.
189+
final List<B2FileVersion> pageOneVersions = B2Collections.listOf(makeVersion(1, 1), makeVersion(2, 1));
190+
final B2ListFileVersionsResponse pageOneResponse = new B2ListFileVersionsResponse(pageOneVersions, fileName(3), fileId(13));
191+
when(client.listFileVersions(request)).thenReturn(pageOneResponse);
192+
193+
// Second page is empty, but has a nextFileName (Maybe millions were checked, but
194+
// didn't match; an empty page is returned to prevent the client from timing out)
195+
final B2ListFileVersionsRequest pageTwoRequest = B2ListFileVersionsRequest
196+
.builder(BUCKET_ID)
197+
.setStart(fileName(3), fileId(13))
198+
.setMaxFileCount(106)
199+
.setPrefix("file/")
200+
.setDelimiter("/")
201+
.build();
202+
final B2ListFileVersionsResponse pageTwoResponse = new B2ListFileVersionsResponse(B2Collections.listOf(), fileName(3000000), fileId(5));
203+
when(client.listFileVersions(pageTwoRequest)).thenReturn(pageTwoResponse);
204+
205+
// Third page has actual results and a nextFileName
206+
final B2ListFileVersionsRequest pageThreeRequest = B2ListFileVersionsRequest
207+
.builder(BUCKET_ID)
208+
.setStart(fileName(3000000), fileId(5))
209+
.setMaxFileCount(106)
210+
.setPrefix("file/")
211+
.setDelimiter("/")
212+
.build();
213+
final List<B2FileVersion> pageThreeVersions = B2Collections.listOf(
214+
makeVersion(3,3000014),
215+
makeVersion(4, 3000015),
216+
makeVersion(5, 3000015));
217+
final B2ListFileVersionsResponse pageThreeResponse = new B2ListFileVersionsResponse(pageThreeVersions, fileName(3000016), fileId(5));
218+
when(client.listFileVersions(pageThreeRequest)).thenReturn(pageThreeResponse);
219+
220+
// note that we expected to have more cuz pageThreeResponse had 'next's, but it turned out we didn't.
221+
final B2ListFileVersionsRequest pageFourRequest = B2ListFileVersionsRequest
222+
.builder(BUCKET_ID)
223+
.setStart(fileName(3000016), fileId(5))
224+
.setMaxFileCount(106)
225+
.setPrefix("file/")
226+
.setDelimiter("/")
227+
.build();
228+
final B2ListFileVersionsResponse pageFourResponse = new B2ListFileVersionsResponse(B2Collections.listOf(), null, null);
229+
when(client.listFileVersions(pageFourRequest)).thenReturn(pageFourResponse);
230+
231+
// iter should have two pageOneVersions.
232+
final Iterator<B2FileVersion> iter = new B2ListFileVersionsIterable(client, request).iterator();
233+
234+
// first page.
235+
assertTrue(iter.hasNext());
236+
assertSame(pageOneVersions.get(0), iter.next());
237+
assertTrue(iter.hasNext());
238+
assertSame(pageOneVersions.get(1), iter.next());
239+
assertTrue(iter.hasNext());
240+
241+
// third page
242+
//noinspection ConstantConditions
243+
assertTrue(iter.hasNext());
244+
assertSame(pageThreeVersions.get(0), iter.next());
245+
assertTrue(iter.hasNext());
246+
assertSame(pageThreeVersions.get(1), iter.next());
247+
assertTrue(iter.hasNext());
248+
assertSame(pageThreeVersions.get(2), iter.next());
249+
assertFalse(iter.hasNext());
250+
}
251+
133252
@Test
134253
public void testOkForNextFileIdToBeNull() throws B2Exception {
135254
final B2ListFileVersionsRequest request = B2ListFileVersionsRequest
@@ -141,6 +260,15 @@ public void testOkForNextFileIdToBeNull() throws B2Exception {
141260
final B2ListFileVersionsResponse smallResponse = new B2ListFileVersionsResponse(versions, fileName(2), null);
142261
when(client.listFileVersions(request)).thenReturn(smallResponse);
143262

263+
// note that we expected to have more cuz pageTwoResponse had nextFileName, but it turned out we didn't.
264+
final B2ListFileVersionsRequest pageTwoRequest = B2ListFileVersionsRequest
265+
.builder(BUCKET_ID)
266+
.setStart(fileName(2), null)
267+
.build();
268+
final B2ListFileVersionsResponse pageTwoResponse = new B2ListFileVersionsResponse(B2Collections.listOf(), null, null);
269+
when(client.listFileVersions(pageTwoRequest)).thenReturn(pageTwoResponse);
270+
271+
144272
// iter should have two versions.
145273
final Iterator<B2FileVersion> iter = new B2ListFileVersionsIterable(client, request).iterator();
146274
assertTrue(iter.hasNext());

0 commit comments

Comments
 (0)