diff --git hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java index f8b5813..a3aa875 100644 --- hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java +++ hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java @@ -1306,10 +1306,10 @@ public class KeyValue implements Cell, HeapSize, Cloneable { // If no delimiter, return array of size 1 return new byte [][] { c }; } else if(index == c.length - 1) { - // Only a family, return array size 1 + // family with empty qualifier, return array size 2 byte [] family = new byte[c.length-1]; System.arraycopy(c, 0, family, 0, family.length); - return new byte [][] { family }; + return new byte [][] { family, HConstants.EMPTY_BYTE_ARRAY}; } // Family and column, return array size 2 final byte [][] result = new byte [2][]; diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java index 56ad585..8e6cccd 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowResource.java @@ -92,6 +92,7 @@ public class RowResource extends ResourceBase { ResultGenerator generator = ResultGenerator.fromRowSpec(tableResource.getName(), rowspec, null); if (!generator.hasNext()) { + servlet.getMetrics().incrementFailedGetRequests(1); return Response.status(Response.Status.NOT_FOUND) .type(MIMETYPE_TEXT).entity("Not found" + CRLF) .build(); @@ -118,7 +119,7 @@ public class RowResource extends ResourceBase { servlet.getMetrics().incrementSucessfulGetRequests(1); return Response.ok(model).build(); } catch (RuntimeException e) { - servlet.getMetrics().incrementFailedPutRequests(1); + servlet.getMetrics().incrementFailedGetRequests(1); if (e.getCause() instanceof TableNotFoundException) { return Response.status(Response.Status.NOT_FOUND) .type(MIMETYPE_TEXT).entity("Not found" + CRLF) @@ -128,7 +129,7 @@ public class RowResource extends ResourceBase { .type(MIMETYPE_TEXT).entity("Bad request" + CRLF) .build(); } catch (Exception e) { - servlet.getMetrics().incrementFailedPutRequests(1); + servlet.getMetrics().incrementFailedGetRequests(1); return Response.status(Response.Status.SERVICE_UNAVAILABLE) .type(MIMETYPE_TEXT).entity("Unavailable" + CRLF) .build(); @@ -145,14 +146,15 @@ public class RowResource extends ResourceBase { // doesn't make sense to use a non specific coordinate as this can only // return a single cell if (!rowspec.hasColumns() || rowspec.getColumns().length > 1) { - return Response.status(Response.Status.BAD_REQUEST) - .type(MIMETYPE_TEXT).entity("Bad request" + CRLF) - .build(); + servlet.getMetrics().incrementFailedGetRequests(1); + return Response.status(Response.Status.BAD_REQUEST).type(MIMETYPE_TEXT) + .entity("Bad request: Either 0 or more than 1 columns specified." + CRLF).build(); } try { ResultGenerator generator = ResultGenerator.fromRowSpec(tableResource.getName(), rowspec, null); if (!generator.hasNext()) { + servlet.getMetrics().incrementFailedGetRequests(1); return Response.status(Response.Status.NOT_FOUND) .type(MIMETYPE_TEXT).entity("Not found" + CRLF) .build(); diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowResultGenerator.java hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowResultGenerator.java index fbd16e8..f96fa57 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowResultGenerator.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowResultGenerator.java @@ -28,6 +28,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.client.Get; import org.apache.hadoop.hbase.client.HTableInterface; @@ -50,8 +51,12 @@ public class RowResultGenerator extends ResultGenerator { if (rowspec.hasColumns()) { for (byte[] col: rowspec.getColumns()) { byte[][] split = KeyValue.parseColumn(col); - if (split.length == 2 && split[1].length != 0) { - get.addColumn(split[0], split[1]); + if (split.length == 2) { + if (split[1].length != 0) { + get.addColumn(split[0], split[1]); + } else { + get.addColumn(split[0], HConstants.EMPTY_BYTE_ARRAY); + } } else { get.addFamily(split[0]); } diff --git hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestRowResource.java hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestRowResource.java index da6f69d..e07d8b7 100644 --- hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestRowResource.java +++ hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestRowResource.java @@ -26,6 +26,7 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.StringWriter; import java.net.URLEncoder; +import java.util.List; import javax.xml.bind.JAXBContext; import javax.xml.bind.JAXBException; @@ -64,6 +65,7 @@ public class TestRowResource { private static final String CFB = "b"; private static final String COLUMN_1 = CFA + ":1"; private static final String COLUMN_2 = CFB + ":2"; + private static final String COLUMN_3 = CFA + ":"; private static final String ROW_1 = "testrow1"; private static final String VALUE_1 = "testvalue1"; private static final String ROW_2 = "testrow2"; @@ -74,7 +76,7 @@ public class TestRowResource { private static final String VALUE_4 = "testvalue4"; private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); - private static final HBaseRESTTestingUtility REST_TEST_UTIL = + private static final HBaseRESTTestingUtility REST_TEST_UTIL = new HBaseRESTTestingUtility(); private static final MetricsAssertHelper METRICS_ASSERT = CompatibilityFactory.getInstance(MetricsAssertHelper.class); @@ -696,7 +698,7 @@ public class TestRowResource { for (CellModel cell: rowModel.getCells()) { assertEquals(COLUMN_1, Bytes.toString(cell.getColumn())); assertEquals(values[i], Bytes.toString(cell.getValue())); - } + } } for (String row : rows) { response = deleteRow(TABLE, row); @@ -721,5 +723,53 @@ public class TestRowResource { assertEquals(response.getCode(), 400); } + @Test + public void testMultiColumnGetXML() throws Exception { + String path = "/" + TABLE + "/fakerow"; + CellSetModel cellSetModel = new CellSetModel(); + RowModel rowModel = new RowModel(ROW_1); + rowModel.addCell(new CellModel(Bytes.toBytes(COLUMN_1), Bytes.toBytes(VALUE_1))); + rowModel.addCell(new CellModel(Bytes.toBytes(COLUMN_2), Bytes.toBytes(VALUE_2))); + rowModel.addCell(new CellModel(Bytes.toBytes(COLUMN_3), Bytes.toBytes(VALUE_2))); + cellSetModel.addRow(rowModel); + StringWriter writer = new StringWriter(); + marshaller.marshal(cellSetModel, writer); + + Response response = client.put(path, Constants.MIMETYPE_XML, Bytes.toBytes(writer.toString())); + Thread.yield(); + + // make sure the fake row was not actually created + response = client.get(path, Constants.MIMETYPE_XML); + assertEquals(response.getCode(), 404); + + // Try getting all the column values at once. + path = "/" + TABLE + "/" + ROW_1 + "/" + COLUMN_1 + "," + COLUMN_2 + "," + COLUMN_3; + response = client.get(path, Constants.MIMETYPE_XML); + assertEquals(200, response.getCode()); + CellSetModel cellSet = (CellSetModel) unmarshaller.unmarshal(new ByteArrayInputStream(response + .getBody())); + assertTrue(cellSet.getRows().size() == 1); + assertTrue(cellSet.getRows().get(0).getCells().size() == 3); + List cells = cellSet.getRows().get(0).getCells(); + + assertTrue(containsCellModel(cells, COLUMN_1, VALUE_1)); + assertTrue(containsCellModel(cells, COLUMN_2, VALUE_2)); + assertTrue(containsCellModel(cells, COLUMN_3, VALUE_2)); + response = deleteRow(TABLE, ROW_1); + assertEquals(response.getCode(), 200); + } + + private boolean containsCellModel(List cells, String column, String value) { + boolean contains = false; + for (CellModel cell : cells) { + if (Bytes.toString(cell.getColumn()).equals(column) + && Bytes.toString(cell.getValue()).equals(value)) { + contains = true; + return contains; + } + } + return contains; + } + }