Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

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

Open
akoita opened this issue May 5, 2019 · 1 comment
Labels
core Core packages enhancement New feature or request

Comments

@akoita
Copy link

akoita commented May 5, 2019

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.

@diestrin
Copy link
Contributor

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 diestrin added core Core packages enhancement New feature or request labels May 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core Core packages enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants