Skip to content

HBASE-28627 REST ScannerModel doesn't support includeStartRow/includeStopRow(addendum) #6499

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 10, 2025
Merged

HBASE-28627 REST ScannerModel doesn't support includeStartRow/includeStopRow(addendum) #6499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/ScannerModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,10 @@ public class ScannerModel implements ProtobufMessageHandler, Serializable {
private boolean cacheBlocks = true;
private int limit = -1;

@JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = IncludeStartRowFilter.class)
private boolean includeStartRow = true;

@JsonInclude(value = JsonInclude.Include.NON_DEFAULT)
private boolean includeStopRow = false;

@XmlAttribute
Expand All @@ -143,6 +145,16 @@ public void setIncludeStartRow(boolean includeStartRow) {
this.includeStartRow = includeStartRow;
}

@edu.umd.cs.findbugs.annotations.SuppressWarnings(
value = { "EQ_CHECK_FOR_OPERAND_NOT_COMPATIBLE_WITH_THIS", "HE_EQUALS_NO_HASHCODE" },
justification = "1.The supplied value from the JSON Value Filter is of Type Boolean, hence supressing the check, 2.hashCode method will not be invoked, hence supressing the check")
private static class IncludeStartRowFilter {
@Override
public boolean equals(Object value) {
return Boolean.TRUE.equals(value);
Copy link
Contributor

@stoty stoty Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this code is not performance critial, we could just check for the type and avoid the suppression:

if(value instanceOf Boolean) {
   return Boolean.TRUE.equals(value);
} else {
   return false;
}

Copy link
Contributor Author

@chandrasekhar-188k chandrasekhar-188k Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

according to the documents EQ_CHECK_FOR_OPERAND_NOT_COMPATIBLE_WITH_THIS checks if we are doing an instanceof check on any thing other than the current class. in this case it is IncludeStartRowFilter.
the equals() implementation of Boolean class as below.
image
Since it is trying to check instanceof Boolean which is not in the Hierachy of IncludeStartRowFilter find bugs is reporting it as voilation.
I had tried your suggestion before, but it still fails due to above reason.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you love static checkers ?

OK, let's leave it like this.

}
}

/**
* Implement lazily-instantiated singleton as per recipe here:
* http://literatejava.com/jvm/fastest-threadsafe-singleton-jvm/
Expand Down
60 changes: 58 additions & 2 deletions hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/model/TestScannerModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import com.fasterxml.jackson.core.JsonParseException;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.rest.ScannerResultGenerator;
import org.apache.hadoop.hbase.testclassification.RestTests;
Expand Down Expand Up @@ -62,8 +63,7 @@ public TestScannerModel() throws Exception {
AS_JSON = "{\"batch\":100,\"caching\":1000,\"cacheBlocks\":false,\"endRow\":\"enp5eng=\","
+ "\"endTime\":1245393318192,\"maxVersions\":2147483647,\"startRow\":\"YWJyYWNhZGFicmE=\","
+ "\"startTime\":1245219839331,\"column\":[\"Y29sdW1uMQ==\",\"Y29sdW1uMjpmb28=\"],"
+ "\"labels\":[\"private\",\"public\"]," + "\"limit\":10000,"
+ "\"includeStartRow\":true,\"includeStopRow\":false}";
+ "\"labels\":[\"private\",\"public\"]," + "\"limit\":10000}";

AS_PB = "CgthYnJhY2FkYWJyYRIFenp5engaB2NvbHVtbjEaC2NvbHVtbjI6Zm9vIGQo47qL554kMLDi57mfJDj"
+ "/////B0joB1IHcHJpdmF0ZVIGcHVibGljWABgkE4=";
Expand Down Expand Up @@ -151,4 +151,60 @@ private void verifyException(final String FILTER) throws Exception {
model.setFilter(FILTER);
ScannerResultGenerator.buildFilterFromModel(model);
}

@Test()
public void testToJsonWithIncludeStartRowAndIncludeStopRow() throws Exception {
String jsonStr =
"{\"batch\":100,\"caching\":1000,\"cacheBlocks\":false,\"endRow\":\"enp5eng=\","
+ "\"endTime\":1245393318192,\"maxVersions\":2147483647,\"startRow\":\"YWJyYWNhZGFicmE=\","
+ "\"startTime\":1245219839331,\"column\":[\"Y29sdW1uMQ==\",\"Y29sdW1uMjpmb28=\"],"
+ "\"labels\":[\"private\",\"public\"]," + "\"limit\":10000,"
+ "\"includeStartRow\":false,\"includeStopRow\":true}";

ObjectNode expObj = mapper.readValue(jsonStr, ObjectNode.class);
ObjectNode actObj = mapper.readValue(
toJSON(buildTestModelWithIncludeStartRowAndIncludeStopRow(false, true)), ObjectNode.class);
assertEquals(expObj, actObj);

jsonStr = "{\"batch\":100,\"caching\":1000,\"cacheBlocks\":false,\"endRow\":\"enp5eng=\","
+ "\"endTime\":1245393318192,\"maxVersions\":2147483647,\"startRow\":\"YWJyYWNhZGFicmE=\","
+ "\"startTime\":1245219839331,\"column\":[\"Y29sdW1uMQ==\",\"Y29sdW1uMjpmb28=\"],"
+ "\"labels\":[\"private\",\"public\"]," + "\"limit\":10000," + "\"includeStopRow\":true}";

expObj = mapper.readValue(jsonStr, ObjectNode.class);
actObj = mapper.readValue(
toJSON(buildTestModelWithIncludeStartRowAndIncludeStopRow(true, true)), ObjectNode.class);
assertEquals(expObj, actObj);

jsonStr = "{\"batch\":100,\"caching\":1000,\"cacheBlocks\":false,\"endRow\":\"enp5eng=\","
+ "\"endTime\":1245393318192,\"maxVersions\":2147483647,\"startRow\":\"YWJyYWNhZGFicmE=\","
+ "\"startTime\":1245219839331,\"column\":[\"Y29sdW1uMQ==\",\"Y29sdW1uMjpmb28=\"],"
+ "\"labels\":[\"private\",\"public\"]," + "\"limit\":10000," + "\"includeStartRow\":false}";

expObj = mapper.readValue(jsonStr, ObjectNode.class);
actObj = mapper.readValue(
toJSON(buildTestModelWithIncludeStartRowAndIncludeStopRow(false, false)), ObjectNode.class);
assertEquals(expObj, actObj);

}

protected ScannerModel buildTestModelWithIncludeStartRowAndIncludeStopRow(boolean includeStartRow,
boolean includeStopRow) {
ScannerModel model = new ScannerModel();
model.setStartRow(START_ROW);
model.setEndRow(END_ROW);
model.addColumn(COLUMN1);
model.addColumn(COLUMN2);
model.setStartTime(START_TIME);
model.setEndTime(END_TIME);
model.setBatch(BATCH);
model.setCaching(CACHING);
model.addLabel(PRIVATE);
model.addLabel(PUBLIC);
model.setCacheBlocks(CACHE_BLOCKS);
model.setLimit(LIMIT);
model.setIncludeStartRow(includeStartRow);
model.setIncludeStopRow(includeStopRow);
return model;
}
}