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 - Migrate LB3 to LB4 is possible #485

Closed
dhmlau opened this issue Aug 1, 2017 · 10 comments
Closed

SPIKE - Migrate LB3 to LB4 is possible #485

dhmlau opened this issue Aug 1, 2017 · 10 comments
Assignees

Comments

@dhmlau
Copy link
Member

dhmlau commented Aug 1, 2017

Acceptance Criteria

  • Attempt to migrate a LB3 application to LB4, testing each of these areas:
    • Reuse
      • Models
        • Properties
        • Relationships
    • LB3 Juggler
      • Operation hooks
      • Properties/Relationships
    • Remote hooks
  • Note which areas are laden with friction as areas for improvement/areas that need tooling
@Setogit
Copy link
Contributor

Setogit commented Aug 8, 2017

LoopBack Next is probably the biggest architectural change we ever made to LoopBack since the first release.

IBM Cloud business as a whole, API Connect in particular, we are trying to expand the business. GBS and the company's Sales&Marketing effort is aligned to the direction.

Purpose

In this spike, I will explore and come up with a list of ENGINEERING OPTIONS AND VALUES to execute in LB Next Beta for:
a. LB 2/3 users migrating to LB Next,
b. New users to start LB Next development, and
c. Beneficial for Both

@Setogit Setogit added this to the Sprint 42 - Asteroid milestone Aug 8, 2017
@bajtos bajtos added committed and removed planning labels Aug 8, 2017
@bajtos bajtos modified the milestone: Sprint 42 - Asteroid Aug 8, 2017
@Setogit
Copy link
Contributor

Setogit commented Aug 17, 2017

A couple of points/thought as a result of this spike:

  • (1) A task is going on in this sprint to implement Repository/Model in the hello-world. From there, we should figure out the migration story/support tool from v2,v3 model/juggler to LB.Next repository.

Note: relationship feature is not currently supported in repository if I understand it correctly. This information might be out-dated..

  • (2) It's valuable for both existing and new users to beef up the OpenAPI-based solution pipeline, i.e., building/debugging specs using the swagger ( https://editor.swagger.io ), spec validation in each package ( already implemented in LB.Next using swagger-parser Nodejs package), and runtime API testing against exiting(v2,v3) or new (LB.Next) service with real data using Dredd package ( initial working sample code is in loopback-next-hello-world )

  • (3) For remote methods -- quick solution is to document the steps. tool may or may not be worthwhile because they

  • (4) it's important to put equal or more emphasis on new users because a. it's IBM' strategic direction to drum up business in Cloud using API Connect story; GBS is deploying resource to chase businesses with API Connect offerings. The target enterprise customers are mostly new and are looking for next-generation solutions with long-term IBM commitment.

In general, it seems more cost effective to focus on new usersand extend the solution for existing users in a 80:20 way. We should invest in building and demonstrating simple/solid values with LB.Next so that new as well as the existing users clearly see the value to climb up to the top of the LB.Next mountain as opposed to try to pave the road, in a sense, for the existing users (migration path). Note that each user lives in a different house, paving the road from each house to the mountain is obviously not cost effective.

@bajtos
Copy link
Member

bajtos commented Aug 18, 2017

Here are my thoughts on the migration story.

At high-level, LoopBack 3.x applications consist of three big "parts"

  • Persistence layer (this includes talking to backend services like SOAP/REST)
  • Outwards facing REST API
  • App-wide setup - Express middleware, boot scripts, etc.

In the persistence layer, users can contribute the following artifacts:

  1. Definitions of Model data types (properties, validations)
  2. Definition of data sources
  3. Configuration of models (which datasource are they attached to)
  4. Operation hooks

At the public API side, users can define:

  1. Which built-in methods should be exposed (think of disableRemoteMethodByName)
  2. Custom remote methods
  3. before/after/afterError hooks at application-level
  4. before/after/afterError hooks at model-level
  5. before/after/afterError hooks at model method level

LoopBack Next was intentionally designed to allow users to choose their ORM/persistence solution, and our initial version of @loopback/repository is based on juggler 3.x. That makes it possible for users to reuse their existing model definitions, migrating their application incrementally.

The proposal

