[GitHub] WillemJiang closed pull request #519: [SCB-256] fix bug: SC 0.5 is not compatible to 0.4, change sdk to suppo?

[GitBox <gi...@apache.org>]

WillemJiang closed pull request #519: [SCB-256] fix bug: SC 0.5 is not compatible to 0.4, change sdk to suppo?
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/519
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java
index 97a40d80a..64f9a9361 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java
@@ -57,6 +57,9 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.annotations.VisibleForTesting;
+
+import io.netty.handler.codec.http.HttpStatusClass;
 import io.vertx.core.Handler;
 import io.vertx.core.buffer.Buffer;
 import io.vertx.core.http.HttpClientResponse;
@@ -85,8 +88,9 @@ private void retry(RequestContext requestContext, Handler<RestResponse> response
     RestUtils.httpDo(requestContext, responseHandler);
   }
 
+  @VisibleForTesting
   @SuppressWarnings("unchecked")
-  private <T> Handler<RestResponse> syncHandler(CountDownLatch countDownLatch, Class<T> cls,
+  protected <T> Handler<RestResponse> syncHandler(CountDownLatch countDownLatch, Class<T> cls,
       Holder<T> holder) {
     return restResponse -> {
       RequestContext requestContext = restResponse.getRequestContext();
@@ -107,6 +111,18 @@ private void retry(RequestContext requestContext, Handler<RestResponse> response
               countDownLatch.countDown();
               return;
             }
+
+            // no need to support 304 in this place
+            if (!HttpStatusClass.SUCCESS.equals(HttpStatusClass.valueOf(response.statusCode()))) {
+              LOGGER.warn("get response for {} failed, {}:{}, {}",
+                  cls.getName(),
+                  response.statusCode(),
+                  response.statusMessage(),
+                  bodyBuffer.toString());
+              countDownLatch.countDown();
+              return;
+            }
+
             try {
               holder.value =
                   JsonUtils.readValue(bodyBuffer.getBytes(), cls);
@@ -259,7 +275,7 @@ public boolean isSchemaExist(String microserviceId, String schemaId) {
           schemaId,
           e);
     }
-    return holder.value != null;
+    return holder.value != null && schemaId.equals(holder.value.getSchemaId());
   }
 
   @Override
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestServiceRegistryClientImpl.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestServiceRegistryClientImpl.java
index a8716ddc2..a171531a9 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestServiceRegistryClientImpl.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestServiceRegistryClientImpl.java
@@ -23,6 +23,7 @@
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 
+import javax.ws.rs.core.Response.Status;
 import javax.xml.ws.Holder;
 
 import org.apache.log4j.Appender;
@@ -30,6 +31,7 @@
 import org.apache.log4j.spi.LoggingEvent;
 import org.apache.servicecomb.serviceregistry.api.registry.Microservice;
 import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceFactory;
+import org.apache.servicecomb.serviceregistry.api.response.GetExistenceResponse;
 import org.apache.servicecomb.serviceregistry.client.ClientException;
 import org.apache.servicecomb.serviceregistry.client.IpPortManager;
 import org.apache.servicecomb.serviceregistry.client.http.ServiceRegistryClientImpl.ResponseWrapper;
@@ -46,6 +48,7 @@
 import io.vertx.core.http.HttpClientResponse;
 import io.vertx.core.http.HttpVersion;
 import mockit.Deencapsulation;
+import mockit.Expectations;
 import mockit.Mock;
 import mockit.MockUp;
 import mockit.Mocked;
@@ -252,6 +255,54 @@ void doRun(java.util.List<LoggingEvent> events) {
     }.run();
   }
 
+  @Test
+  public void syncHandler_failed(@Mocked RequestContext requestContext,
+      @Mocked HttpClientResponse response) {
+    CountDownLatch countDownLatch = new CountDownLatch(1);
+    Class<GetExistenceResponse> cls = GetExistenceResponse.class;
+    Holder<GetExistenceResponse> holder = new Holder<>();
+    Handler<RestResponse> handler = oClient.syncHandler(countDownLatch, cls, holder);
+
+    Holder<Handler<Buffer>> bodyHandlerHolder = new Holder<>();
+    new MockUp<HttpClientResponse>(response) {
+      @Mock
+      HttpClientResponse bodyHandler(Handler<Buffer> bodyHandler) {
+        bodyHandlerHolder.value = bodyHandler;
+        return null;
+      }
+    };
+    new Expectations() {
+      {
+        response.statusCode();
+        result = 400;
+        response.statusMessage();
+        result = Status.BAD_REQUEST.getReasonPhrase();
+      }
+    };
+
+    RestResponse event = new RestResponse(requestContext, response);
+    handler.handle(event);
+
+    Buffer bodyBuffer = Buffer.buffer("{}");
+    bodyHandlerHolder.value.handle(bodyBuffer);
+
+    Assert.assertNull(holder.value);
+  }
+
+  @Test
+  public void isSchemaExist() {
+    String microserviceId = "msId";
+    String schemaId = "schemaId";
+
+    new MockUp<RestUtils>() {
+      @Mock
+      void httpDo(RequestContext requestContext, Handler<RestResponse> responseHandler) {
+        Holder<GetExistenceResponse> holder = Deencapsulation.getField(responseHandler, "arg$4");
+        holder.value = new GetExistenceResponse();
+      }
+    };
+    Assert.assertFalse(oClient.isSchemaExist(microserviceId, schemaId));
+  }
 
   @Test
   public void testFindServiceInstance() {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services