From 4b3c42f3555a9367d32b7a050e07979a1cf0d6e3 Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Tue, 17 Sep 2019 09:11:35 +0100 Subject: [PATCH] =?UTF-8?q?HBASE-22846=20Internal=20Error=20500=20when=20U?= =?UTF-8?q?sing=20HBASE=20REST=20API=20to=20Create=20Na=E2=80=A6=20(#524)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: stack (cherry picked from commit f6ff970f397cc6cdef6608ec8de352b139ea11c6) --- .../rest/NamespacesInstanceResource.java | 71 +++++-------------- .../rest/model/NamespacesInstanceModel.java | 1 + .../rest/TestNamespacesInstanceResource.java | 25 +++++-- 3 files changed, 38 insertions(+), 59 deletions(-) diff --git a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/NamespacesInstanceResource.java b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/NamespacesInstanceResource.java index 3ff25f99ef7..215abaea0b3 100644 --- a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/NamespacesInstanceResource.java +++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/NamespacesInstanceResource.java @@ -135,35 +135,9 @@ public class NamespacesInstanceResource extends ResourceBase { @Consumes({MIMETYPE_XML, MIMETYPE_JSON, MIMETYPE_PROTOBUF, MIMETYPE_PROTOBUF_IETF}) public Response put(final NamespacesInstanceModel model, final @Context UriInfo uriInfo) { - if (LOG.isTraceEnabled()) { - LOG.trace("PUT " + uriInfo.getAbsolutePath()); - } - servlet.getMetrics().incrementRequests(1); return processUpdate(model, true, uriInfo); } - /** - * Build a response for PUT alter namespace with no properties specified. - * @param message value not used. - * @param headers value not used. - * @return response code. - */ - @PUT - public Response putNoBody(final byte[] message, - final @Context UriInfo uriInfo, final @Context HttpHeaders headers) { - if (LOG.isTraceEnabled()) { - LOG.trace("PUT " + uriInfo.getAbsolutePath()); - } - servlet.getMetrics().incrementRequests(1); - try{ - NamespacesInstanceModel model = new NamespacesInstanceModel(namespace); - return processUpdate(model, true, uriInfo); - }catch(IOException ioe){ - servlet.getMetrics().incrementFailedPutRequests(1); - throw new RuntimeException("Cannot retrieve info for '" + namespace + "'."); - } - } - /** * Build a response for POST create namespace with properties specified. * @param model properties used for create. @@ -175,39 +149,26 @@ public class NamespacesInstanceResource extends ResourceBase { MIMETYPE_PROTOBUF_IETF}) public Response post(final NamespacesInstanceModel model, final @Context UriInfo uriInfo) { - - if (LOG.isTraceEnabled()) { - LOG.trace("POST " + uriInfo.getAbsolutePath()); - } - servlet.getMetrics().incrementRequests(1); return processUpdate(model, false, uriInfo); } - /** - * Build a response for POST create namespace with no properties specified. - * @param message value not used. - * @param headers value not used. - * @return response code. - */ - @POST - public Response postNoBody(final byte[] message, - final @Context UriInfo uriInfo, final @Context HttpHeaders headers) { - if (LOG.isTraceEnabled()) { - LOG.trace("POST " + uriInfo.getAbsolutePath()); - } - servlet.getMetrics().incrementRequests(1); - try{ - NamespacesInstanceModel model = new NamespacesInstanceModel(namespace); - return processUpdate(model, false, uriInfo); - }catch(IOException ioe){ - servlet.getMetrics().incrementFailedPutRequests(1); - throw new RuntimeException("Cannot retrieve info for '" + namespace + "'."); - } - } // Check that POST or PUT is valid and then update namespace. - private Response processUpdate(final NamespacesInstanceModel model, final boolean updateExisting, + private Response processUpdate(NamespacesInstanceModel model, final boolean updateExisting, final UriInfo uriInfo) { + if (LOG.isTraceEnabled()) { + LOG.trace((updateExisting ? "PUT " : "POST ") + uriInfo.getAbsolutePath()); + } + if (model == null) { + try { + model = new NamespacesInstanceModel(namespace); + } catch(IOException ioe) { + servlet.getMetrics().incrementFailedPutRequests(1); + throw new RuntimeException("Cannot retrieve info for '" + namespace + "'."); + } + } + servlet.getMetrics().incrementRequests(1); + if (servlet.isReadOnly()) { servlet.getMetrics().incrementFailedPutRequests(1); return Response.status(Response.Status.FORBIDDEN).type(MIMETYPE_TEXT) @@ -265,7 +226,9 @@ public class NamespacesInstanceResource extends ResourceBase { } servlet.getMetrics().incrementSucessfulPutRequests(1); - return Response.created(uriInfo.getAbsolutePath()).build(); + + return updateExisting ? Response.ok(uriInfo.getAbsolutePath()).build() : + Response.created(uriInfo.getAbsolutePath()).build(); } private boolean doesNamespaceExist(Admin admin, String namespaceName) throws IOException{ diff --git a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesInstanceModel.java b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesInstanceModel.java index 022ec38d3bf..af3b0b067a4 100644 --- a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesInstanceModel.java +++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/model/NamespacesInstanceModel.java @@ -166,4 +166,5 @@ public class NamespacesInstanceModel implements Serializable, ProtobufMessageHan } return this; } + } diff --git a/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestNamespacesInstanceResource.java b/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestNamespacesInstanceResource.java index e0beeab4909..53eeecb4890 100644 --- a/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestNamespacesInstanceResource.java +++ b/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestNamespacesInstanceResource.java @@ -50,6 +50,7 @@ import org.apache.hadoop.hbase.rest.model.TestNamespacesInstanceModel; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.testclassification.RestTests; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.http.Header; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.ClassRule; @@ -329,6 +330,12 @@ public class TestNamespacesInstanceResource { jsonString = jsonMapper.writeValueAsString(model2); response = client.post(namespacePath2, Constants.MIMETYPE_JSON, Bytes.toBytes(jsonString)); assertEquals(201, response.getCode()); + //check passing null content-type with a payload returns 415 + Header[] nullHeaders = null; + response = client.post(namespacePath1, nullHeaders, toXML(model1)); + assertEquals(415, response.getCode()); + response = client.post(namespacePath1, nullHeaders, Bytes.toBytes(jsonString)); + assertEquals(415, response.getCode()); // Check that created namespaces correctly. nd1 = findNamespace(admin, NAMESPACE1); @@ -379,8 +386,12 @@ public class TestNamespacesInstanceResource { model4 = testNamespacesInstanceModel.buildTestModel(NAMESPACE4, NAMESPACE4_PROPS); testNamespacesInstanceModel.checkModel(model4, NAMESPACE4, NAMESPACE4_PROPS); + //Defines null headers for use in tests where no body content is provided, so that we set + // no content-type in the request + Header[] nullHeaders = null; + // Test cannot PUT (alter) non-existent namespace. - response = client.put(namespacePath3, Constants.MIMETYPE_BINARY, new byte[]{}); + response = client.put(namespacePath3, nullHeaders, new byte[]{}); assertEquals(403, response.getCode()); response = client.put(namespacePath4, Constants.MIMETYPE_PROTOBUF, model4.createProtobufOutput()); @@ -388,7 +399,7 @@ public class TestNamespacesInstanceResource { // Test cannot create tables when in read only mode. conf.set("hbase.rest.readonly", "true"); - response = client.post(namespacePath3, Constants.MIMETYPE_BINARY, new byte[]{}); + response = client.post(namespacePath3, nullHeaders, new byte[]{}); assertEquals(403, response.getCode()); response = client.put(namespacePath4, Constants.MIMETYPE_PROTOBUF, model4.createProtobufOutput()); @@ -399,12 +410,16 @@ public class TestNamespacesInstanceResource { assertNull(nd4); conf.set("hbase.rest.readonly", "false"); - // Create namespace via no body and protobuf. - response = client.post(namespacePath3, Constants.MIMETYPE_BINARY, new byte[]{}); + // Create namespace with no body and binary content type. + response = client.post(namespacePath3, nullHeaders, new byte[]{}); assertEquals(201, response.getCode()); + // Create namespace with protobuf content-type. response = client.post(namespacePath4, Constants.MIMETYPE_PROTOBUF, model4.createProtobufOutput()); assertEquals(201, response.getCode()); + //check setting unsupported content-type returns 415 + response = client.post(namespacePath3, Constants.MIMETYPE_BINARY, new byte[]{}); + assertEquals(415, response.getCode()); // Check that created namespaces correctly. nd3 = findNamespace(admin, NAMESPACE3); @@ -415,7 +430,7 @@ public class TestNamespacesInstanceResource { checkNamespaceProperties(nd4, NAMESPACE4_PROPS); // Check cannot post tables that already exist. - response = client.post(namespacePath3, Constants.MIMETYPE_BINARY, new byte[]{}); + response = client.post(namespacePath3, nullHeaders, new byte[]{}); assertEquals(403, response.getCode()); response = client.post(namespacePath4, Constants.MIMETYPE_PROTOBUF, model4.createProtobufOutput());