Find better way to support empty constructor and constructor with arguments

[tobiasschweizer]

Currently, complex values like ReadDateValue or ReadGeomValue are instantiated in two steps: first, json2typescript creates an instance of ParseReadDateValue or ParseReadGeomValue, respectively. From this, a ReadDateValue or ReadGeomValue is constructed. All classes are subclasses of ReadValue and therefore we need to support two constructor signatures: an empty and a non-empty one:

https://github.com/dhlab-basel/knora-api-js-lib/blob/4bbfa88e95e351662e2e3e171d625d8ae96bc338/src/models/v2/resources/values/read-value.ts#L42-L66

The solution was to make every parameter optional. But this is error prone, see #66.

[tobiasschweizer]

I got the current solution from https://stackoverflow.com/questions/39407311/typescript-class-overload-signature-is-not-compatible-with-function-implementa

[andreas-aeschlimann]

I usually use static methods to overload:

static createInstanceFromSomething(id: string, type: string, ...): ReadValue {
    const readValue = new ReadValue();
    readValue.id = id;
    readValue.type = type;
    ...
    return readValue;
}

[tobiasschweizer]

@andreas-aeschlimann Thanks, I will try out factory methods after my holidays!

[tobiasschweizer]

Sample for user endpoint:

https://github.com/dasch-swiss/knora-api-js-lib/blob/ed2e232afa062fc4073811ad5d3265cd88d2a338/src/api/admin/users/users-endpoint.spec.ts#L283-L292

[andreas-aeschlimann]

Problem: The class generated by Knora defines an empty constructor because of json2typescript. This could lead to the creation of – in view of the backend – invalid instances.

Solution: Provide a protected constructor and define a static factory method with all properties that are required by the backend to have a consistent instance.

protected constructor() {}

static createInstanceFromSomething(firstName: string, lastName: string): User {
   
    const instance = new User();
   
    // Set all properties that are required in terms of the backend to have a complete instance
    instance.firstName = firstName
    instance.lastName = lastName;
   
    // Create a consistency method that checks the current state of the class
    instance.checkConsistency();

    return instance;

}

Furthermore, the consistency checks could be in a method that always look the same:

checkConsistency(): void {
    if (this.lastName.length > 0 && this.firstName.length === 0) {
        throw new Error("User must have a firstName if it has a lastName");
    }
}

If we also need checks on specific properties, I suggest that we use getters and setters as follows:

@JsonConvert("firstName", String); // the decorator must be assigned to the private property
private _firstName: string = "";
get firstName(): string { return this._firstName; }
set firstName(value: string) {
    if (value === "root") {
        throw new Error("First name must not be root");
    }
    this._firstName = value;
}

The frontend has different needs as the backend and therefore should be able to easily extend these classes. For example, if we need stricter consistency checks or deeper checks in setters, we can still extend a class and implement this.

In any case that we want to adjust a class on the client, we will need to extend it anyway. It makes no sense to implement too much TypeScript through Knora.

[tobiasschweizer]

Provide a protected constructor and define a static factory method with all properties that are required by the backend to have a consistent instance.

So there would be no way to call the empty constructor directly, but only via the public static method. I think that's a good approach.

The frontend has different needs as the backend and therefore should be able to easily extend these classes. For example, if we need stricter consistency checks or deeper checks in setters, we can still extend a class and implement this.

I like your idea. We would have to come up with a way to keep in sync with the Knora API. More properties could be added or their names could change.

I have added integrate-generated-files-admin.sh to make this easier for the admin API.

I think the proper way to do this is coordinating the releases with @subotic and add automated e2e test with an actual Knora, see https://github.com/dasch-swiss/knora-api-js-lib-test/blob/angular/.travis.yml.

In any case that we want to adjust a class on the client, we will need to extend it anyway. It makes no sense to implement too much TypeScript through Knora.

Yes, we should get all necessary information from Knora regarding the API's interface, but not more than that.