ctubbsii commented on code in PR #5197:
URL: https://github.com/apache/accumulo/pull/5197#discussion_r2054979365
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/LoadMappingIterator.java:
##########
@@ -72,12 +76,30 @@ public boolean hasNext() {
@Override
public Map.Entry<KeyExtent,Bulk.Files> next() {
- Bulk.Mapping bm = gson.fromJson(reader, Bulk.Mapping.class);
- if (renameMap != null) {
- return new AbstractMap.SimpleEntry<>(bm.getKeyExtent(tableId),
- bm.getFiles().mapNames(renameMap));
- } else {
- return new AbstractMap.SimpleEntry<>(bm.getKeyExtent(tableId),
bm.getFiles());
+ try {
+ Bulk.Mapping bm = gson.fromJson(reader, Bulk.Mapping.class);
+ if (bm == null) {
+ throw new NoSuchElementException("No more elements in input");
+ }
Review Comment:
Should probably add test cases for this, and the other exceptions that can
be thrown due to malformed JSON
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/LoadMappingIterator.java:
##########
@@ -72,12 +76,30 @@ public boolean hasNext() {
@Override
public Map.Entry<KeyExtent,Bulk.Files> next() {
- Bulk.Mapping bm = gson.fromJson(reader, Bulk.Mapping.class);
- if (renameMap != null) {
- return new AbstractMap.SimpleEntry<>(bm.getKeyExtent(tableId),
- bm.getFiles().mapNames(renameMap));
- } else {
- return new AbstractMap.SimpleEntry<>(bm.getKeyExtent(tableId),
bm.getFiles());
+ try {
+ Bulk.Mapping bm = gson.fromJson(reader, Bulk.Mapping.class);
+ if (bm == null) {
+ throw new NoSuchElementException("No more elements in input");
+ }
+
+ KeyExtent currentKeyExtent = bm.getKeyExtent(tableId);
+
+ if (lastKeyExtent != null && currentKeyExtent.compareTo(lastKeyExtent) <
0) {
+ throw new IllegalStateException(
+ String.format("KeyExtents are not in sorted order: %s comes after
%s", lastKeyExtent,
+ currentKeyExtent));
+ }
+
+ lastKeyExtent = currentKeyExtent;
+
+ if (renameMap != null) {
+ return new AbstractMap.SimpleEntry<>(currentKeyExtent,
bm.getFiles().mapNames(renameMap));
+ } else {
+ return new AbstractMap.SimpleEntry<>(currentKeyExtent, bm.getFiles());
+ }
+
+ } catch (JsonSyntaxException | JsonIOException e) {
Review Comment:
These are both JsonParseException, so you only need to catch that parent.
You could also shorten this try-catch block to just wrap the JSON parsing code,
and not the entire method. Most of the rest of the logic can exist after the
try-catch block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