Here is my proposal:

  • Keep the existing model definitions in .json and .js files, datasources configuration, etc. (But consider moving them around to get a saner file structure - this should be done by our migration tool we need to build.)
  • Write a small amount of code to bind LB 3.x Models with LB Next Context, e.g. for each {Model} create models.{Model} binding.
  • This allows us to reuse model definitions, datasources & connectors, operation hooks.
  • The biggest difference is in the REST API. Fortunately, we have tools to inspect all remote methods and generate Swagger specification for them.
  • For each public model, use introspection and swagger generator to code-generate a controller class implementing the same public REST API as the original LB 3.x model. This is a tool we need to build.
  • Each controller method should be a simple wrapper calling the original model method.

A simplified example:

@api({basePath: '/products'})
export class ProductController {
  constructor(
    @inject('models.Product') private Product: Entity
  ){}

  @get('/')
  @param.query.object('where')
  // TODO describe response type
  find(where: Object): Product[] {
    return await this.Product.find(where);
  }

  @get('/custom')
  // ...
  customMethod(arg: number): Object {
    return await this.Product.customMethod(arg);
  }

  @patch('/{id}')
  // ...
  prototypeUpdateAttributes(id: number, data: Partial<Model>) {
    const instance = await this.Model.sharedCtor(id);
    return await instance.updateAttributes(data);
  }

  // ...
}

Remoting hooks

Let's talk about remoting hooks now and how to translate them to LB Next:

  1. Application-level hooks should be invoked as part of the Sequence.
  2. For model-level hooks, I am proposing the convention that a controller class can provide before, after and afterError methods. These methods can be either invoked explicitly from each method (which is brittle), or we need to improve our framework to invoke them automatically as part of invokeMethod.
  3. For method-level hooks, the code should go directly into the controller method itself.
@api({basePath: '/products'})
export class ProductController {
  constructor(
    @inject('models.Product') private Product: Entity
  ){}

  before(route: Route, args: OperationArgs) {
    console.log('Invoking remote method %s with arguments %s', 
      route.describe(); JSON.stringify(args));
  }

  @get('/')
  @param.query.object('where')
  find(where: Object): Product[] {
    console.log('Here we can modify `where` filter like we were in a before-remote hook');

    return await this.Product.find(where);
  }

  // etc.
}

I don't think we can automate this migration step, because remoting hooks expect to receive a strong-remoting context that we don't have in LoopBack next. A good documentation with helpful examples is needed.

Alternatively, we can let the controller to provide invokeMethod where it's up to users to define whatever phases they need, so that they are not constrained by three phases before/after/afterError. (However, they can implement invokeMethod calling before/after/afterError if they like it.)

class MyController {
  invokeMethod(route: Route, args: OperationArgs) {
    console.log('Invoking remote method %s with arguments %s', 
      route.describe(); JSON.stringify(args));
    return await this[route.methodName](...args);
  }
}

// alternate invokeMethod implementation supporting before/after/afterError hooks
// this can be a Controller Mixin provided by an extension
  invokeMethod(route: Route, args: OperationArgs) {
    if (this.before)
      await this.before(route, args);

    try {
      const result = await this[route.methodName](...args);
      if (this.after)
        result = await this.after(route, args, result);
      return result;
    } catch(err) {
     if (this.afterError)
       const handled = await this.afterError(route, args, error);
       if (handled) return;
    }
    throw err;
  }

Summary:

  • A tool for shuffling files around to get a sane structure for LB3 + LB4 files
  • A tool for generating controllers from remoting metadata
  • Runtime support for controller hooks (before, after, afterError)
  • Instructions on how to migrate remoting hooks

Remaining things to figure out:

  • how to migrate middleware into Sequence
  • how to migrate boot scripts
  • where to keep Swagger definitions for Models so that we can reference them in controller specs

Possibly out of scope:

  • how to migrate ACLs that we don't support yet
  • how to migrate built-in models like User, AccessToken, etc.
  • change-tracking, replication and offline-sync
  • push notifications
  • storage component

Next level

  • Migrate LB 3.x .json and .js files to use @loopback/repository. The generated controllers should use typed Repositories instead of juggler 3.x based Model classes.

Justification

You may ask why to code-generate all those controller files? In my experience, one of the most frequently sought feature of LB 3.x is the ability to control and customize the public API. Users start with a wide built-in CRUD API provided by the LoopBack, but soon enough then need to lock the API down, expose only subset of endpoints, and customize how exactly these endpoints work.

