Computed model properties

[ibmjm]

Opening this issue per discussion with Miroslav Bajtos...

Feature proposal

Some way to declare a computed property on a model. I think there are times where the model is the correct place to centralise some model-specific computation.

My use case

I am modelling a business process where we start with a request, and as the request is reviewed, approved, etc. it will have a status to reflect where it is in the process. But I also want to be able to be able to categorise the various status codes more generally, as to whether they are "open" or "closed", so that I can display the requests and enable/disable features as necessary. It seems like a bad idea to duplicate the logic that maps status codes to an "open" / "closed" meta-status in both back-end and probably multiple places in the front-end, when a computed property on the model could definitively provide that.

To be clear, the computed properties I'm thinking of would not themselves be saved to any persistence layer via the repository -- there would be no point since they can be / are computed from the current state of the model.

Current Behavior

At present, we can decorate a class with @model, and within the class decorate properties with @property, but there doesn't seem to be a way to write a method and decorate it to indicate it is a computed property.

Expected Behavior

Decorating a method with @computed or similarly-named decorator, and having LB4 expose the result of that method as if it were a property when responding, would open up some interesting ways to simplify and centralise some APIs.

See Reporting Issues for more tips on writing good issues

[dhmlau]

Since it's feature parity gap, moving to Needs Priority.

[bajtos]

Thank you @ibmjm for starting this discussion.

In LoopBack 3.x, it was possible to define computed properties via MyModel.getter[propertyName] syntax, as I explained in https://stackoverflow.com/questions/25438663/dynamic-properties-or-aggregate-functions-in-loopback-models.

We don't have a first-class support for computed properties in LB4 yet. /cc @raymondfeng

To make computed properties work, I think we need to:

• Ensure that model's .toJSON() function includes the computed property in the data object returned.
• Ensure that the computed property is discarded when sending data to the database.
• In OpenAPI spec describing input & output values of REST API endpoints, the computed property must be present in the description of response bodies, it must be omitted (or at least optional) in request bodies.

Temporary workaround

I can see different workaround solutions available, each having its own pros and cons. I think the following may be the easiest option:

In your model class, define your computed property as a getter. In the model constructor, remove the computed property from the provided data object.

class MyModel extends Entity {
  // ...

  @property({
    type: 'string',
  })
  get computed(): string {
    // build the computed value here, e.g.
    // return this.title + ': ' + this.desc;
  }

  constructor(data?: Partial<MyModel>) {
    if (data) {
      delete (data as any).computed;
    }
    super(data);
  }
}

In your Repository class, setup a LB3 Operation Hook to remove the computed property from model data before its persisted.

export class MyModelRepository extends DefaultCrudRepository<
  MyModel,
  typeof MyModel.prototype.id
> {
  constructor(@inject('datasources.db') dataSource: juggler.DataSource) {
    super(MyModel, dataSource);

    (this.modelClass as any).observe('persist', async (ctx: any) => {
      delete ctx.data.computed;
    });
  }
}

In this solution:

• Data returned by your REST APIs will always include the computed property.
• Requests made to your REST API can exclude the computed property.
• If a request includes the computed property, the value is silently ignored.
• The computed property is removed from model data stored in the database. (Beware: automigration will still create a column for that property).

In the near feature, we will be implementing new API allowing controller methods to exclude certain model properties from the OpenAPI schema generated by our helpers, see #2653. With that feature in place, it will be possible to improve the OpenAPI schemas for write endpoints to exclude the computed property, and therefore reject requests containing the computed property as part of the built-in validation step.

