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/main/java/org/apache/hadoop/hbase/rest/RowSpec.java hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowSpec.java index 814ed3b..b8e70d4 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowSpec.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/rest/RowSpec.java @@ -122,11 +122,7 @@ public class RowSpec { } String s = URLDecoder.decode(column.toString(), HConstants.UTF8_ENCODING); - if (!s.contains(":")) { - this.columns.add(Bytes.toBytes(s + ":")); - } else { this.columns.add(Bytes.toBytes(s)); - } column.setLength(0); i++; continue; @@ -136,14 +132,10 @@ public class RowSpec { } i++; // trailing list entry - if (column.length() > 1) { + if (column.length() >= 1) { String s = URLDecoder.decode(column.toString(), HConstants.UTF8_ENCODING); - if (!s.contains(":")) { - this.columns.add(Bytes.toBytes(s + ":")); - } else { this.columns.add(Bytes.toBytes(s)); - } } } catch (IndexOutOfBoundsException e) { throw new IllegalArgumentException(e); diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java hbase-server/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java index 1fbf56a..59e902d 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java @@ -115,9 +115,11 @@ public class RemoteHTable implements HTableInterface { sb.append(','); } } + } else if(quals == null){ + sb.append(Bytes.toStringBinary((byte[])e.getKey())); } else { - sb.append(Bytes.toStringBinary((byte[])e.getKey())); - sb.append(':'); + sb.append(Bytes.toStringBinary((byte[])e.getKey())); + sb.append(':'); } if (i.hasNext()) { sb.append(','); 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; + } + } diff --git hbase-server/src/test/java/org/apache/hadoop/hbase/rest/client/TestRemoteTable.java hbase-server/src/test/java/org/apache/hadoop/hbase/rest/client/TestRemoteTable.java index b5972c0..77dfa5c 100644 --- hbase-server/src/test/java/org/apache/hadoop/hbase/rest/client/TestRemoteTable.java +++ hbase-server/src/test/java/org/apache/hadoop/hbase/rest/client/TestRemoteTable.java @@ -148,7 +148,7 @@ public class TestRemoteTable { assertNull(value2); get = new Get(ROW_2); - result = remoteTable.get(get); + result = remoteTable.get(get); value1 = result.getValue(COLUMN_1, QUALIFIER_1); value2 = result.getValue(COLUMN_2, QUALIFIER_2); assertNotNull(value1); @@ -158,7 +158,7 @@ public class TestRemoteTable { get = new Get(ROW_2); get.addFamily(COLUMN_1); - result = remoteTable.get(get); + result = remoteTable.get(get); value1 = result.getValue(COLUMN_1, QUALIFIER_1); value2 = result.getValue(COLUMN_2, QUALIFIER_2); assertNotNull(value1); @@ -168,7 +168,7 @@ public class TestRemoteTable { get = new Get(ROW_2); get.addColumn(COLUMN_1, QUALIFIER_1); get.addColumn(COLUMN_2, QUALIFIER_2); - result = remoteTable.get(get); + result = remoteTable.get(get); value1 = result.getValue(COLUMN_1, QUALIFIER_1); value2 = result.getValue(COLUMN_2, QUALIFIER_2); assertNotNull(value1); @@ -182,7 +182,7 @@ public class TestRemoteTable { get.addFamily(COLUMN_1); get.addFamily(COLUMN_2); get.setTimeStamp(TS_1); - result = remoteTable.get(get); + result = remoteTable.get(get); value1 = result.getValue(COLUMN_1, QUALIFIER_1); value2 = result.getValue(COLUMN_2, QUALIFIER_2); assertNotNull(value1); @@ -195,7 +195,7 @@ public class TestRemoteTable { get.addFamily(COLUMN_1); get.addFamily(COLUMN_2); get.setTimeRange(0, TS_1 + 1); - result = remoteTable.get(get); + result = remoteTable.get(get); value1 = result.getValue(COLUMN_1, QUALIFIER_1); value2 = result.getValue(COLUMN_2, QUALIFIER_2); assertNotNull(value1); @@ -325,7 +325,7 @@ public class TestRemoteTable { Delete delete = new Delete(ROW_3); delete.deleteColumn(COLUMN_2, QUALIFIER_2); remoteTable.delete(delete); - + get = new Get(ROW_3); get.addFamily(COLUMN_1); get.addFamily(COLUMN_2); @@ -349,7 +349,7 @@ public class TestRemoteTable { assertNotNull(value1); assertTrue(Bytes.equals(VALUE_1, value1)); assertNull(value2); - + delete = new Delete(ROW_3); remoteTable.delete(delete); @@ -360,7 +360,7 @@ public class TestRemoteTable { value1 = result.getValue(COLUMN_1, QUALIFIER_1); value2 = result.getValue(COLUMN_2, QUALIFIER_2); assertNull(value1); - assertNull(value2); + assertNull(value2); } public void testScanner() throws IOException { diff --git hbase-server/src/test/ruby/hbase/table_test.rb hbase-server/src/test/ruby/hbase/table_test.rb index 8170c53..ee494bc 100644 --- hbase-server/src/test/ruby/hbase/table_test.rb +++ hbase-server/src/test/ruby/hbase/table_test.rb @@ -78,10 +78,10 @@ module Hbase assert_nil(qual) end - define_test "parse_column_name should not return a qualifier for family-only column specifiers" do + define_test "parse_column_name should return empty qualifier for family-only column specifiers" do col, qual = table('hbase:meta').parse_column_name('foo:') assert_not_nil(col) - assert_nil(qual) + assert_not_nil(qual) end define_test "parse_column_name should return a qualifier for family:qualifier column specifiers" do