A self-hosted API Explorer as a new extension #2014

bajtos · 2018-11-12T09:18:12Z

In this pull request, I am picking up the best parts from #1664 and #1976 and proposing an implementation of a self-hosted API explorer build on the features that are already available. The internal design is not as pretty & clean as it could be, but it works OOTB (as long as the user configures a custom url path, see below).

I expect that the internal details of the explorer component will evolve over time as we keep adding more extension points to our REST layer, but that's way beyond this pull request.

The missing piece: how to disable the built-in redirect to explorer.loopback.io. ~~I'll be opening a new pull request for that (hopefully today).~~ See feat(rest): add config option to disable API Explorer redirects #2016.

Close #1664, close #1976, close #1960.

This pull request is one of the tasks for [API Explorer] Self-hosted API Explorer #559.

@strongloop/loopback-maintainers please review

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

bajtos

The pull request contains three commits to make the review easier:

The first one adds an empty "explorer" package
The second implements self-hosted API explorer
The third one enables self-hosted API explorer in example projects

I'll work on the documentation and CLI templates once the general direction is approved.

packages/explorer/README.md

packages/explorer/docs.json

packages/explorer/src/explorer.component.ts

packages/explorer/src/explorer.controller.ts

bajtos · 2018-11-12T09:30:28Z

@strongloop/loopback-maintainers How should we name the extension providing self-hosted API Explorer?

Currently, I am using @loopback/explorer, even though the component is using "ApiExplorer" name everywhere. Should we rename it to @loopback/api-explorer?

Also, do we want to start some kind of convention for LB extensions? In LB 3.x, extension packages have names starting with loopback-component-, e.g. loopback-component-explorer. Should we do the same in LB4 and call the new package @loopback/extension-api-explorer? Personally, I find such name awfully long and prefer shorter @loopback/api-explorer.

When deciding between explorer and api-explorer (or even something else): Are we expecting other kinds of explorers to be available in LB4 apps in the future? I believe GraphQL has an interactive explorer/client. I think we should use a naming scheme that will make it clear which extension is exploring which protocol. Both explorer and api-explorer seem too generic to me, I am proposing rest-explorer (to go with @loopback/rest component) or perhaps openapi-explorer.

Thoughts?

raymondfeng · 2018-11-12T15:38:46Z

@bajtos CI is failing.

bajtos · 2018-11-12T15:48:17Z

CI is failing.

I know, this is waiting for feat(rest): add config option to disable API Explorer redirects #2016 to be landed first.

raymondfeng · 2018-11-12T16:01:21Z

The proposal combines the ability of controllers and static assets to serve API explorer with server-side templating. I think it's acceptable first step. LGTM.

jannyHou

Mostly LGTM, left a few nitpicks. I am good with the name @loopback/rest-explorer.

jannyHou · 2018-11-12T18:50:48Z

packages/explorer/test/acceptance/explorer.acceptance.ts

+
+      const response = await request.get('/explorer/').expect(200);
+      const body = response.body;
+      expect(body).to.match(/^\s*url: '\/apispec',\s*$/m);


there are 3 endpoint mappings above, do we want to assert the other 2 as well?

This test is verifying that we are picking the correct endpoints (OpenAPI v3 in JSON format). This test is configuring other mappings to verify that the algorithm picks the right mapping..

@jannyHou How can I improve the test to make this more clear to readers?

packages/explorer/src/explorer.controller.ts

packages/explorer/test/acceptance/explorer.acceptance.ts

packages/explorer/src/explorer.controller.ts

packages/explorer/src/explorer.keys.ts

marioestradarosa · 2018-11-12T23:27:39Z

How about rest-api-explorer ? (too long?), but rest-explorer LGTM also.

dhmlau · 2018-11-13T03:00:53Z

Just my two cents.. Between rest-explorer and openapi-explorer, I'd prefer rest-explorer because it's clearer that it's for REST. But no strong preference for either way.

bajtos · 2018-11-13T09:29:59Z

Re: package name - thank you @jannyHou @marioestradarosa @dhmlau for the feedback. I am going to use @loopback/rest-explorer going forward.

bajtos · 2018-11-13T12:00:16Z

I have rebased the patch on the latest master and addressed all remaining issues & todos. The pull request is ready for final review. I'll squash the commit history before landing.

