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

Spike: Discover and define models at runtime #4235

Closed
wants to merge 4 commits into from

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Dec 2, 2019

This pull request implements a controller allowing clients to define models & expose their REST APIs at runtime, with no model/repository/controller source files required.

Cross-posting from #2484:

  1. Accept a MySQL connection string (host+port+database+credentials) and a list of table names.
  2. Connect to the specified MySQL database.
  3. Discover model schemas from the given tables
  4. Define LB4 models for the given tables
  5. Expose these models via REST API
  6. Return a list of URL paths of the newly added models (e.g. /Products, /CoffeeShops)

We can use the public database from loopback-getting-started) when testing this new endpoint manually.

Please note that public database is no longer available. What I did instead:

Try it out

  1. Setup the MySQL server, create some tables

  2. Checkout the feature branch spike/import-model-at-runtime

  3. npm install && npm build

  4. Open API Explorer - http://127.0.0.1:3000/explorer/#/ModelAdminController/ModelAdminController.discoverAndPublishModels

  5. Post a valid MySQL connection string and a list of existing tables, for example:

    {
      "connectionString": "mysql://root@localhost/test",
      "tableNames": [
        "Coffeeshop"
      ]
    }
  6. Reload the API Explorer to see the new endpoints for your discovered model(s)

Status

  • Proof of concept (implementation)
  • Spike document, including a list of GitHub issues - follow-up user stories covering the implementation.

@bajtos bajtos added the spike label Dec 2, 2019
@bajtos bajtos added this to the Dec 2019 milestone Dec 2, 2019
@bajtos bajtos requested review from raymondfeng and a team December 2, 2019 14:16
@bajtos bajtos self-assigned this Dec 2, 2019
@raymondfeng
Copy link
Contributor

raymondfeng commented Dec 3, 2019

@bajtos Thank you for kicking off the topic.

I have been thinking about leveraging Kubernetes's Operator and Custom Resource Definition pattern/formats to achieve similar goals recently.

  • Each artifact will be described as CRD
  • Allow extensions for artifact controllers/operators, for example, we can add LB4 datasource/connector operators

See:

Clarification:

I'm NOT proposing to require Kubernetes. The intention is to use the similar design: CRD + Operator to extend LB4 for such resources.

We can have an extension point for Operators, each of them can handle a specific kind/version of CRDs.

@@ -133,6 +133,11 @@ export class RestApplication extends Application implements HttpServerLike {
this.restServer.basePath(path);
}