Also once we implement native operation hooks in LB4 (see #1919 and #2095), it will be possible to simplify the implementation in the repository class.

[ibmjm]

Thanks for the suggestion @bajtos . It appears to work up until the controller tries to send the response, at which point (I believe) it runs afoul of the schema for my ReturnRequest model class.

This is the error I get:
Unhandled error in GET /request/22: 500 Error: ER_BAD_FIELD_ERROR: Unknown column 'isOpen' in 'field list'

I don't know enough about schemas to be able to do anything robust (let alone elegant) that wouldn't be a maintenance headache... it seems to me that a lot of the benefit of LoopBack is in letting it infer schemas from the models in the first place.

For reference, my repository constructor:

  constructor(
    @inject('datasources.iarDatabase') dataSource: IarDatabaseDataSource,
    @repository.getter(AssetRepository) protected assetRepositoryGetter: Getter<AssetRepository>,
  ) {
    super(ReturnRequest, dataSource);
    (this.modelClass as any).observe('persist', async (ctx: any) => { delete ctx.data.isOpen; });
    this.assets = this.createHasManyRepositoryFactoryFor('assets', assetRepositoryGetter);
  }

and my computed property definition per your workaround suggestion, kindly provided above:

  @property({
    type: 'boolean'
  })
  get isOpen (): boolean { 
    return this.statusCode != 'XR' && this.statusCode != 'RC';
  }

[bajtos]

@ibmjm Hmm, that's weird. Could you please create a small application I can use to reproduce the problem and play with different solutions to find which works? See https://loopback.io/doc/en/contrib/Reporting-issues.html#loopback-4x-bugs

[ibmjm]

@bajtos Weird indeed. I tried to do as you asked (cloned loopback-next, checked it out to a new branch, tweaked the todo example) but I was not able to reproduce the error. In other words, your proscribed "workaround" for adding a computed property worked when I modified the Todo application, but fails when I try adding similar to my own application. 😞

[ibmjm]

Could dependency differences between my application's package.json (-) and the TODO example's package.json (+) make the difference, I wonder:

   "dependencies": {
-    "@loopback/authentication": "^2.0.3",
-    "@loopback/boot": "^1.2.7",
-    "@loopback/context": "^1.15.0",
-    "@loopback/core": "^1.7.0",
-    "@loopback/openapi-v3": "^1.3.11",
-    "@loopback/repository": "^1.5.5",
-    "@loopback/rest": "^1.11.2",
-    "@loopback/rest-explorer": "^1.1.22",
-    "@loopback/service-proxy": "^1.1.10",
-    "@types/cookie-parser": "^1.4.1",
-    "@types/express-session": "^1.15.12",
-    "connect-history-api-fallback": "^1.6.0",
-    "cookie-parser": "^1.4.4",
-    "dotenv": "^8.0.0",
-    "express-session": "^1.16.1",
-    "libreoffice-convert": "^1.0.0",
-    "loopback-connector-db2": "^2.1.2",
-    "loopback-connector-mysql": "^5.3.1",
-    "loopback-connector-rest": "^3.4.1",
-    "nodemailer": "^6.1.0",
-    "passport": "^0.4.0",
-    "passport-http": "^0.3.0",
-    "passport-idaas-openidconnect": "^2.0.2",
-    "pdfmake": "github:bpampuch/pdfmake",
-    "tslint-consistent-codestyle": "^1.15.1"
+    "@loopback/boot": "^1.2.3",
+    "@loopback/context": "^1.12.0",
+    "@loopback/core": "^1.6.0",
+    "@loopback/openapi-v3": "^1.3.7",
+    "@loopback/openapi-v3-types": "^1.0.14",
+    "@loopback/repository": "^1.5.1",
+    "@loopback/rest": "^1.10.4",
+    "@loopback/rest-explorer": "^1.1.18",
+    "@loopback/service-proxy": "^1.1.6",
+    "loopback-connector-rest": "^3.1.1"
   },
   "devDependencies": {
-    "@loopback/build": "^1.5.4",
-    "@loopback/testlab": "^1.2.9",
-    "@loopback/tslint-config": "^2.0.2",
-    "@types/node": "^10.12.30",
-    "@types/passport": "^1.0.0",
-    "@types/passport-http": "^0.3.8",
-    "tslint": "^5.13.1",
-    "typescript": "^3.3.3333"
-  }
+    "@loopback/build": "^1.5.0",
+    "@loopback/http-caching-proxy": "^1.0.15",
+    "@loopback/testlab": "^1.2.5",
+    "@loopback/tslint-config": "^2.0.4",
+    "@types/lodash": "^4.14.109",
+    "@types/node": "^10.11.2",
+    "lodash": "^4.17.10",
+    "tslint": "^5.15.0",
+    "typescript": "^3.4.3"
+  },

[bajtos]

Could dependency differences between my application's package.json (-) and the TODO example's package.json (+) make the difference, I wonder

Probably not. AFAICT, my example code does not use anything specific to recent LB4 versions that would not work before.

[ibmjm]

@bajtos Weird indeed. I tried to do as you asked (cloned loopback-next, checked it out to a new branch, tweaked the todo example) but I was not able to reproduce the error. In other words, your proscribed "workaround" for adding a computed property worked when I modified the Todo application, but fails when I try adding similar to my own application. 😞

I got it to work following the posted solution in issue #1919. Within my repository constructor:

    super(ApplianceSerial, dataSource);

    // delete computed properties from the model before db read/write ops
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    (this.modelClass as any).observe('access', async (ctx: any) => {
     delete ctx.Model.definition.properties.ibmSerial;
    });

[stale]

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

[stale]

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

[mike-bernard]

@bajtos workaround didn't quite work for me and since I wanted to continue using auto-migration, I couldn't annotate the class property with @property. Here's what I was able to get working, if it helps anyone in the future.

In the Model, status is defined on the class, and a property getter is defined in the constructor.

  status: 'Not Started' | 'Running' | 'Completed' = 'Not Started';

  constructor(data?: Partial<Campaign>) {
    super(data);

	Object.defineProperty(this, 'status', {
		get() {
			const now = Date.now();
			const start = new Date(this.startDate ? this.startDate : '1/1/1970').getTime();
			const end = new Date(this.endDate ? this.endDate : now + 500 * 365 * 24 * 60 * 60 * 1000).getTime();

			if (now > end) {
				return 'Completed';
			} else if (now < start) {
				return 'Not Started';
			} else {
				return 'Running'
			}
		}
	});
  }

Using LB3 operation hooks in the repository as suggested above, I was able to use the 'before save' hook and clear out the unknown properties array that the model is being validated against.

constructor(
    @inject('datasources.dev_db') dataSource: DevDbDataSource,
) {
    super(Campaign, dataSource);
  
    (this.modelClass as any).observe('before save', async (ctx: any) => {
		ctx.instance.__unknownProperties = [];
		//can remove the specific property from this array instead if that works better for you
    });
  }

Thanks @bajtos for the direction!