In LoopBack 3.x, this was a complex process that required hooks at many different levels (middleware/strong-remoting phases, remoting hooks, operation hooks) and unintuitive API like disableRemoteMethodByName.

In my proposal, customization of the REST API is as easy as editing the generated code.

  • Need to hide a built-in (generated) endpoint? Just delete the controller method!
  • Need to modify the request arguments before executing the underlying CRUD method? Add that code into the controller method.
  • Need to add more parameters to a built-in (generated) endpoint? Just modify the controller method.
  • Need to post-process the value returned from the model? Add the transformation after you awaited the result of the model method.

@raymondfeng @ritch @Setogit @superkhau thoughts?
cc @strongloop/loopback-next

@crandmck
Copy link
Contributor

crandmck commented Aug 18, 2017

@bajtos This is an excellent, detailed proposal.

Regardless of whether we adopt every aspect of it or not, I suggest that we move the details into a wiki document that can evolve as we refine our approach to migration. IMO it's not efficient to keep a great level of detail of such a discussion in a GH issue. An issue does make sense to have a threaded discussion on specific aspects of migration (or other topics), but I think it will become cumbersome to carry on a broad-ranging conversation here.

Just my view.... Please LMK if I can help in any way. I think in an case, documentation will be a significant piece of the migration solution.

@bajtos
Copy link
Member

bajtos commented Aug 21, 2017

@crandmck You proposal makes sense to me. I'd like to get the first round of feedback before creating a wiki page - if there is a lot of resistance against my proposal, then I see little value in capturing this content in wiki.

@crandmck
Copy link
Contributor

crandmck commented Aug 21, 2017

Fair enough. We'll keep initial discussion here. Here are my comments....

In general, the proposal looks good. In particular, I think keeping the existing model definition JSON/JS formats will simplify things, even if the files are in a different location in project layout.

I have a question about

how to migrate ACLs that we don't support yet

It's not clear (to me at least) which ACLs that v4 will support and which it will not.

I'm assuming that the goal for GA is to have either a tool or detailed docs for migrating every aspect of a v3 app (which you've outlined above) to v4. Although we certainly won't have this for beta 1, I think we should try to have it in place at some point before GA to ensure that migration process is as smooth as possible.

@bajtos
Copy link
Member

bajtos commented Aug 22, 2017

@Setogit Could you please look into "remaining things" in this sprint? Please check with @raymondfeng and @ritch the priority of these items (which are important for the initial release, which can be left for later).

Remaining things to figure out:

  • how to migrate middleware into Sequence
  • where to keep Swagger definitions for Models so that we can reference them in controller specs (work in progress - Kevin)

Possibly out of scope:

  • how to migrate boot scripts
  • how to migrate ACLs that we don't support yet
  • how to migrate built-in models like User, AccessToken, etc.
  • change-tracking, replication and offline-sync
  • push notifications
  • storage component

Next level

Migrate LB 3.x .json and .js files to use @loopback/repository. The generated controllers should use typed Repositories instead of juggler 3.x based Model classes.

@bajtos bajtos assigned bajtos and unassigned Setogit Aug 31, 2017
@dhmlau
Copy link
Member Author

dhmlau commented Sep 6, 2017

@bajtos , is it considered as done?

@crandmck
Copy link
Contributor

crandmck commented Sep 6, 2017

At a minimum, I think we need to capture some of this migration info for docs: https://github.com/strongloop/loopback-next/wiki/Migrating-from-v3-to-v4. AFAIK this hasn't been done....

@bajtos
Copy link
Member

bajtos commented Oct 3, 2017

Let's close this as done as far as the Beta release goes.

@bajtos bajtos closed this as completed Oct 3, 2017
@bajtos bajtos removed the planning label Oct 3, 2017
raymondfeng pushed a commit to loopbackio/loopback.io that referenced this issue Oct 3, 2017
raymondfeng pushed a commit to loopbackio/loopback.io that referenced this issue Oct 10, 2017
raymondfeng pushed a commit to loopbackio/loopback.io that referenced this issue Oct 10, 2017
raymondfeng added a commit to loopbackio/loopback.io that referenced this issue Oct 10, 2017
* Rename Migration-guide to LoopBack-3.x

* Copy content from issue-485

See loopbackio/loopback-next#485

* Add basic mapping between LB3 and LB4
@bajtos bajtos mentioned this issue Dec 7, 2017
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

No branches or pull requests

5 participants