To make the review easier, I left the first three commits unchanged and added a bunch of new small commits (one for each logical step, kind of):
chore: rename explorer to rest-explorer

fix(rest): allow users to disable Explorer redirects after RestServer…
fix: disable built-in swagger-ui redirects when a self-hosted compone…
docs: describe rest-explorer component
fix: apidocs for rest-explorer
docs: add a mention of hard-coded visual style to README
feat(cli): enable self-hosted REST API explorer

@raymondfeng @jannyHou @marioestradarosa LGTY?

bajtos · 2018-11-13T12:01:20Z

The documentation changes require us to land loopbackio/loopback.io#767, I think that other PR should be landed first.

bajtos · 2018-11-13T12:43:16Z

The documentation changes require us to land loopbackio/loopback.io#767, I think that other PR should be landed first.

Actually, I found a better solution - see #2025

bajtos · 2018-11-13T12:46:48Z

continuous-integration/travis-ci/pr — The Travis CI build failed

✖   scope must be one of [
    benchmark, docs, example-hello-world, example-log-extension, example-rpc-server, 
    example-soap-calculator, example-todo-list, example-todo, authentication, boot, build, 
    cli, context, core, http-caching-proxy, http-server, metadata, openapi-spec-builder,
    openapi-v3-types, openapi-v3, repository-json-schema, repository, rest-explorer, rest, 
    service-proxy, testlab, sandbox-example
] [scope-enum]
✖   found 1 problems, 0 warnings

Please ignore this build failure. It's just the commit linter using a wrong version of package list to verify scope. This error will get fixed when I clean up the commit history.

bajtos · 2018-11-13T12:49:34Z

Liquid Exception: Could not locate the included file
  'readmes/loopback-next/packages/rest-explorer/README.md' in any of
  ["/home/travis/build/strongloop/loopback-next/sandbox/loopback.io/pages/en/lb4"].
  Ensure it exists in one of those directories and, if it is a symlink, does not point 
  outside your site source. in /_layouts/readme.html

The failure of verify-docs build step will be fixed once #2025 is landed and this pull request is rebased on the new master.

b-admike

Nice work! I've left some comments, but LGTM overall.

packages/explorer/test/acceptance/explorer.acceptance.ts

packages/cli/generators/app/templates/src/application.ts.ejs

b-admike · 2018-11-13T16:11:09Z

packages/rest/src/rest.server.ts

-      debug('Redirect to swagger-ui was disabled by configuration.');
-      return;
-    }
-
    const explorerPaths = ['/swagger-ui', '/explorer'];


Should explorerPaths take into account custom paths the user configures with our RestExplorer component?

I am not opposed to that, but I think it's out of scope of this pull request.

packages/rest-explorer/README.md

b-admike · 2018-11-13T16:15:58Z

docs/site/Self-hosted-REST-API-Explorer.md

+summary:
+  A tutorial on using `@loopback/rest-explorer` to add a self-hosted REST API
+  Explorer
+---


Are you planning to add docs to this page in this PR?

This page will automatically include the contents of packages/rest-explorer/README.md, see https://loopback.io/doc/en/lb4/Creating-decorators.html as an example of a page based on README contents.

docs/site/Server.md

… was created Before this change, the built-in Explorer redirect could be disabled only via the config object passed to RestServer. Once a RestServer was instantiated, changes to the configuration were no longer honored. This commit moves the detection of `apiExplorer.disabled` to the Express routes handling redirects.

Add a new package @loopback/rest-explorer that provides a self-hosted swagger-ui instance and disables the built-in swagger-ui redirect. Modify example projects to use @loopback/rest-explorer. Modify the CLI template to include self-hosted REST API Explorer in new applications.

cloudwheels · 2018-11-16T18:30:48Z

@bajtos when will this extension be released? Thanks

bajtos · 2018-11-26T15:49:54Z

I believe @loopback/[email protected] has been already published on November 17th, see e0fe37d

bajtos added this to the November 2018 Milestone milestone Nov 12, 2018

bajtos self-assigned this Nov 12, 2018

bajtos requested review from raymondfeng and a team November 12, 2018 09:18

bajtos requested review from hacksparrow and marioestradarosa as code owners November 12, 2018 09:18

