Optimize the workflow of getting an model instance

[Keith-CY]

Now in the mvp dapp, an instance of model is getting by method defined in the controller https://github.com/ckb-js/kuai/blob/91a5ad378b47a420970fde6e86e0b7f2ddf72bda/packages/samples/mvp-dapp/src/app.controller.ts#L67-L87

It returns an instance if one has been registered or instantiates one if there's no live model.

This logic could be encapsulated in the registry, so users don't have to write it in different dapps repeatedly.

[Daryl-L]

What I though before was to implement this in find method in registry, so that developers don't need to write repeatedly, is is right?

[yanguoyu]

I think currently it's right, but In the future users will not need to call find with registry, they will call actor.call or actor.cast, and it will automatically create an instance in registry and handle message.value.

[Daryl-L]

From my understanding, what I designed and implemented is like this.

I think, the ActorRef is essential for every actor object, so I added a parameter in constructor for Actor to initiate the ActorRef, and it should be added in every sub class. With this, every actor object could know its own ActorRef, and no need to define a new class to reflect to metadata.

class Actor {
  constructor(ref?: ActorRef) {
    const metadata: { ref: ActorRef | undefined } = Reflect.getMetadata(ProviderKey.Actor, this.constructor)
    // TODO: add explicit error message
    ref = ref ? ref : metadata?.ref
    if (!ref) throw new Error()
    this.#ref = ref
    this.receiveMail()
  }
}

constructor(
    ref?: ActorRef,
    schemaOption?: GetStorageOption<StructSchema>,
    params?: {
      states?: Record<OutPointString, GetStorageStruct<StructSchema>>
      chainData?: Record<OutPointString, UpdateStorageValue>
      cellPattern?: CellPattern
      schemaPattern?: SchemaPattern
      options?: Option
    },
  ) {
    super(ref)
    this.cellPattern = Reflect.getMetadata(ProviderKey.CellPattern, this.constructor, this.ref.uri) || params?.cellPattern
    this.schemaPattern = Reflect.getMetadata(ProviderKey.SchemaPattern, this.constructor) || params?.schemaPattern
    this.schemaOption = schemaOption
    this.states = params?.states || {}
    this.chainData = params?.chainData || {}
    this.options = params?.options
    this.#lock = Reflect.getMetadata(ProviderKey.LockPattern, this.constructor, this.ref.uri)
    this.#type = Reflect.getMetadata(ProviderKey.TypePattern, this.constructor, this.ref.uri)
  }

For registry, I think it is customized for Actor, so I added the parameter ref to private method bind which was defined in metadata. And the find method will call bind while the Actor is not initiated.

class Registry {
  find = async <T = Actor>(ref: ActorRef, module: new (...args: Array<unknown>) => unknown): Promise<T | undefined> => {
    try {
      let actor = this.#container.get<T>(ref.uri)
      if (!actor) {
        this.#bind(module, { ref })
        actor = this.#container.get<T>(ref.uri)
        const resourceBindingRegister = Reflect.getMetadata(ProviderKey.ResourceBindingRegister, ref.uri)
        if (resourceBindingRegister) {
          await (actor as Actor).call('local://resource', resourceBindingRegister)
        }
      }
      return actor
    } catch (e) {
      console.log('Registry `find` catch error', e)
      return undefined
    }
  }

  bind = (module: new (...args: Array<unknown>) => unknown): void =>
    this.#bind(module, Reflect.getMetadata(ProviderKey.Actor, module))

  #bind = (module: new (...args: Array<unknown>) => unknown, metadata?: Record<'ref', ActorRef>): void => {
  }
}

The implementation is still in test now. Please correct me if I was wrong.

[Daryl-L]

https://github.com/ckb-js/kuai/pull/181

[Keith-CY]

From my understanding, what I designed and implemented is like this.

I think, the ActorRef is essential for every actor object, so I added a parameter in constructor for Actor to initiate the ActorRef, and it should be added in every sub class. With this, every actor object could know its own ActorRef, and no need to define a new class to reflect to metadata.

class Actor {
  constructor(ref?: ActorRef) {
    const metadata: { ref: ActorRef | undefined } = Reflect.getMetadata(ProviderKey.Actor, this.constructor)
    // TODO: add explicit error message
    ref = ref ? ref : metadata?.ref
    if (!ref) throw new Error()
    this.#ref = ref
    this.receiveMail()
  }
}

