docs: add geo-coding service to Todo tutorial #1441

bajtos · 2018-06-19T11:06:15Z

Update Todo tutorial - describe how to add a Geocoder service.

Checklist

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

raymondfeng · 2018-06-19T15:22:46Z

docs/site/todo-tutorial-geocoding-service.md

+  geocode(address: string): Promise<GeoPoint[]>;
+}
+
+export class GeocoderServiceProvider implements Provider<GeocoderService> {


I wonder why we don't use @serviceProxy - http://loopback.io/doc/en/lb4/Calling-other-APIs-and-web-services.html#declare-service-proxies-for-your-controller.

At the moment, services are backed/configured by datasources. If we want to promote service as the 1st class artifact, we probably should evaluate the possibility of dividing datasource into two types:

datasource

service

WDYT?

When I started to look into testing services, I found @serviceProxy to make testing difficult, because tests cannot use the same code path for obtaining the service proxy instance as the production code uses.

See this integration test for an example:

https://github.com/strongloop/loopback-next/blob/9d82c9e67faba57a7d274f1231c2d11548dd7629/examples/todo/test/integration/services/geocoder.service.integration.ts#L37-L41

I am of the opinion that integration tests should not be using Application, Context and Dependency Injection, instead they should instantiate all classes directly from code. This makes refactorings easier, because all dependencies and couplings are explicitly expressed in code and IDE tooling can update them accordingly. (Compare that to acceptance tests where a lot of configuration is based on convention and string names.)

If we did not need to tweak datasource settings to add caching proxy, the code would become even more simple:

const dataSource = new GeocoderDataSource(); service = new GeocoderServiceProvider(dataSource).value();

Originally, before support for booting datasources was added, this used to be a one-liner. I think we may be able to get there using default arguments in the current design too.

service = new GeocoderServiceProvider().value();

At the moment, services are backed/configured by datasources. If we want to promote service as the 1st class artifact, we probably should evaluate the possibility of dividing datasource into two types:

datasource

service

I am pretty happy about the current design, to be honest.

A datasource is a long-living singleton-like object that manages state shared by all incoming HTTP requests - PersitedModel definitions, connection pool, etc.

A Repository or a Service is a lightweight cheap-to-create facade exposing user-friendly API to controllers, leveraging datasources to do the heavy lifting.

In my mind, a Service (or a Service Proxy) is similar to a Repository.

@raymondfeng thoughts?

I am going to land this pull request. If there are any changes identified in the outcome of this discussion, then I'll open a new pull request to address them.

Such changes are likely to affect the following places:

examples/todo implementation

"Calling other services" docs

"Testing your application" docs

shimks

I wonder what approach we should consider with the 'bonus' service proxy tutorial. I think the service proxy tutorial should live in its own sidebar entry under something like 'advanced tutorial' so that it can live alongside with future tutorials on features such as relations. Thoughts?

shimks · 2018-06-19T18:46:56Z

docs/site/todo-tutorial-geocoding-service.md

+
+To call
+[other APIs and web services](./Calling-other-APIs-and-web-services.html) from
+LoopBack applications, we recommend to use Service Proxies as a design pattern


What is a Service Proxy? I think a link to its definition (either our own or external) would be helpful here

Good point. The concept of a Service Proxy is described in Calling-other-APIs-and-web-services.html. I think it should have it's own page in http://loopback.io/doc/en/lb4/Concepts.html, but that's out of scope of this pull request.

I have described Service Proxy in this very same paragraph:

a design pattern
for encapsulating low-level implementation details of communication with
3rd-party services and providing JavaScript/TypeScript API that's easy to
consume e.g. from Controllers.

I'll reword this paragraph to make it (hopefully) easier to understand.

shimks · 2018-06-19T18:49:59Z

docs/site/todo-tutorial-geocoding-service.md

+$ lb4 datasource
+? Datasource name: geocoder
+? Select the connector for geocoder: REST services (supported by StrongLoop)
+? Base URL for the REST service:


If these prompts were defaulted, I think we should still add in the default choices after the colons

The default choice is an empty string.

shimks · 2018-06-19T18:51:18Z

docs/site/todo-tutorial-geocoding-service.md

+endpoints.
+
+#### /src/datasources/geocoder.datasource.json
+


I think a link to a page explaning what the settings actually mean in context to the connector would be great here

Good point, will add.

shimks · 2018-06-19T18:53:35Z

docs/site/todo-tutorial-geocoding-service.md

+
+### Implement a service provider
+
+Create a new directory `src/services` and add the following two new files:


What do these new files do?

Addressed in be80a19

shimks · 2018-06-19T18:56:24Z

docs/site/todo-tutorial-geocoding-service.md

+
+### Register the service for dependency injection
+
+Because `@loopback/boot` does not support loading of services yet (see


Do we want a place holder for this section until the boot feature can be completed?

What do you mean by a placeholder? IMO, the content of the tutorial should describe steps that people can do right now (as of DP3).

The pull request adding support for automatic loading of services should update examples/todo and this tutorial section to match the new simplified approach.

My bad, I think I had this mistaken with something else :p

shimks · 2018-06-19T18:57:01Z

docs/site/todo-tutorial-geocoding-service.md

+  }
+
+  setupServices() {
+    this.service(GeocoderServiceProvider);


We should highlight this section or at least mention it before to make it easier for the readers to identify what's been added

shimks · 2018-06-19T19:01:15Z

docs/site/todo-tutorial-putting-it-together.md


 Previous step: [Add a controller](todo-tutorial-controller.md)
+
+Next step:


Should we be adding a link to the geoservice section as a 'next step' if it's a bonus?

Sure, why not? The reader finished the core tutorial and the next step is to look at bonus sections.

bajtos · 2018-06-21T08:09:25Z

I wonder what approach we should consider with the 'bonus' service proxy tutorial. I think the service proxy tutorial should live in its own sidebar entry under something like 'advanced tutorial' so that it can live alongside with future tutorials on features such as relations. Thoughts?

In my understanding, the Todo tutorial is matching the implemention of examples/todo project in our monorepo. If we want to move service proxy tutorial to a different tutorial section, then I think the implementation in examples/todo will need rework/split too and to be honest that's a major scope creep into #1311 :(

In my mind, integrating with backend HTTP services (SOAP, REST, Swagger/OpenAPI) is one of the primary use cases for LoopBack and therefore it deserves a mention in the first tutorial we are presenting to our users.

Maybe I should reorganize the content to better integrate the new section about services into tutorial structure and flow? I choose "bonus section" approach because it seemed to be the fastest way. The task of updating the Todo tutorial was not in the original acceptance criteria of #1311, I discovered it while updating the Todo example project.

To move this pull request forward (and finish #1311), I am proposing to keep the current "bonus section" structure and defer any improvements/reorganization to a follow-up issue.

Thoughts?

bajtos · 2018-06-21T08:33:45Z

@shimks I addressed your feedback in be80a19. LGTY now?

shimks · 2018-06-21T14:37:57Z

I am proposing to keep the current "bonus section" structure and defer any improvements/reorganization to a follow-up issue.

Sounds good to me 👍

shimks

LGTM

shimks · 2018-06-21T14:39:21Z

docs/site/todo-tutorial-geocoding-service.md

+use Service Proxies as a design pattern for encapsulating low-level
+implementation details of communication with 3rd-party services and providing
+JavaScript/TypeScript API that's easy to consume e.g. from Controllers. See
+[Calling other APIs and web services](./Calling-other-APIs-and-web-services.html)


.md here so that users browsing the docs from github can go to a valid link

Good catch, fixed.

bajtos added Examples Docs labels Jun 19, 2018

bajtos added this to the June Milestone milestone Jun 19, 2018

bajtos self-assigned this Jun 19, 2018

bajtos requested review from a team and raymondfeng June 19, 2018 11:06

bajtos requested a review from virkt25 as a code owner June 19, 2018 11:06

bajtos force-pushed the docs/tutorial-with-geo-service branch from 9266b9b to 98ac0f2 Compare June 19, 2018 11:20

bajtos mentioned this pull request Jun 19, 2018

Make services easy to test + docs #1311

Closed

6 tasks

raymondfeng reviewed Jun 19, 2018

View reviewed changes

shimks reviewed Jun 19, 2018

View reviewed changes

bajtos force-pushed the docs/tutorial-with-geo-service branch from 98ac0f2 to be80a19 Compare June 21, 2018 08:33

bajtos mentioned this pull request Jun 21, 2018

docs: describe how to tests services #1445

Merged

3 tasks

shimks approved these changes Jun 21, 2018

View reviewed changes

bajtos force-pushed the docs/tutorial-with-geo-service branch from be80a19 to aa8aed3 Compare June 21, 2018 14:54

docs: add geo-coding service to Todo tutorial

e236423

bajtos force-pushed the docs/tutorial-with-geo-service branch from aa8aed3 to e236423 Compare June 22, 2018 12:13

bajtos merged commit 0d47aca into master Jun 22, 2018

bajtos deleted the docs/tutorial-with-geo-service branch June 22, 2018 12:35


		### Implement a service provider

		Create a new directory `src/services` and add the following two new files:


		### Register the service for dependency injection

		Because `@loopback/boot` does not support loading of services yet (see


		Previous step: [Add a controller](todo-tutorial-controller.md)

		Next step:

		endpoints.

		#### /src/datasources/geocoder.datasource.json

docs: add geo-coding service to Todo tutorial #1441

docs: add geo-coding service to Todo tutorial #1441

Uh oh!

Conversation

bajtos commented Jun 19, 2018

Checklist

Uh oh!

raymondfeng Jun 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shimks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bajtos commented Jun 21, 2018

Uh oh!

bajtos commented Jun 21, 2018

Uh oh!

shimks commented Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shimks left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

raymondfeng Jun 19, 2018 •

edited

Loading

shimks commented Jun 21, 2018 •

edited

Loading

shimks left a comment •

edited

Loading