convector icon indicating copy to clipboard operation
convector copied to clipboard

ConvectorModel.getOne: Check the matching between the JSON fetched from the database and the target Convector model

Open akoita opened this issue 6 years ago • 1 comments

The problem

The method ConvectorModel.getOne is supposed to throw an exception when the value of the field type is different between the Convector model and the JSON fetched from the database.

  • First, the reason why the exception is not thrown in this method can be a bug.
  • And second, I think that Convector should be in general be able to throw an exception when one or some details do not match between the data fetched from the ledger and the Convector model in TypeScript. This can be seen as one advantage of strong typing and therefore allow to detect potential incoherence early during the execution.

Environment

  • Convector version (or git revision) that exhibits the issue: 1.3.3

Details

In a situation where I have for example two types of participants in my network: InvestorParticipant and CareerAdvisorParticipant. The two models have the same structure, their unique difference is the value of the field type.

export class InvestorParticipant extends AbstractTrainingParticipantModel<InvestorParticipant> {
    @ReadOnly()
    @Required()
    readonly type = 'io.worldsibu.investor';
}
export class CareerAdvisorParticipant extends AbstractTrainingParticipantModel<CareerAdvisorParticipant> {
     @ReadOnly()
    @Required()
    readonly type =  'io.worldsibu.careerAdvisor';
}

And we have the set of AssetA owned by CareerAdvisorParticipant :

// AssetA owned by CareerAdvisorParticipant 
export  class AssetA<T extends AssetA<any>> extends ConvectorModel<T> {
    @ReadOnly()
    @Required()
    public readonly type = 'io.worldsibu.AssetA';
    
    @Required()
    @Validate(yup.string())
     ownerId: string;
}

Current Behavior

Before saving AssetA for example, I must verify that the owner exists and is a CareerAdvisorParticipant: const participant = await CareerAdvisorParticipant.getOne(assetA.ownerId); Surprisingly, this call wil work fine if assetA.ownerId is the id of an InvestorParticipant. But in the code, the method ConvectorModel.getOne is supposed to throw an exception because we were expecting from the database a "data of type CareerAdvisorParticipant", but we got a "data of type InvestorParticipant" instead. This is the code:

    const content = await BaseStorage.current.get(id, storageOptions);

    const model = new type(content);

    if ((content && model) && content.type !== model.type) {
      throw new Error(`Possible ID collision, element ${id} of type ${content.type} is not ${model.type}`);
    }

    return model;

The exception is not thown because the value of the field "type" of model that is supposed to be a constant is overwritten by the value of the field "type" in the JSON(content) during the conversion.

Expected Behavior

As shown above, the expected behavior of the method ConvectorModel.getOne seems to be to throw an exception in this kind of scenario.

And in general, I think that Convector should help us to detect incoherence when we are trying to use the data fetched from the ledger with Convector models.

akoita avatar May 05 '19 06:05 akoita

I agree with you that Convector should help detecting this kind of things. This looks like a valid issue, and I think it can be easily addressed since a Convector model has a MyModel.schema() method to return a YUP schema for it. Yup has a validate method to be strict when parsing, so we can make this even configurable in the .getOne() call, by passing some options on the behavior of this method

diestrin avatar May 30 '19 19:05 diestrin