constructor(
    ref?: ActorRef,
    schemaOption?: GetStorageOption<StructSchema>,
    params?: {
      states?: Record<OutPointString, GetStorageStruct<StructSchema>>
      chainData?: Record<OutPointString, UpdateStorageValue>
      cellPattern?: CellPattern
      schemaPattern?: SchemaPattern
      options?: Option
    },
  ) {
    super(ref)
    this.cellPattern = Reflect.getMetadata(ProviderKey.CellPattern, this.constructor, this.ref.uri) || params?.cellPattern
    this.schemaPattern = Reflect.getMetadata(ProviderKey.SchemaPattern, this.constructor) || params?.schemaPattern
    this.schemaOption = schemaOption
    this.states = params?.states || {}
    this.chainData = params?.chainData || {}
    this.options = params?.options
    this.#lock = Reflect.getMetadata(ProviderKey.LockPattern, this.constructor, this.ref.uri)
    this.#type = Reflect.getMetadata(ProviderKey.TypePattern, this.constructor, this.ref.uri)
  }

For registry, I think it is customized for Actor, so I added the parameter ref to private method bind which was defined in metadata. And the find method will call bind while the Actor is not initiated.

class Registry {
  find = async <T = Actor>(ref: ActorRef, module: new (...args: Array<unknown>) => unknown): Promise<T | undefined> => {
    try {
      let actor = this.#container.get<T>(ref.uri)
      if (!actor) {
        this.#bind(module, { ref })
        actor = this.#container.get<T>(ref.uri)
        const resourceBindingRegister = Reflect.getMetadata(ProviderKey.ResourceBindingRegister, ref.uri)
        if (resourceBindingRegister) {
          await (actor as Actor).call('local://resource', resourceBindingRegister)
        }
      }
      return actor
    } catch (e) {
      console.log('Registry `find` catch error', e)
      return undefined
    }
  }

  bind = (module: new (...args: Array<unknown>) => unknown): void =>
    this.#bind(module, Reflect.getMetadata(ProviderKey.Actor, module))

  #bind = (module: new (...args: Array<unknown>) => unknown, metadata?: Record<'ref', ActorRef>): void => {
  }
}

The implementation is still in test now. Please correct me if I was wrong.

It seems that the ref is injected imperatively instead of declaratively

[Keith-CY]

The gola of this design is

I think, the ActorRef is essential for every actor object, so I added a parameter in constructor for Actor to initiate the ActorRef, and it should be added in every sub class. With this, every actor object could know its own ActorRef, and no need to define a new class to reflect to metadata.

So a ref param is required in constrcutor, but define a new class is not skipped if a sub class will be declared

[Daryl-L]

The gola of this design is

I think, the ActorRef is essential for every actor object, so I added a parameter in constructor for Actor to initiate the ActorRef, and it should be added in every sub class. With this, every actor object could know its own ActorRef, and no need to define a new class to reflect to metadata.

So a ref param is required in constrcutor, but define a new class is not skipped if a sub class will be declared

Yes, but actually, not only one sub class will be declared before, like this.

First, a Model was declared as a parent class. https://github.com/ckb-js/kuai/blob/9f2da62f1a799ffffce72d009deaed169b6cb0ce/packages/samples/mvp-dapp/src/actors/omnilock.model.ts#L10

Then, a sub class which extended the Model should be declared to bind the metadata. https://github.com/ckb-js/kuai/blob/9f2da62f1a799ffffce72d009deaed169b6cb0ce/packages/samples/mvp-dapp/src/app.controller.ts#L52 https://github.com/ckb-js/kuai/blob/9f2da62f1a799ffffce72d009deaed169b6cb0ce/packages/samples/mvp-dapp/src/app.controller.ts#L53

Finally, the registry initiated the Model and bound it.

As I could not find a way to pass metadata by both constructor and ref, what I do is to bind an instance of the Model to registry with the ref parameter.

[Keith-CY]

The gola of this design is

I think, the ActorRef is essential for every actor object, so I added a parameter in constructor for Actor to initiate the ActorRef, and it should be added in every sub class. With this, every actor object could know its own ActorRef, and no need to define a new class to reflect to metadata.

So a ref param is required in constrcutor, but define a new class is not skipped if a sub class will be declared

Yes, but actually, not only one sub class will be declared before, like this.

First, a Model was declared as a parent class.

