[SCB-926] support invoking 3rd party service #915

yhs0092 · 2018-09-19T09:05:07Z

Follow this checklist to help us incorporate your contribution quickly and easily:

Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
Each commit in the pull request should have a meaningful subject line and body.
Format the pull request title like [SCB-XXX] Fixes bug in ApproximateQuantiles, where you replace SCB-XXX with the appropriate JIRA issue.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Run mvn clean install to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

Support invoking 3rd party service just like invoking ServiceComb microservice registered in service-center.

coveralls · 2018-09-19T09:27:32Z

Coverage increased (+0.08%) to 86.194% when pulling f210638 on yhs0092:3rd_party_invocation into c019638 on apache:master.

wujimin · 2018-09-19T14:27:13Z

service-registry/src/main/java/org/apache/servicecomb/serviceregistry/ServiceRegistry.java

+
+  /**
+   * @see #registryMicroserviceMapping(String, String, Class, List)
+   * @param endpoints the endpoints of 3rd party service. Each of endpoints will be treated as a separated instance.


add example for endpoint?

Yes, the example is added in the comment of method registerMicroserviceMappingByEndpoints

wujimin · 2018-09-19T14:28:50Z

...y/src/main/java/org/apache/servicecomb/serviceregistry/registry/AbstractServiceRegistry.java

+            .computeIfAbsent(microserviceName,
+                svcName -> new StaticMicroserviceVersions(this.appManager, app, microserviceName, schemaIntfCls));
+
+    microserviceVersions.addInstances(version, instances);


addInstances should put inside computeIfAbsent?

addInstances is put inside computeIfAbsent. Currently only initial instance method is provided. Users cannot re-set instance info of a 3rd party service.

wujimin · 2018-09-19T17:51:17Z

...umer/src/main/java/org/apache/servicecomb/it/testcase/thirdparty/Test3rdPartyInvocation.java

+    Assert.assertEquals("abc", stringBodyResponse.getBody());
+  }
+
+  @Path("/rest/it-producer/v1/dataTypeJaxrs")


for different deploy mode, target microservice name is different.
so "it-producer" can not hard code
based on zhengyangyong's PR, if that one merged first, then this one will failed.

I've checked zhengyangyong's commits and test it by rebasing those commits into my branch. There is no problem with the producer's service name because other deploy mode will not trigger this test.
The only problem is the initialization method of GateRestTemplate is changed. And I've changed to use new initialization method in new commit.

wujimin · 2018-09-19T17:54:18Z

core/src/main/java/org/apache/servicecomb/core/CseContext.java

@@ -40,6 +41,8 @@ public static CseContext getInstance() {

  private ConsumerSchemaFactory consumerSchemaFactory;

+  private StaticSchemaFactory staticSchemaFactory;


CseContext will de Deprecated, use ScbEngine

wujimin · 2018-09-19T17:56:26Z

core/src/main/java/org/apache/servicecomb/core/definition/schema/AbstractSchemaFactory.java

@@ -103,4 +109,12 @@ protected SwaggerGenerator generateSwagger(CONTEXT context) {

    return generator;
  }
+
+  protected String getSwaggerContent(Swagger swagger) {


can we just use SwaggerUtils?

liubao68 · 2018-09-20T03:41:16Z

...umer/src/main/java/org/apache/servicecomb/it/testcase/thirdparty/Test3rdPartyInvocation.java

+
+    MicroserviceInstance instance = new MicroserviceInstance();
+    instance.setEndpoints(Collections.singletonList(endpoint));
+    RegistryUtils.getServiceRegistry()


There are two scenarios need to consider:

consumers register producers endpoints and schemas

consumers register only schemas, this is quite common in mesher mode which the service is already registered to service center but schemas are not, and used very widely

Can we provide scenario 2 support?

I've asked wujimin's opinion and maybe we can have a new discussion on the scenario that there are microservices registered in sc with only instance info but no schema.
This issue is planned to provide the basic ability to invoke 3rd party service.

liubao68 · 2018-09-26T12:12:16Z

core/src/main/java/org/apache/servicecomb/core/definition/schema/AbstractSchemaFactory.java

@@ -45,6 +48,8 @@
  @Inject
  protected CompositeSwaggerGeneratorContext compositeSwaggerGeneratorContext;

+  private ObjectWriter writer = Yaml.pretty();


unused code?

Yes, has been fixed.

liubao68 · 2018-09-26T12:40:47Z

core/src/main/java/org/apache/servicecomb/core/definition/MicroserviceVersionMeta.java

-  private MicroserviceMeta microserviceMeta;
+  MicroserviceMeta microserviceMeta;
+
+  MicroserviceVersionMeta(Microservice microservice) {


microserviceMeta is not initialized, and getMicroserviceMeta may return null and cause NPE.
From the following code, I think it's better to define the method protected that should be overrided by sub class.
Or remove this constructor and define microserviceMeta to be protected.

microserviceMeta in MicroserviceVersionMeta is initialized in constructors and has package access modifier which ensures that the classes out of its package can only get access to it by getter method. So I think NPE can be avoid since users can only get microserviceMeta after it is initialized.

liubao68 · 2018-09-26T12:51:41Z

service-registry/src/main/java/org/apache/servicecomb/serviceregistry/ServiceRegistry.java

+   * @param instances the instances of this 3rd party service. Users only need to specify the endpoint information, other
+   * necessary information will be generate and set in the implementation of this method.
+   */
+  void registerMicroserviceMapping(String microserviceName, String version, Class<?> schemaIntfCls,


It's better to put schemaIntfCls as last parameter, since we can extend it to schemaIntfCls... to support several schemas. I think this is quite normal in many scenarios like register a gateway service as a third party service.

Yes, I think we should consider multiple schemas in the next step.

…hema

…e, move StaticSchemaFactory into SCBEngine

wujimin reviewed Sep 19, 2018

View reviewed changes

liubao68 reviewed Sep 20, 2018

View reviewed changes

yhs0092 force-pushed the 3rd_party_invocation branch from f09fd7e to 9c233d6 Compare September 25, 2018 08:14

yhs0092 closed this Sep 26, 2018

yhs0092 reopened this Sep 26, 2018

liubao68 reviewed Sep 26, 2018

View reviewed changes

yhs0092 force-pushed the 3rd_party_invocation branch from 9c233d6 to 49bd8bf Compare September 26, 2018 12:56

yhs0092 mentioned this pull request Sep 26, 2018

[SCB-926] 3rd party invoke #926

Closed

6 tasks

wujimin approved these changes Sep 27, 2018

View reviewed changes

yhs0092 added 10 commits September 27, 2018 09:25

[SCB-926] fix AccessLogHandlerTest random failure

00a235a

[SCB-926] add entry point of adding 3rd party service instances

ff287ce

[SCB-926] implement StaticMicroserviceVersions

345626c

[SCB-926] define and load StaticMicroserviceVersionFactory

1357f10

[SCB-926] add StaticMicroserviceVersionMeta, generate and register sc…

ad9b12a

…hema

[SCB-926] support reactive interface invoke

7ac5e08

[SCB-926] add IT for 3rd party service invocation

6efa512

[SCB-926] only allow initialize 3rd party services' instances for onc…

c71cd34

…e, move StaticSchemaFactory into SCBEngine

[SCB-926] reduce CI log

60816c8

[SCB-926] modify method signature

be9f265

yhs0092 force-pushed the 3rd_party_invocation branch from 74f1fbe to be9f265 Compare September 27, 2018 01:55

liubao68 merged commit f304598 into apache:master Sep 28, 2018

yhs0092 deleted the 3rd_party_invocation branch December 14, 2019 02:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SCB-926] support invoking 3rd party service #915

