Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add best practices for bottom up approach #624

Merged
merged 7 commits into from
Mar 9, 2018
Merged

Add best practices for bottom up approach #624

merged 7 commits into from
Mar 9, 2018

Conversation

shimks
Copy link
Contributor

@shimks shimks commented Feb 17, 2018

@virkt25
Copy link
Contributor

virkt25 commented Feb 20, 2018

Haven't reviewed the entire PR but a high level question ...

Do people normally understand what is meant by top-down and bottom-up approach? It personally took me a while to remember which approach is which and even now at times I have to think about it. I think we need better names (but don't know any off the top of my head).

@virkt25
Copy link
Contributor

virkt25 commented Feb 20, 2018

Whoops. Accidentally hit Close and comment.

@shimks
Copy link
Contributor Author

shimks commented Feb 20, 2018

I think it's ok to keep the names, but I agree on the fact that those names need clarification on what we mean by them.

React seems to be using the terms (https://reactjs.org/docs/thinking-in-react.html#step-2-build-a-static-version-in-react), but not all that often. I'd like to hear other people's thoughts on this

Copy link

@kjdelisle kjdelisle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly good, just a few typos and some areas of confusion that need tidying up.

4. [**Implementing features**](./Implementing-features.html): This section demonstrates how the tests for each feature of your application should be written, and how to write the logic to make these tests pass. In the example, the tests for the controller, model, repository, data source, and sequence are written and then implemented.
5. [**Preparing the API for consumption**](./Preparing-the-API-for-consumption.html): This section shows how the endpoints can be physically tested using the Swagger UI.
1. **Defining the API**: There are two possible approaches to take in this section
- [**Defining the API using bottom-up appraoch**](./Defining-the-API-using-bottom-up-approach.html): This section guides you through setting up a skeleton of your application so that a full API can be automatically generated.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Typo in approach
  • Not sure what you mean by so that a full API can be automatically generated; the bottom-up approach is one where you're building the controllers, functions, routes and so on that make up the bulk of your API. The portion that's generated is the OpenAPI/Swagger specification that describes it.

5. [**Preparing the API for consumption**](./Preparing-the-API-for-consumption.html): This section shows how the endpoints can be physically tested using the Swagger UI.
1. **Defining the API**: There are two possible approaches to take in this section
- [**Defining the API using bottom-up appraoch**](./Defining-the-API-using-bottom-up-approach.html): This section guides you through setting up a skeleton of your application so that a full API can be automatically generated.
- [**Defining the API using top-down approach**](./Defining-the-API-using-top-down-approach.html): This section guides you through constructing your API first before any internal logic is added.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow the convention established by @b-admike 's PRs and mark this with a warning or indicator that says this approach is not yet functional/viable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the warning on the page is sufficient, since they will click the links and be redirected there. But if we can do it here as well, then I'm fine with it.


Your models will be used to provide schemas to validate against the data
your API will intercept and consequently are likely to be referenced as
argument types in your controllers, so they should be the first to be defined.
Copy link

@kjdelisle kjdelisle Feb 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Your models are not necessarily going to be used to validate anything. I can easily write my controller functions to accept objects that are neither strictly nor logically equivalent to my model objects.
  • The value of using models is that they give you a common, workable definition for:
    • representing inbound and outbound data at the API layer
    • handling data and relationships at the datasource layer
  • Intercept carries a very particular connotation; that the intended recipient is not the first to receive or act upon the incoming payload. In our case, we're not intercepting the request, we're handling it.

```

This model is just a TypeScript class without any decorators in its current
form, so features such as automatic schema generation are not available to it

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just filler. It's more than adequate to say that this is just a plain TypeScript class.

form, so features such as automatic schema generation are not available to it
yet.

Now decorate the class with `@model` and `@property`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explain what these decorators accomplish in the same sentence/paragraph here.

We can use the @model and @property decorators to create _metadata_ that
represents the model; a model definition. LoopBack and LoopBack extensions
can use this model definition for a wide variety of uses, such as:
- generating OpenAPI schema for your APIs
- validating instances of the models during the request/response lifecycle
- automatically inferring relationships between models during datasource
   operations

To apply these decorators to your model, you simply prefix the class definition
with the @model decorator, and prefix each property with the @property decorator:

The controller's routes in their current state has no information on which
API endpoints they belong to. Add them in by using `@operation` and `@param`
decorators.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a tag here to indicate the name of this example file.
{% include code-caption.html content="src/todo.controller.ts" %}

for each categories of your API.

```ts
class TodoController {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export

decorators.

```ts
class TodoController {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export

its API; when an instance of `RestServer` is run, an OpenAPI specification
representing your application's API is built. The spec is generated
entirely from the decorated elements' metadata, which in turn provides
routing logic for your API when your application is running.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -9,7 +9,7 @@ summary:
---

{% include previous.html content="
This article continues off from [Defining and validating the API](./Defining-and-validating-the-API.html).
This article continues off from [Defining the API using top-down approach](./Defining-the-API-using-top-down-approach.html).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continues from

5. [**Preparing the API for consumption**](./Preparing-the-API-for-consumption.html): This section shows how the endpoints can be physically tested using the Swagger UI.
1. **Defining the API**: There are two possible approaches to take in this section
- [**Defining the API using bottom-up appraoch**](./Defining-the-API-using-bottom-up-approach.html): This section guides you through setting up a skeleton of your application so that a full API can be automatically generated.
- [**Defining the API using top-down approach**](./Defining-the-API-using-top-down-approach.html): This section guides you through constructing your API first before any internal logic is added.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the warning on the page is sufficient, since they will click the links and be redirected there. But if we can do it here as well, then I'm fine with it.

" %}

Once your models are defined, create a controller to host your routes
for each categories of your API.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say for each [Paths object](https://swagger.io/specification/#pathsObject) of your API specification here.

class TodoController {
constructor() {}

@post('/todo') // sugar for @operation('post', '/todo');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if people would know what sugar means, so I suggest you rephrase as same as @operation('post', '/todo');

### Reviewing your API specification

To review your complete API specification, run your application with the
decorated controllers registered. Once it is running, visit `/swagger.json`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can also go to openapi.json to get OpenAPI 3.0 spec and .yaml for both to get them in yaml flavour.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to leave it at just swagger for now since we don't support OpenAPI v3 yet, but I'll add in short sentence saying that yaml flavour is also available. I'll leave it up to Janny to fix the swagger to openapi related doc switch-overs when she's doing her docs.


Sometimes, it's hard to know what your API is going to look like without
writing out your application first. It also might be difficult to adjust your
API as plans change or as features need to be reworked.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, the reasoning for bottom-up is misleading here. Please rewrite it to align with some ideas below:

  1. Designing APIs in vacuum is not only difficult but also not wise if you don't have a good understanding of what your existing system can offer and what your clients need.

  2. Bottom-up approach allows developers to capture existing business domain models and corresponding operations and reflect them for external consumption as APIs. Such information can be defined as metadata with code, JSON/YAML documents, or decorations.

  3. Bottom-up approach better aligns APIs with implementations from the beginning and evolve them all together as API consumers and providers reach agreements incrementally.

  4. Bottom-up approach makes it possible to have APIs up and running for play and polish at very early stages, which is critical to the success of APIs as stakeholders can see the big picture collaboratively before it's too late.

@shimks shimks force-pushed the bottom-up-doc branch 2 times, most recently from 21bb5b1 to 9ed1d01 Compare February 21, 2018 21:44
easily envision the big picture of the end product.

There are various tools available to LoopBack which allows this bottom-up
approach of building your application simple through the usages of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simple -> simply

@b-admike
Copy link
Contributor

In terms of what to name the sections, how about code first application design vs API first application design or something along those lines?

@bajtos
Copy link
Member

bajtos commented Feb 26, 2018

The new content in Defining-the-API-using-code-first-approach.md does not say anything about testing your new application. Originally, Thinking in LoopBack was heavily focused on showing people how to write application in test-first/test-driven style. It would be great to preserve this spirit for code-first approach too.

@shimks
Copy link
Contributor Author

shimks commented Feb 28, 2018

@bajtos Could you be more specific as to what you're envisioning? The code in Defining-the-API-using-code-first-approach.md contains no logic, so do you mean that I should show a skeleton of the endpoints that I need to test out for the controller?

@@ -103,13 +103,18 @@ children:
output: 'web, pdf'
children:

- title: 'Defining and validating the API'
url: Defining-and-validating-the-API.html
- title: 'Defining the API using code-first approach'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defining API's using code-first approach might be better. Defining the API ... seems odd.

Copy link
Contributor Author

@shimks shimks Mar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. As a helpful comment though, never try not to use 's to make something plural :p, even for acronyms (unless it ends with an S, then I have no clue lol).

output: 'web, pdf'

- title: 'Testing the API'
url: Testing-the-API.html
- title: 'Defining the API using API-first approach'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

output: 'web, pdf'
children:

- title: 'Testing the API'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing API's ... ?

1. **Defining the API**: There are two possible approaches to take in this section
- [**Defining the API using code-first approach**](./Defining-the-API-using-code-first-approach.html): This section guides you through setting up a skeleton of your application so that its full OpenAPI specification can be automatically generated.
- [**Defining the API using API-first approach**](./Defining-the-API-using-API-first-approach.html): This section guides you through constructing your API first before any internal logic is added. __*Not fully supported*__
- [**Testing the API**](./Testing-the-API.html): This section describes the process of writing smoke test for your API and its spec. __*Not fully supported*__
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be under Defining your testing strategy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back when I worked on breaking up the Thinking in LoopBack page, the rough structure of the page went from:

  1. Define your API
  2. Test your API
  3. Summary of TTD
  4. Writing your code using TTD

To me testing the app is different from testing the API (spec), and it makes sense to me that when you're using the build-your-API-spec-first approach you would go about testing the API (in the form of openapi spec) first before you test anything that has to do with the application.

@bajtos
Copy link
Member

bajtos commented Mar 1, 2018

Could you be more specific as to what you're envisioning? The code in Defining-the-API-using-code-first-approach.md contains no logic, so do you mean that I should show a skeleton of the endpoints that I need to test out for the controller?

Right now, your narrative is "Define your models", "Define your routes", "Reviewing your API specification". That's not how test-first people work! Take a look at Incrementally implement features to see test-first approach in work for top-down approach.

For the documention of code-first approach, I would like to see a similar content/narrative:

  1. Pick one (and only one!) endpoint you are going to implement.
  2. Start with an acceptance test describing endpoint's behaviour (see Write an acceptance test). This acceptance test is going to server as an indicator telling you when you are done with this endpoint.
  3. Write a unit-test for the new controller method you are going to implement (see Write a unit-test for the new controller method, this section contains an example of the actual controller implementation at the bottom.)
  4. Write the first implementation of the controller method to make the test pass. This is time to put code-first approach in work, e.g. use REST decorators like @get and @param.
  5. Write an integration test to verify that the controller is using data from the database (see Write an integration test for the new controller method
  6. Now comes the best time for defining models & repositories - see Define Product model, repository, and data source. Because you are describing bottom-up approach, I think it's acceptable to assume that our data/repository/model layer is already set up and/or was configured before we started writing controllers for the REST API. This configuration should still be implemented in test-first way though!
  7. Update test helpers and the controller use real model and repository
  8. Run the tests again and fix the issues - see Run tests
  9. Decouple controller from Repository
  10. Handle 'product not found' error

Basically I am asking you to create a guide that's similar to what we have for bottom-down approach, but with specific details changed to accommodate the bottom-up (code-first) approach.

If there are sections that can or should be reused by both guides, then it may be worth extracting them into shared content. (I don't know how easy it is, or whether it's feasible at all.)

For example, the last step in the api-first guide, Implement a custom sequence, should be pretty easy to extract into a standalone document that both code-first and api-first guide can refer to.

I am not sure what's the exact scope of your task (the user story you are working on). If a guide as I described above is beyond the scope, then I can live with the shorter version you proposed here and will open a new issue to bring the code-first guide up to the level of the api-first guide. Please let me know!

@shimks
Copy link
Contributor Author

shimks commented Mar 1, 2018

Thanks for a very detailed narrative @bajtos ! Considering that @kjdelisle is working on the tutorial story (loopbackio/loopback-next#729), I personally thought the story I'm working on (loopbackio/loopback-next#735) would serve to merely accompany the Defining and validating the API page with the bottom-up approach. It seems like the narrative you proposed requires the docs I'm writing to be essentially an extension piece of example-getting-started that shows how the tutorial is written the test-first approach.

Right now I'm thinking that I rewrite this section as an extension piece of the tutorial, continuing off from https://github.com/strongloop/loopback-next/blob/master/packages/example-getting-started/docs/6-repository.md. From there I'm thinking of following your proposed content structure to finish off example-getting-started application with complete tests to go with it. Does this sound good to you?

@bajtos
Copy link
Member

bajtos commented Mar 1, 2018

merely accompany the Defining and validating the API page with the bottom-up approach.

I see. I quickly checked the content in Defining and validating the API, I agree your current content is closer to that page than my long proposal, which makes more sense in the context of your task.

So maybe it's enough if you add notes to your existing content to tell people that the examples in your content are just showing how to define API and pointing them to "Incrementally implement features" for better examples showing the implementation process including test coverage?

Right now I'm thinking that I rewrite this section as an extension piece of the tutorial, continuing off from https://github.com/strongloop/loopback-next/blob/master/packages/example-getting-started/docs/6-repository.md. From there I'm thinking of following your proposed content structure to finish off example-getting-started application with complete tests to go with it. Does this sound good to you?

Sounds reasonable to me. I think it may be best to split the work into two (or more) pull requests, the first one covering "I rewrite this section as an extension piece of the tutorial", so that we can land something sooner than later?

@shimks
Copy link
Contributor Author

shimks commented Mar 1, 2018

I think it may be best to split the work into two (or more) pull requests, the first one covering "I rewrite this section as an extension piece of the tutorial", so that we can land something sooner than later?

Could you clarify how I could go about breaking up the work into multiple pull requests? Are you thinking that the first PR to be less detailed than what it should be? Or do you mean I should break up the actual contents in chunks that resembles something like this ?:
acceptance test -> unit test -> write logic -> integration test -> update tests/code -> tahdah you're done!!!

@bajtos
Copy link
Member

bajtos commented Mar 2, 2018

Could you clarify how I could go about breaking up the work into multiple pull requests? Are you thinking that the first PR to be less detailed than what it should be?

As I was reading your comment, it occurred to me that your plan seems to have two distinctive parts/tasks:

Right now I'm thinking that I rewrite this section as an extension piece of the tutorial, continuing off from https://github.com/strongloop/loopback-next/blob/master/packages/example-getting-started/docs/6-repository.md. From there I'm thinking of following your proposed content structure to finish off example-getting-started application with complete tests to go with it. Does this sound good to you?

I was thinking:

Just an idea to consider. Please organize your work and pull requests in a way that works best for you!

@bajtos
Copy link
Member

bajtos commented Mar 2, 2018

@shimks I think we should find a smaller scope for your work, considering the description in loopbackio/loopback-next#735:

Add a new page at the same level, something like "Define your API bottom-up way". Explain basic concepts (controller decorators like @post, @param etc.) and point people to example-getting-started as the tutorial.

The outline described in #624 (comment) may require more effort that we have planned for this task. Could you please talk to @kjdelisle to find a good balance between the ideal I described and the MVP/Walking Skeleton scope.

Copy link

@kjdelisle kjdelisle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all cases, we're defining an application, not an API. The API is a part of the application (since it's what consumers will use), but the phrasing on a lot of the top-down methods is wobbly if you refer to the whole app as an API.

5. [**Preparing the API for consumption**](./Preparing-the-API-for-consumption.html): This section shows how the endpoints can be physically tested using the Swagger UI.
1. **Defining the API**: There are two possible approaches to take in this section
- [**Defining the API using code-first approach**](./Defining-the-API-using-code-first-approach.html): This section guides you through setting up a skeleton of your application so that its full OpenAPI specification can be automatically generated.
- [**Defining the API using API-first approach**](./Defining-the-API-using-API-first-approach.html): This section guides you through constructing your API first before any internal logic is added. __*Not fully supported*__

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rephrase this bit to say "Defining your app using the API-first approach" or something similar.
"Defining the API using API-first approach" sounds a bit like saying "Making your app by using the app-first approach" :P

@@ -13,7 +13,7 @@ page are outdated and may not work out of the box. They will be revisited after
our MVP release.
"%}

## Define the API
## Define the API from top to bottom (API-first)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define the app from top to bottom

@shimks shimks changed the title [WIP] Add best practices for bottom up approach Add best practices for bottom up approach Mar 5, 2018
@shimks shimks force-pushed the bottom-up-doc branch 2 times, most recently from dfdf921 to 873e8d3 Compare March 5, 2018 16:31
- title: 'Testing the API'
url: Testing-the-API.html
- title: 'Defining the app using API-first approach'
url: Defining-the-app-using-API-first-approach.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@jannyHou jannyHou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

### Reviewing your API specification

To review your complete API specification, run your application with the
decorated controllers registered. Once it is running, visit `/swagger.json`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/openapi.json

Copy link
Contributor

@bschrammIBM bschrammIBM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will need to copyedit this doc after it is posted due to time constraints with Velox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants