Skip to content

Commit ad600bd

Browse files
wu-huiwu-hui
committed
More feedback.
1 parent 33b1f4a commit ad600bd

File tree

3 files changed

+43
-42
lines changed

3 files changed

+43
-42
lines changed

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/BundleTest.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
import java.io.InputStream;
3333
import java.io.UnsupportedEncodingException;
3434
import java.nio.ByteBuffer;
35-
import java.nio.charset.StandardCharsets;
35+
import java.nio.charset.Charset;
3636
import java.util.ArrayList;
3737
import java.util.List;
3838
import java.util.concurrent.ExecutionException;
@@ -44,6 +44,7 @@
4444

4545
@RunWith(AndroidJUnit4.class)
4646
public class BundleTest {
47+
private static Charset UTF8_CHARSET = Charset.forName("UTF-8");
4748
private static String[] BUNDLE_TEMPLATES =
4849
new String[] {
4950
"{\"metadata\":{\"id\":\"test-bundle\",\"createTime\":{\"seconds\":1001,\"nanos\":9999},"
@@ -251,8 +252,8 @@ private void verifyQueryResults() throws ExecutionException, InterruptedExceptio
251252
assertEquals(asList(map("bar", 1L, "k", "a")), querySnapshotToValues(limitToLastSnapshot));
252253
}
253254

254-
private int getUTF8BytesCount(String s) {
255-
return s.getBytes(StandardCharsets.UTF_8).length;
255+
private int getUTF8ByteCount(String s) {
256+
return s.getBytes(UTF8_CHARSET).length;
256257
}
257258

258259
/**
@@ -267,7 +268,7 @@ private byte[] createBundle(String projectId) throws UnsupportedEncodingExceptio
267268
for (int i = 1; i < BUNDLE_TEMPLATES.length; ++i) {
268269
// Extract elements from BUNDLE_TEMPLATE and replace the project ID.
269270
String element = BUNDLE_TEMPLATES[i].replaceAll("\\{projectId\\}", projectId);
270-
bundle.append(getUTF8BytesCount(element));
271+
bundle.append(getUTF8ByteCount(element));
271272
bundle.append(element);
272273
}
273274

@@ -278,7 +279,7 @@ private byte[] createBundle(String projectId) throws UnsupportedEncodingExceptio
278279
BUNDLE_TEMPLATES[0].replace(
279280
"{totalBytes}", Integer.toString(bundleString.getBytes("UTF-8").length));
280281

281-
String fullBundle = getUTF8BytesCount(metadata) + metadata + bundleString;
282+
String fullBundle = getUTF8ByteCount(metadata) + metadata + bundleString;
282283
return fullBundle.getBytes("UTF-8");
283284
}
284285

‎firebase-firestore/src/main/java/com/google/firebase/firestore/bundle/BundleReader.java

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,10 @@ public class BundleReader {
3434
/** The capacity for the internal char buffer. */
3535
protected static final int BUFFER_CAPACITY = 1024;
3636

37+
private static final Charset UTF8_CHARSET = Charset.forName("UTF-8");
38+
3739
private final BundleSerializer serializer;
3840
private final InputStream bundleInputStream;
39-
private final Charset charset = Charset.forName("UTF-8");
4041

4142
@Nullable BundleMetadata metadata;
4243
private ByteBuffer buffer;
@@ -103,7 +104,7 @@ private BundleElement readNextElement() throws IOException, JSONException {
103104

104105
int jsonStringByteCount = Integer.parseInt(lengthPrefix);
105106
String json = readJsonString(jsonStringByteCount);
106-
bytesRead += lengthPrefix.getBytes(charset).length + jsonStringByteCount;
107+
bytesRead += lengthPrefix.getBytes(UTF8_CHARSET).length + jsonStringByteCount;
107108
return decodeBundleElement(json);
108109
}
109110

@@ -136,7 +137,7 @@ private BundleElement readNextElement() throws IOException, JSONException {
136137

137138
byte[] c = new byte[nextOpenBracket];
138139
buffer.get(c);
139-
return charset.decode(ByteBuffer.wrap(c)).toString();
140+
return UTF8_CHARSET.decode(ByteBuffer.wrap(c)).toString();
140141
}
141142

142143
/** Returns the index of the first open bracket, or -1 if none is found. */
@@ -175,13 +176,13 @@ private String readJsonString(int bytesToRead) throws IOException {
175176
// `read` is the number of bytes guaranteed to exist in `this.buffer` after the above
176177
// call to `pullMoreData`. Copy them to `jsonBytes` and advance `this.buffer`'s position.
177178
int read = Math.min(remaining, buffer.remaining());
178-
jsonBytes.write(buffer.array(), buffer.position(), read);
179+
jsonBytes.write(buffer.array(), buffer.arrayOffset() + buffer.position(), read);
179180
buffer.position(buffer.position() + read);
180181

181182
remaining -= read;
182183
}
183184

184-
return jsonBytes.toString(charset.name());
185+
return jsonBytes.toString(UTF8_CHARSET.name());
185186
}
186187

187188
/**
@@ -191,12 +192,23 @@ private String readJsonString(int bytesToRead) throws IOException {
191192
*/
192193
private boolean pullMoreData() throws IOException {
193194
buffer.compact();
194-
int bytesToRead = Math.min(bundleInputStream.available(), buffer.remaining());
195+
196+
int available = bundleInputStream.available();
197+
int bytesToRead = Math.min(available, buffer.remaining());
198+
// `available` is an estimation, we still try to read if the estimation is 0, to move things
199+
// forward.
200+
if (available == 0) {
201+
bytesToRead = buffer.remaining();
202+
}
195203
byte[] bytes = new byte[bytesToRead];
196204
int bytesRead = bundleInputStream.read(bytes);
197-
buffer.put(bytes);
205+
boolean readSuccess = bytesRead > 0;
206+
if (readSuccess) {
207+
buffer.put(bytes, 0, bytesRead);
208+
}
209+
198210
buffer.flip();
199-
return bytesRead > 0;
211+
return readSuccess;
200212
}
201213

202214
/** Converts a JSON-encoded bundle element into its model class. */

‎firebase-firestore/src/test/java/com/google/firebase/firestore/bundle/BundleReaderTest.java

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
import com.google.firestore.v1.Value;
3333
import java.io.ByteArrayInputStream;
3434
import java.io.IOException;
35-
import java.nio.charset.StandardCharsets;
35+
import java.nio.charset.Charset;
3636
import java.util.ArrayList;
3737
import java.util.Collections;
3838
import java.util.List;
@@ -121,6 +121,7 @@ public class BundleReaderTest {
121121
30004002L,
122122
ObjectValue.fromMap(
123123
map("unicodeValue", Value.newBuilder().setStringValue("\uD83D\uDE0A").build()))));
124+
private static final Charset UTF8_CHARSET = Charset.forName("UTF-8");
124125