[SCB-926] support invoking 3rd party service #915

yhs0092 commented Sep 19, 2018

coveralls commented Sep 19, 2018

wujimin Sep 19, 2018

yhs0092 Sep 24, 2018

wujimin Sep 19, 2018

yhs0092 Sep 24, 2018

wujimin Sep 19, 2018

yhs0092 Sep 24, 2018

wujimin Sep 19, 2018

yhs0092 Sep 24, 2018

wujimin Sep 19, 2018

yhs0092 Sep 24, 2018

liubao68 Sep 20, 2018 •

edited

yhs0092 Sep 24, 2018

liubao68 Sep 26, 2018

yhs0092 Sep 26, 2018

liubao68 Sep 26, 2018 •

edited

yhs0092 Sep 26, 2018

liubao68 Sep 26, 2018

yhs0092 Sep 26, 2018

yhs0092 Sep 26, 2018

		@@ -40,6 +41,8 @@ public static CseContext getInstance() {

		private ConsumerSchemaFactory consumerSchemaFactory;

		private StaticSchemaFactory staticSchemaFactory;

[SCB-926] support invoking 3rd party service #915

[SCB-926] support invoking 3rd party service #915

Conversation

yhs0092 commented Sep 19, 2018

coveralls commented Sep 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liubao68 Sep 20, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liubao68 Sep 26, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liubao68 Sep 20, 2018 •

edited

liubao68 Sep 26, 2018 •

edited