https://github.com/ckb-js/kuai/blob/9f2da62f1a799ffffce72d009deaed169b6cb0ce/packages/samples/mvp-dapp/src/actors/omnilock.model.ts#L10

Then, a sub class which extended the Model should be declared to bind the metadata.

https://github.com/ckb-js/kuai/blob/9f2da62f1a799ffffce72d009deaed169b6cb0ce/packages/samples/mvp-dapp/src/app.controller.ts#L52

https://github.com/ckb-js/kuai/blob/9f2da62f1a799ffffce72d009deaed169b6cb0ce/packages/samples/mvp-dapp/src/app.controller.ts#L53

Finally, the registry initiated the Model and bound it.

As I could not find a way to pass metadata by both constructor and ref, what I do is to bind an instance of the Model to registry with the ref parameter.

The NewStore could be skipped by injecting metadata(with parameters) to OmnilockModel directly, as following

// class NewStore extends OmnilockModel {}
// Reflect.defineMetadata(ProviderKey.Actor, { ref: actorRef }, NewStore)
// Reflect.defineMetadata(ProviderKey.CellPattern, createCellPattern(lock), NewStore)
// Reflect.defineMetadata(ProviderKey.LockPattern, lock, NewStore)

@Ref(`${omnilock_code_hash}/${omnilock_hash_type}/:args`)
@CellPattern({ codeHash: 'custom_code_hash', hashType: 'custom_hash_type' })
@LockPattern({ codeHash: '...', hashType: '...' })
class OmnilockModel {
  constructor(@Params('lock') lock: Omit<Script, 'args'>, @Params('args') args: string) {
    this.lock = { ...lock, args }
  }
}

And in the registry, when a request hits ref /omnilock_code_hash/omnilock_hash_type/, the args could be matched and injected into the instance by the registry.

The idea was from route parameters of nestjs, ref: https://docs.nestjs.com/controllers#route-parameters

[Daryl-L]

The gola of this design is

I think, the ActorRef is essential for every actor object, so I added a parameter in constructor for Actor to initiate the ActorRef, and it should be added in every sub class. With this, every actor object could know its own ActorRef, and no need to define a new class to reflect to metadata.

So a ref param is required in constrcutor, but define a new class is not skipped if a sub class will be declared

Yes, but actually, not only one sub class will be declared before, like this. First, a Model was declared as a parent class. https://github.com/ckb-js/kuai/blob/9f2da62f1a799ffffce72d009deaed169b6cb0ce/packages/samples/mvp-dapp/src/actors/omnilock.model.ts#L10

Then, a sub class which extended the Model should be declared to bind the metadata. https://github.com/ckb-js/kuai/blob/9f2da62f1a799ffffce72d009deaed169b6cb0ce/packages/samples/mvp-dapp/src/app.controller.ts#L52

https://github.com/ckb-js/kuai/blob/9f2da62f1a799ffffce72d009deaed169b6cb0ce/packages/samples/mvp-dapp/src/app.controller.ts#L53

Finally, the registry initiated the Model and bound it. As I could not find a way to pass metadata by both constructor and ref, what I do is to bind an instance of the Model to registry with the ref parameter.

The NewStore could be skipped by injecting metadata(with parameters) to OmnilockModel directly, as following

// class NewStore extends OmnilockModel {}
// Reflect.defineMetadata(ProviderKey.Actor, { ref: actorRef }, NewStore)
// Reflect.defineMetadata(ProviderKey.CellPattern, createCellPattern(lock), NewStore)
// Reflect.defineMetadata(ProviderKey.LockPattern, lock, NewStore)

@Ref(`${omnilock_code_hash}/${omnilock_hash_type}/:args`)
@CellPattern({ codeHash: 'custom_code_hash', hashType: 'custom_hash_type' })
@LockPattern({ codeHash: '...', hashType: '...' })
class OmnilockModel {
  constructor(@Params('lock') lock: Omit<Script, 'args'>, @Params('args') args: string) {
    this.lock = { ...lock, args }
  }
}

And in the registry, when a request hits ref /omnilock_code_hash/omnilock_hash_type/, the args could be matched and injected into the instance by the registry.

The idea was from route parameters of nestjs, ref: https://docs.nestjs.com/controllers#route-parameters

I modify the ActorProvider to implement the router instead of new decorator Ref.

And after implementing the CellPattern and LockPattern, I find out that it is not easy to use because it is too customization. So I think it is better for developers to define the logic in their own sub Store class.