125126
@Test
126127
public void testReadsQueryAndDocument() throws IOException, JSONException {
@@ -133,8 +134,7 @@ public void testReadsQueryAndDocument() throws IOException, JSONException {
133134
bundleBuilder.build("bundle-1", /* createTimeMicros= */ 6000000L, /* version= */ 1);
134135

135136
BundleReader bundleReader =
136-
new BundleReader(
137-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
137+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
138138

139139
List<BundleElement> bundleElements =
140140
verifyAllElements(bundleReader, limitQuery, limitToLastQuery, documentMetadata, document);
@@ -157,8 +157,7 @@ public void testReadsQueryAndDocumentWithUnexpectedOrder() throws IOException, J
157157
bundleBuilder.build("bundle-1", /* createTimeMicros= */ 6000000L, /* version= */ 1);
158158

159159
BundleReader bundleReader =
160-
new BundleReader(
161-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
160+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
162161

163162
List<BundleElement> bundleElements =
164163
verifyAllElements(bundleReader, doc1Metadata, doc1, limitQuery, doc2Metadata, doc2);
@@ -179,8 +178,7 @@ public void testReadsWithoutNamedQuery() throws IOException, JSONException {
179178
bundleBuilder.build("bundle-1", /* createTimeMicros= */ 6000000L, /* version= */ 1);
180179

181180
BundleReader bundleReader =
182-
new BundleReader(
183-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
181+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
184182

185183
List<BundleElement> bundleElements =
186184
verifyAllElements(bundleReader, documentMetadata, document);
@@ -199,8 +197,7 @@ public void testReadsWithDeletedDocument() throws IOException, JSONException {
199197
bundleBuilder.build("bundle-1", /* createTimeMicros= */ 6000000L, /* version= */ 1);
200198

201199
BundleReader bundleReader =
202-
new BundleReader(
203-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
200+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
204201

205202
List<BundleElement> bundleElements =
206203
verifyAllElements(bundleReader, deletedDocumentMetadata, documentMetadata, document);
@@ -217,8 +214,7 @@ public void testReadsWithoutDocumentOrQuery() throws IOException, JSONException
217214
bundleBuilder.build("bundle-1", /* createTimeMicros= */ 6000000L, /* version= */ 1);
218215

219216
BundleReader bundleReader =
220-
new BundleReader(
221-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
217+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
222218

223219
verifyAllElements(bundleReader);
224220
}
@@ -228,8 +224,7 @@ public void testThrowsWithoutLengthPrefix() throws IOException, JSONException {
228224
String bundle = "{metadata: 'no length prefix' }";
229225

230226
BundleReader bundleReader =
231-
new BundleReader(
232-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
227+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
233228

234229
bundleReader.getBundleMetadata();
235230
}
@@ -239,8 +234,7 @@ public void testThrowsWithMissingBrackets() throws IOException, JSONException {
239234
String bundle = "3abc";
240235

241236
BundleReader bundleReader =
242-
new BundleReader(
243-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
237+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
244238
bundleReader.getBundleMetadata();
245239
}
246240

@@ -249,8 +243,7 @@ public void testThrowsWithInvalidJSON() throws IOException, JSONException {
249243
String bundle = "3{abc}";
250244

251245
BundleReader bundleReader =
252-
new BundleReader(
253-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
246+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
254247
bundleReader.getBundleMetadata();
255248
}
256249

@@ -261,8 +254,7 @@ public void testThrowsWhenSecondElementIsMissing() throws IOException, JSONExcep
261254
bundleBuilder.build("bundle-1", /* createTimeMicros= */ 6000000L, /* version= */ 1) + "foo";
262255

263256
BundleReader bundleReader =
264-
new BundleReader(
265-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
257+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
266258
bundleReader.getNextElement();
267259
}
268260

@@ -271,8 +263,7 @@ public void testThrowsWhenBundleDoesNotContainEnoughData() throws IOException, J
271263
String bundle = "3{}";
272264

273265
BundleReader bundleReader =
274-
new BundleReader(
275-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
266+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
276267
bundleReader.getBundleMetadata();
277268
}
278269

@@ -290,8 +281,7 @@ public void testWhenFirstElementIsNotBundleMetadata() throws IOException, JSONEx
290281
String bundle = json.length() + json;
291282

292283
BundleReader bundleReader =
293-
new BundleReader(
294-
SERIALIZER, new ByteArrayInputStream(bundle.getBytes(StandardCharsets.UTF_8)));
284+
new BundleReader(SERIALIZER, new ByteArrayInputStream(bundle.getBytes(UTF8_CHARSET)));
295285

296286
bundleReader.getBundleMetadata();
297287
}
@@ -309,8 +299,7 @@ public void testCombinesMultipleBufferReads() throws IOException, JSONException
309299
"bundle-" + longString, /* createTimeMicros= */ 6000000L, /* version= */ 1);
310300

311301
BundleReader bundleReader =
312-
new BundleReader(
313-
SERIALIZER, new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)));
302+
new BundleReader(SERIALIZER, new ByteArrayInputStream(json.getBytes(UTF8_CHARSET)));
314303

315304
BundleMetadata expectedMetadata =
316305
new BundleMetadata("bundle-" + longString, 1, version(6000000L), 0, 0);
@@ -327,8 +316,7 @@ public void testReadsUnicodeData() throws IOException, JSONException {
327316
bundleBuilder.build("bundle-1", /* createTimeMicros= */ 6000000L, /* version= */ 1);
328317

329318
BundleReader bundleReader =
330-
new BundleReader(
331-
SERIALIZER, new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)));
319+
new BundleReader(SERIALIZER, new ByteArrayInputStream(json.getBytes(UTF8_CHARSET)));
332320

333321
List<BundleElement> bundleElements = verifyAllElements(bundleReader, docMetadata, doc);
334322

@@ -432,7 +420,7 @@ private List<BundleElement> verifyAllElements(
432420
}
433421
elements.add(nextElement);
434422

435-
int elementLength = expectedElement.getBytes(StandardCharsets.UTF_8).length;
423+
int elementLength = expectedElement.getBytes(UTF8_CHARSET).length;
436424
actualBytesRead += (int) (Math.log10(elementLength) + 1) + elementLength;
437425
assertEquals(actualBytesRead, bundleReader.getBytesRead());
438426
}
@@ -531,7 +519,7 @@ private int getUTF8BytesCountWithPrefix(String json) {
531519
}
532520

533521
private int getUTF8BytesCount(String json) {
534-
return json.getBytes(StandardCharsets.UTF_8).length;
522+
return json.getBytes(UTF8_CHARSET).length;
535523
}
536524

537525
String getMetadataElement(String id, long createTimeMicros, int version) {

0 commit comments

Comments
 (0)