feat: add integration for service-oriented backends #1119

raymondfeng · 2018-03-12T15:30:48Z

The PR adds basic integration for service oriented backends such as REST and SOAP web services using legacy connectors.

Implements #1069

Checklist

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
Related API Documentation was updated
Affected artifact templates in packages/cli were updated
Affected example projects in packages/example-* were updated

raymondfeng · 2018-04-23T15:47:17Z

#1267

bajtos · 2018-05-02T09:07:32Z

packages/service/src/loopback-datasource-juggler.ts

+  /**
+   * DataSource instance properties/operations
+   */
+  export class DataSource {


Isn't this type definition shared with @loopback/repository? Can we move commonly-used type definitions directly into loopback-datasource-juggler please?

My understanding is that we need to create index.d.ts file alongside existing index.js and place all type definitions there.

That's on my plate to move TS type definitions into loopback-datasource-juggler. Let's deal it in a separate PR.

Fair enough.

bajtos · 2018-05-02T09:08:54Z

packages/service/LICENSE

@@ -0,0 +1,25 @@
+Copyright (c) IBM Corp. 2018. All Rights Reserved.
+Node module: @loopback/service


Let's use a more descriptive name than @loopback/service please. For example, I would expect @loopback/service to provide functionality for building services.

Few proposals:

@loopback/service-client

@loopback/service-integration

@loopback/service-proxy

@loopback/service-repository

bajtos

Must have

Please update the main documentation too, see e.g. https://github.com/strongloop/loopback-next/blob/d2daf382fc38e16abbb2087354ce75b5b8efe215/docs/site/Calling-other-APIs-and-Web-Services.md

Perhaps you should move the content from README to the docs, so that README contains only a link to our docs site?

A discussion

I am confused about the invocation proxy part as I don't see how to use it together with DataSource-backed services.

Additionally, I am concerned about performance impacts of using ES6 Services in our codebase. While there have been performance improvements made towards the end of 2017 (see this blog post), these speed ups did not make it into Node.js 8.x LTS.

Additionally, Proxies still introduce performance overhead:

Converting NamedNodeMap to use Proxy increased processing time by

1.9 s on V8 6.0 (Node v8.4.0)

0.5 s on V8 6.3 (Node v9.0.0-v8-canary-20170910)

Can we remove the proxy part from this initial pull request?

bajtos · 2018-05-02T12:22:07Z

packages/service/README.md

+
+**NOTE**: Once we start to support declarative datasources with `@loopback/boot`,
+the datasource configuration files can be dropped into `src/datasources` to be
+discovered and bound automatically.


Please add a link to the GitHub issue tracking the task of adding support for data-sources to @loopback/boot.

bajtos · 2018-05-02T12:25:20Z

packages/service/README.md

+export class MyController {
+
+  @serviceProxy('geoService')
+  private geoService: GeoService;


Are all users required to write service interfaces manually? Shouldn't there be a type-unsafe interface allowing a simpler usage?

Something along the following lines:

// provided by us interface GenericService { [method: string]: Function } // usage @serviceProxy('geoService') private geoService: GenericService;

Or maybe it's our intention to force LB4 users to always write the service interface, to prevent them from being lazy and losing type safety?

bajtos · 2018-05-02T12:26:40Z

packages/service/README.md

+
+export class MyController {
+
+  @serviceProxy('geoService')


Why a proxy? Have you considered other names like @serviceClient?

bajtos · 2018-05-02T12:28:09Z

packages/service/src/decorators/service.decorator.ts

+      'datasources.' + meta.dataSourceName,
+    );
+    return getService(ds);
+  }


What if neither dataSource nor dataSourceName is set?

Please throw a descriptive error in such case.

raymondfeng · 2018-05-02T16:14:36Z

@bajtos Your comments are addressed. PTAL.

bajtos

LGTM, please consider addressing the last two comments below (Use vs. Usage, juggler namespace).

No further review is needed from my side.

bajtos · 2018-05-03T12:23:10Z

docs/site/Calling-other-APIs-and-Web-Services.md


-{% include content/tbd.html %}
+```
+$ npm install --save @loopback/service-proxy


FWIW, --save is no longer needed with the npm version shipped in Node.js 8.x LTS, at least AFAIK. If you decide to change this line, then please check other places in this pull request that are using --save argument.

bajtos · 2018-05-03T12:25:27Z

packages/service-proxy/README.md

+$ npm install --save @loopback/service-proxy
+```
+
+## Usage


This should be Basic use, see http://loopback.io/doc/en/contrib/README-guidelines.html

This section is often titled “Usage,” but we prefer “Use” because it is a shorter and more common word, and means the same thing.

bajtos · 2018-05-03T12:28:46Z

docs/site/Calling-other-APIs-and-Web-Services.md

+```ts
+import {DataSourceConstructor, juggler} from '@loopback/service-proxy';
+
+const ds: juggler.DataSource = new DataSourceConstructor({


This may be outside of this pull request, but it would be great if consumers of @loopback/service-proxy did not need to know about juggler namespace.

import {DataSourceConstructor, DataSource} from '@loopback/service-proxy'; const ds: DataSource = new DataSourceConstructor({ // ... });

Ideally, we should not need two type entities (DataSource + DataSourceConstructor) and have a single entity behaving like a proper ES6 class:

import {DataSource} from '@loopback/service-proxy'; const ds = new DataSource({ // ... });

raymondfeng · 2018-05-03T14:56:43Z

See loopbackio/loopback-datasource-juggler#1578

raymondfeng requested review from bajtos and kjdelisle as code owners March 12, 2018 15:30

raymondfeng added the needs-priority label Mar 12, 2018

raymondfeng mentioned this pull request Mar 12, 2018

docs: delete calling other apis page loopbackio/loopback.io#641

Closed

raymondfeng force-pushed the service-proxy branch from bcd2e89 to 0a6cbf1 Compare March 13, 2018 15:26

dhmlau mentioned this pull request Mar 27, 2018

Monthly Milestone - April 2018 #1044

Closed

23 tasks

raymondfeng force-pushed the service-proxy branch from 0a6cbf1 to 71af4f4 Compare April 9, 2018 15:22

raymondfeng force-pushed the service-proxy branch 2 times, most recently from 418fa32 to 5fb6604 Compare April 23, 2018 15:45

raymondfeng force-pushed the service-proxy branch 3 times, most recently from 96c5aca to 83fa5ac Compare April 27, 2018 17:39

raymondfeng force-pushed the service-proxy branch from 83fa5ac to 62dccfd Compare April 30, 2018 17:23

raymondfeng changed the title ~~[WIP - PoC] feat: add integration for service-oriented backends~~ [WIP] feat: add integration for service-oriented backends Apr 30, 2018

raymondfeng force-pushed the service-proxy branch 2 times, most recently from 25eec64 to e77f7f3 Compare April 30, 2018 21:57

raymondfeng removed the request for review from kjdelisle May 1, 2018 04:46

raymondfeng changed the title ~~[WIP] feat: add integration for service-oriented backends~~ feat: add integration for service-oriented backends May 1, 2018

bajtos reviewed May 2, 2018

View reviewed changes

bajtos suggested changes May 2, 2018

View reviewed changes

bajtos mentioned this pull request May 2, 2018

[Spike] Weakly typed integration with REST/SOAP service #1069

Closed

raymondfeng force-pushed the service-proxy branch 3 times, most recently from 637699c to ee98cf2 Compare May 2, 2018 16:14

bajtos approved these changes May 3, 2018

View reviewed changes

raymondfeng force-pushed the service-proxy branch from ee98cf2 to 57d6524 Compare May 3, 2018 14:49

raymondfeng force-pushed the service-proxy branch from 57d6524 to 95b4025 Compare May 3, 2018 15:14

feat: add integration for service-oriented backends

aacd0b8

raymondfeng force-pushed the service-proxy branch from 95b4025 to aacd0b8 Compare May 3, 2018 15:33

raymondfeng merged commit b9e2d4e into master May 3, 2018

raymondfeng deleted the service-proxy branch May 3, 2018 17:11

bajtos mentioned this pull request May 4, 2018

Integration of services with controllers (technical preview) #1267

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add integration for service-oriented backends #1119

feat: add integration for service-oriented backends #1119

raymondfeng commented Mar 12, 2018 •

edited

Loading

raymondfeng commented Apr 23, 2018

bajtos May 2, 2018

raymondfeng May 2, 2018

bajtos May 3, 2018

bajtos May 2, 2018

bajtos left a comment

bajtos May 2, 2018

bajtos May 2, 2018

bajtos May 2, 2018

bajtos May 2, 2018

raymondfeng commented May 2, 2018

bajtos left a comment

bajtos May 3, 2018

bajtos May 3, 2018

bajtos May 3, 2018

raymondfeng commented May 3, 2018

		@@ -0,0 +1,25 @@
		Copyright (c) IBM Corp. 2018. All Rights Reserved.
		Node module: @loopback/service

feat: add integration for service-oriented backends #1119

feat: add integration for service-oriented backends #1119

Conversation

raymondfeng commented Mar 12, 2018 • edited Loading

Checklist

raymondfeng commented Apr 23, 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

bajtos left a 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

raymondfeng commented May 2, 2018

bajtos left a 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

raymondfeng commented May 3, 2018

raymondfeng commented Mar 12, 2018 •

edited

Loading