bajtos commented Nov 12, 2018

View reviewed changes

This was referenced Nov 12, 2018

[WIP]feat: add local API Explorer #1976

Closed

[WIP][RFC] Add local swagger ui for LoopBack 4 API explorer #1664

Closed

jannyHou reviewed Nov 12, 2018

View reviewed changes

bajtos mentioned this pull request Nov 13, 2018

Sugar API for defining URL redirects #2022

Open

6 tasks

This was referenced Nov 13, 2018

Custom visual style of REST API Explorer #2023

Closed

Add rest-explorer to v4 readmes. loopbackio/loopback.io#767

Closed

bajtos force-pushed the feat/explorer-extension branch from 968d42e to b090e07 Compare November 13, 2018 11:56

bajtos requested a review from jannyHou November 13, 2018 12:00

bajtos mentioned this pull request Nov 13, 2018

build(docs): include READMEs from all packages at publish time #2025

Merged

2 tasks

b-admike reviewed Nov 13, 2018

View reviewed changes

raymondfeng reviewed Nov 14, 2018

View reviewed changes

docs/site/Server.md Outdated Show resolved Hide resolved

raymondfeng reviewed Nov 14, 2018

View reviewed changes

docs/site/Server.md Show resolved Hide resolved

bajtos force-pushed the feat/explorer-extension branch from b090e07 to 9ed2384 Compare November 15, 2018 08:20

bajtos force-pushed the feat/explorer-extension branch from 9ed2384 to e40b653 Compare November 15, 2018 08:26

bajtos merged commit 4c165c7 into master Nov 15, 2018

bajtos deleted the feat/explorer-extension branch November 15, 2018 11:57

dhmlau mentioned this pull request Nov 20, 2018

[Roadmap] 🚀1Q2019 🚀 #1839

Closed

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A self-hosted API Explorer as a new extension #2014

A self-hosted API Explorer as a new extension #2014

bajtos commented Nov 12, 2018 •

edited

Loading

bajtos left a comment

bajtos commented Nov 12, 2018

raymondfeng commented Nov 12, 2018

bajtos commented Nov 12, 2018

raymondfeng commented Nov 12, 2018

jannyHou left a comment

jannyHou Nov 12, 2018

bajtos Nov 13, 2018

marioestradarosa commented Nov 12, 2018

dhmlau commented Nov 13, 2018

bajtos commented Nov 13, 2018

bajtos commented Nov 13, 2018

bajtos commented Nov 13, 2018

bajtos commented Nov 13, 2018

bajtos commented Nov 13, 2018

bajtos commented Nov 13, 2018

b-admike left a comment

b-admike Nov 13, 2018

bajtos Nov 14, 2018

b-admike Nov 13, 2018

bajtos Nov 14, 2018

cloudwheels commented Nov 16, 2018

bajtos commented Nov 26, 2018

A self-hosted API Explorer as a new extension #2014

A self-hosted API Explorer as a new extension #2014

Conversation

bajtos commented Nov 12, 2018 • edited Loading

Checklist

bajtos left a comment

Choose a reason for hiding this comment

bajtos commented Nov 12, 2018

raymondfeng commented Nov 12, 2018

bajtos commented Nov 12, 2018

raymondfeng commented Nov 12, 2018

jannyHou left a comment

Choose a reason for hiding this comment

jannyHou Nov 12, 2018

Choose a reason for hiding this comment

bajtos Nov 13, 2018

Choose a reason for hiding this comment

marioestradarosa commented Nov 12, 2018

dhmlau commented Nov 13, 2018

bajtos commented Nov 13, 2018

bajtos commented Nov 13, 2018

bajtos commented Nov 13, 2018

bajtos commented Nov 13, 2018

bajtos commented Nov 13, 2018

bajtos commented Nov 13, 2018

b-admike left a comment

Choose a reason for hiding this comment

b-admike Nov 13, 2018

Choose a reason for hiding this comment

bajtos Nov 14, 2018

Choose a reason for hiding this comment

b-admike Nov 13, 2018

Choose a reason for hiding this comment

bajtos Nov 14, 2018

Choose a reason for hiding this comment

cloudwheels commented Nov 16, 2018

bajtos commented Nov 26, 2018

bajtos commented Nov 12, 2018 •

edited

Loading