loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.97k stars 1.07k forks source link

SPIKE - Migrate LB3 to LB4 is possible #485

Closed dhmlau closed 7 years ago

dhmlau commented 7 years ago

Acceptance Criteria

Setogit commented 7 years ago

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 commented 7 years ago

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

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

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 commented 7 years ago

Here are my thoughts on the migration story.

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

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:

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:

Remaining things to figure out:

Possibly out of scope:

Next level

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.


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

crandmck commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

@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:

Possibly out of scope:

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.

dhmlau commented 7 years ago

@bajtos , is it considered as done?

crandmck commented 7 years ago

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 commented 7 years ago

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