controller<T>(controllerCtor: ControllerClass<T>, name?: string): Binding {
this.restServer.invalidateRoutingCache();
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather to use a ContextView to invalidate the routing cache.

Copy link
Member Author

Choose a reason for hiding this comment

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

+1, I would like to see a ContextView-based solution too. In this spike, I used the fastest path to get done what I need.

@raymondfeng Do you have bandwidth to implement ContextView-based version yourself? Here is the GitHub issue tracking this user story: #433

modelClass: typeof Entity,
options: CrudRestControllerOptions,
) {
const RepositoryClass = defineCrudRepositoryClass(modelClass);
Copy link
Member Author

Choose a reason for hiding this comment

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

I am thinking about moving defineCrudRepositoryClass to @loopback/repository. I see the function as very coupled to DefaultCrudRepository class, not adding anything opinionated or controversial on top of the named constructor.

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
A short-term workaround for the missing feature tracked by
#433

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
- Accept a MySQL connection string (host+port+database+credentials)
  and a list of table names.
- Connect to the specified MySQL database.
- Discover model schemas from the given tables
- Define LB4 models for the given tables
- Expose these models via REST API
- Return a list of URL paths of the newly added models

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
@bajtos bajtos force-pushed the spike/import-model-at-runtime branch from 5f43345 to a878d83 Compare December 9, 2019 07:45
@bajtos
Copy link
Member Author

bajtos commented Dec 9, 2019

In a878d83, I have simplified the demo controller implementation using the recently released features loopbackio/loopback-datasource-juggler#1807 and #4266. I have also reorganized the code to make it more clear how a model can stay private (not exposed via REST API).

I consider the demo app as done and proposing the following follow-up stories:

  1. Dynamic binding/rebinding of controllers after app start Dynamic binding/rebinding of controllers after app start #433 (already exists)

  2. How to define models, repositories and controllers at runtime

    The idea is create a new documentation page that will explain how to:

    1. Define a model class from a ModelDefinition object
    2. Define a repository class for the given model, how to configure dependencies (the datasource to use)
    3. Define a controller class for the given model & repository, how to configure dependencies (the repository to use)
    4. Discover model definition from a database, build ModelDefinition object from the discovered schema.

    The page can use code snippets from my demo controller to illustrate the approach in practice.

In my demo, I have a comment mentioning that it would be nice to have a LB4-native method for discovering model schemas, a method that return LB4 ModelDefinition object instead of LB3-like data. Considering that we don't know how many people will actually implement model discovery at runtime, I am proposing to leave this feature out of scope of the initial work.

@strongloop/sq-lb-apex Please take a look and let me know if you have any comments or suggestions. I'd like to get at least one approval and then I'll move on to create the follow-up story/stories.

@bajtos bajtos changed the title Spike: Discover and define models at runtime [WIP] Spike: Discover and define models at runtime Dec 9, 2019
Copy link
Contributor

@agnes512 agnes512 left a comment

Choose a reason for hiding this comment

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

👏 It's really great that I can use endpoints with just a datasource config JSON!

I only have one suggestion. If I were a user who uses this feature a lot, I'd like to know how I can modify/config the controller to match my requirements. It'd be good if we can make this part of documentation clear/detailed in the follow-up story.

@@ -703,6 +713,9 @@ export class RestServer extends Context implements Server, HttpServerLike {
let spec = this.getSync<OpenApiSpec>(RestBindings.API_SPEC);
const defs = this.httpHandler.getApiDefinitions();

// Apply shallow-clone to prevent modification of user-provided API_SPEC
Copy link
Contributor

Choose a reason for hiding this comment

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

@bajtos could you explain more about the shallow clone here?
Wouldn't it ALLOW modification of user-provided API_SPEC?

The line below it(L#706) applies deep clone which prevents the modification of the original spec.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just a temporary workaround to get my demo working. The actual implementation is likely to be different, see the discussion in Dynamic binding/rebinding of controllers after app start #433.

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.

Tested the /discover endpoint with a working url, everything is good 👍

(maybe another story)From UX's perspective...shall we create another endpoint that prints out the generated model def, like GET /showModelDefByName/{modelname}.
When testing the dynamically generated APIs, people can learn the signature of endpoints from /Todos, but It's not very easy to figure out the model properties by only having the path of the model.

console.log('Defined repository %s', RepositoryClass.name);

// Step 4: Optionally, expose the new model via REST API
const basePath = '/' + modelDef.name;
Copy link
Contributor

@jannyHou jannyHou Dec 9, 2019

Choose a reason for hiding this comment

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

nitpick: to be consistent with Todo model, maybe use the plural of the name as base path?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, plural names would be better. There are different ways how to build plurals, I am leaving it up to our users to decide.

@bajtos
Copy link
Member Author

bajtos commented Dec 10, 2019

Thank you @jannyHou and @agnes512 for thoughtful comments!

I only have one suggestion. If I were a user who uses this feature a lot, I'd like to know how I can modify/config the controller to match my requirements. It'd be good if we can make this part of documentation clear/detailed in the follow-up story.

Good idea 👍

In my mind, this is related to the epic From model definition to REST API #2036, where we already have a story to document how can extension authors build custom controller factories - see #2740.

Let's include #2740 in the follow-up stories for this spike.

(maybe another story)From UX's perspective...shall we create another endpoint that prints out the generated model def, like GET /showModelDefByName/{modelname}.
When testing the dynamically generated APIs, people can learn the signature of endpoints from /Todos, but It's not very easy to figure out the model properties by only having the path of the model.

I see your point. At the moment, you can find model properties in OpenAPI schema.

It is possible to build an endpoint you have described, or perhaps add the model definition to the response returned by POST /discover endpoint. I feel those changes are out of scope of this spike/demo app though. The purpose of this work is to discover missing building blocks and ensure everything fits together.

IMO, real world applications will never follow exactly our demo version, they will always combine these blocks in unique ways.

@bajtos
Copy link
Member Author

bajtos commented Dec 12, 2019

How to define models, repositories and controllers at runtime

Created #4296

@bajtos
Copy link
Member Author

bajtos commented Dec 12, 2019

Follow-up stories:

I am closing this spike as done.

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

Successfully merging this pull request may close these issues.

4 participants