feat(example-validation-app): add validation example #4895

dhmlau · 2020-03-16T18:40:34Z

Related to: #4874

Add a validation example.
Next step:

PR feat(docs): add validation docs #4874 will be updated accordingly to include documentation.
submit a PR to add it to the example list and docs changes.

Files changed apart from the generated files:

src/models/coffee-shop.model.ts: validation logic
src/interceptors/validate-phone-num.interceptor.ts: validation logic
src/contollers/coffee-shop.controller.ts: use of class level interceptor
src/tests/acceptance/validate.acceptance.ts

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
API Documentation in code was updated
Documentation in /docs/site was updated
Affected artifact templates in packages/cli were updated
Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

dhmlau · 2020-03-18T18:20:34Z

The coverage decrease reported by coverall are in examples/hello-world and examples/rpc-server packages which I didn't make any changes.

emonddr · 2020-03-18T18:47:27Z

examples/validation-app/README.md

+
+The application will start on port 3000. Open http://localhost:3000/explorer in
+your browser. You can try to test the validation for the `/coffee-shops`
+endpoints.


Please point to the document you will be completing in this PR : #4874 . This way, the user won't be frustrated. We shouldn't expect them to dig into the acceptance and unit tests to figure out what steps they should take to test the validation in the API Explorer.

Great example, by the way :)

jannyHou

👏 Good to have the json spec + interceptor's validation example, mostly LGTM. A few suggestions and minor comments.

examples/validation-app/src/migrate.ts

jannyHou · 2020-03-18T19:03:31Z

examples/validation-app/src/interceptors/validate-phone-num.interceptor.ts

+ */
+@bind({tags: {key: ValidatePhoneNumInterceptor.BINDING_KEY}})
+export class ValidatePhoneNumInterceptor implements Provider<Interceptor> {
+  static readonly BINDING_KEY = `interceptors.${ValidatePhoneNumInterceptor.name}`;


nitpick: I think usually we create a keys.ts file to expose the bingding keys. It's easy for organizing, especially when turn it into a component.

it's generated by lb4 interceptor.

examples/validation-app/src/interceptors/validate-phone-num.interceptor.ts

jannyHou · 2020-03-18T19:19:41Z

examples/validation-app/src/interceptors/validate-phone-num.interceptor.ts

+  ) {
+    // Add pre-invocation logic here
+    const coffeeShop: CoffeeShop = new CoffeeShop();
+    if (invocationCtx.methodName === 'create')


hmm, how about using method level interceptor to generate the instance?
I think the situation is similar to what I did in the access control example's method level voter: before executing the validation/authorization logic, we need do some pre-work based on method.

And I am a bit confused why have to create a new model instance first? Wouldn't the data from request already a model instance?

(Considering this example aims to demo validation. The interceptor details doesn't can be improved in a 2nd round :p)

@emonddr had the same question about instantiating the CoffeeShop instance, but I couldn't find a good alternatives. See discussion: https://github.com/strongloop/loopback-next/pull/4874/files#r392316159. Please let me know if you have a better solution! Thanks.

For method level interceptor, I'm thinking the interceptor should be applied to multiple methods in the CoffeeShopController (create, update, updateById, patch), so that's why i'm using class level interceptor.

You can still apply the decorator at relevant methods. That might be more explicit.

@dhmlau The method level interceptor can be separated from the global one, e.g. create method has an endpoint level interceptor which adds the coffeeshop instance to the invocationContext, and the controller level interceptor does the validation.

I am good with the current implementation to land this PR, we can always discuss and improve after it.

Talked to @jannyHou. She was thinking to have the class level interceptor to validate the instance and retrieve the instance based on the method name in the method level interceptor. But it's good with my current implementation.

examples/validation-app/src/interceptors/validate-phone-num.interceptor.ts

examples/validation-app/README.md

raymondfeng · 2020-03-18T23:16:37Z

examples/validation-app/src/controllers/coffee-shop.controller.ts

@@ -28,7 +28,7 @@ import {ValidatePhoneNumInterceptor} from '../interceptors';
 import {CoffeeShop} from '../models';
 import {CoffeeShopRepository} from '../repositories';

-// Add this line to apply interceptor to this class
+// Add this line to apply interceptor to this method


You apply the decorator at class level and use if logic in the implementation to skip other methods.

I meant to say the comment should be // Add this line to apply interceptor to this class

examples/validation-app/src/interceptors/validate-phone-num.interceptor.ts

dhmlau · 2020-03-19T01:12:08Z

@jannyHou @raymondfeng, I've got the "undefined" code working in the interceptor. Please review my latest commit on the changes: 1cecd21.

@emonddr, I've modified the README to contain more info on how to trigger the validation error.

Arathy-sivan · 2020-03-20T07:22:30Z

When I used this code I got the error ,,

Server is running at http://[::1]:3000
Try http://[::1]:3000/ping
Unhandled error in POST /phonemds: 500 TypeError: Cannot convert undefined or null to object
at Function.assign ()
at ValidatePhoneNumInterceptor.intercept (D:\phone\src\interceptors\validateph.interceptor.ts:47:14)
at D:\phone\node_modules@loopback\context\src\interceptor-chain.ts:175:14
at Object.transformValueOrPromise (D:\phone\node_modules@loopback\context\src\value-promise.ts:270:12)
at GenericInterceptorChain.invokeNextInterceptor (D:\phone\node_modules@loopback\context\src\interceptor-chain.ts:170:12)
at GenericInterceptorChain.next (D:\phone\node_modules@loopback\context\src\interceptor-chain.ts:158:17)
at GenericInterceptorChain.invokeInterceptors (D:\phone\node_modules@loopback\context\src\interceptor-chain.ts:144:17)
at Object.invokeInterceptors (D:\phone\node_modules@loopback\context\src\interceptor-chain.ts:207:16)
at D:\phone\node_modules@loopback\context\src\interceptor.ts:343:14
at Object.tryWithFinally (D:\phone\node_modules@loopback\context\src\value-promise.ts:196:14)
at Object.invokeMethodWithInterceptors (D:\phone\node_modules@loopback\context\src\interceptor.ts:337:10)
at Object.invokeMethod (D:\phone\node_modules@loopback\context\src\invocation.ts:184:10)
at ControllerRoute.invokeHandler (D:\phone\node_modules@loopback\rest\src\router\controller-route.ts:145:12)
at processTicksAndRejections (internal/process/task_queues.js:97:5)
at MySequence.handle (D:\phone\src\sequence.ts:29:22)
at HttpHandler._handleRequest (D:\phone\node_modules@loopback\rest\src\http-handler.ts:78:5)

emonddr · 2020-03-20T14:18:05Z

examples/validation-app/src/interceptors/validate-phone-num.interceptor.ts

+    // Add pre-invocation logic here
+    let coffeeShop: CoffeeShop | undefined;
+    if (invocationCtx.methodName === 'create')
+      Object.assign(coffeeShop, invocationCtx.args[0]);


Does this work? coffeeShop is undefined at this point. Won't the Object.assign fail if LHS is undefined?

you're right. it failed when i'm running the actual app. I've reverted it back to the original:

const coffeeShop: CoffeeShop = new CoffeeShop();

If you have a better solution, please let me know!

ok. Just had webex with Dom, and we have a solution. Updated the PR. Thanks!

hacksparrow · 2020-03-20T15:01:41Z

examples/validation-app/README.md

+npm start
+```
+
+The application will start on port 3000. Open http://localhost:3000/explorer in


start on -> listen on?

dhmlau · 2020-03-20T15:06:35Z

@Arathy-sivan, thanks for bringing it up. I've fixed it in the latest commit. Please check again. Thanks.

examples/validation-app/README.md

nabdelgadir · 2020-03-20T22:26:37Z

examples/validation-app/src/controllers/ping.controller.ts

@@ -0,0 +1,57 @@
+// Copyright IBM Corp. 2020. All Rights Reserved.


I know this is generated automatically, but if it's not needed for the example, can it be removed?

do you mean to remove copyrights for all files in this example?

Oh no sorry, I meant the ping controller.

examples/validation-app/src/__tests__/acceptance/validate.acceptance.ts

Arathy-sivan · 2020-03-21T06:54:55Z

@Arathy-sivan, thanks for bringing it up. I've fixed it in the latest commit. Please check again. Thanks.

Thanks, it's working.

Arathy-sivan · 2020-03-21T10:49:58Z

Hi,
I want to use a field 'Landphoneno' and i wish to use this as an optional,
ie; @Property({required: false}). I used the above code and executed without using the field 'Landphoneno' then i got an error ,

Unhandled error in POST /phonemds: 500 TypeError: Cannot read property 'slice' of undefined
at ValidatePhoneNumInterceptor.isAreaCodeValid (D:\phone\src\interceptors\validateph.interceptor.ts:62:35)
at ValidatePhoneNumInterceptor.intercept (D:\phone\src\interceptors\validateph.interceptor.ts:46:13)
at D:\phone\node_modules@loopback\context\src\interceptor-chain.ts:175:14
at Object.transformValueOrPromise (D:\phone\node_modules@loopback\context\src\value-promise.ts:270:12)
at GenericInterceptorChain.invokeNextInterceptor (D:\phone\node_modules@loopback\context\src\interceptor-chain.ts:170:12)
at GenericInterceptorChain.next (D:\phone\node_modules@loopback\context\src\interceptor-chain.ts:158:17)
at GenericInterceptorChain.invokeInterceptors (D:\phone\node_modules@loopback\context\src\interceptor-chain.ts:144:17)
at Object.invokeInterceptors (D:\phone\node_modules@loopback\context\src\interceptor-chain.ts:207:16)
at D:\phone\node_modules@loopback\context\src\interceptor.ts:343:14
at Object.tryWithFinally (D:\phone\node_modules@loopback\context\src\value-promise.ts:196:14)
at Object.invokeMethodWithInterceptors (D:\phone\node_modules@loopback\context\src\interceptor.ts:337:10)
at Object.invokeMethod (D:\phone\node_modules@loopback\context\src\invocation.ts:184:10)
at ControllerRoute.invokeHandler (D:\phone\node_modules@loopback\rest\src\router\controller-route.ts:145:12)
at processTicksAndRejections (internal/process/task_queues.js:97:5)
at MySequence.handle (D:\phone\src\sequence.ts:29:22)
at HttpHandler._handleRequest (D:\phone\node_modules@loopback\rest\src\http-handler.ts:78:5)

achrinza · 2020-03-21T13:37:37Z

@Ararathy-sivan Cannot read property “slice” of undefined hints that the interceptor may not have code to handle cases where the property is optional.

Ensure that your interceptor is coded to handle this case.

nabdelgadir

Nice job 👍

nabdelgadir · 2020-03-23T16:51:32Z

examples/validation-app/src/index.ts

+
+  const url = app.restServer.url;
+  console.log(`Server is running at ${url}`);
+  console.log(`Try ${url}/ping`);


Remove this line too

dhmlau · 2020-03-23T20:18:42Z

FYI - The decrease in coverage complained by coverall are from other examples which I didn't change.

dhmlau added the Examples label Mar 16, 2020

dhmlau requested review from bajtos and raymondfeng as code owners March 16, 2020 18:40

dhmlau self-assigned this Mar 16, 2020

This was referenced Mar 17, 2020

[Spike] How to migrate Angular 1.x and 2+ LB3 client #4610

Closed

feat(docs): add validation docs #4874

Merged

dhmlau force-pushed the validation-example branch 2 times, most recently from 6def69d to a107e7a Compare March 18, 2020 02:02

dhmlau requested review from agnes512, emonddr, jannyHou, marioestradarosa and nabdelgadir as code owners March 18, 2020 17:39

dhmlau force-pushed the validation-example branch 2 times, most recently from adeb0f4 to 6d5f4d8 Compare March 18, 2020 17:40

dhmlau added the feature label Mar 18, 2020

emonddr suggested changes Mar 18, 2020

View reviewed changes

jannyHou reviewed Mar 18, 2020

View reviewed changes

raymondfeng reviewed Mar 18, 2020

View reviewed changes

examples/validation-app/src/interceptors/validate-phone-num.interceptor.ts Outdated Show resolved Hide resolved

jannyHou mentioned this pull request Mar 19, 2020

for verify the email after registration, I need email connector ? strongloop/loopback#2601

Closed

dhmlau force-pushed the validation-example branch 2 times, most recently from bb0fc66 to 38ed026 Compare March 20, 2020 00:10

dhmlau force-pushed the validation-example branch from 38ed026 to 9776f90 Compare March 20, 2020 14:06

emonddr suggested changes Mar 20, 2020

View reviewed changes

dhmlau force-pushed the validation-example branch from 20a5881 to a356baf Compare March 20, 2020 14:36

hacksparrow reviewed Mar 20, 2020

View reviewed changes

dhmlau force-pushed the validation-example branch from d4c2a06 to bf023e8 Compare March 20, 2020 15:03

emonddr approved these changes Mar 20, 2020

View reviewed changes

nabdelgadir reviewed Mar 20, 2020

View reviewed changes

dhmlau force-pushed the validation-example branch from 210849e to e8c8fd7 Compare March 23, 2020 16:48

nabdelgadir approved these changes Mar 23, 2020

View reviewed changes

feat(example-validation-app): add validation example

3c87a5e

dhmlau force-pushed the validation-example branch from 3b91e25 to 3c87a5e Compare March 23, 2020 17:38

chore: update package locks file

dfbe1cf

dhmlau requested a review from deepakrkris as a code owner March 23, 2020 17:45

raymondfeng approved these changes Mar 23, 2020

View reviewed changes

dhmlau merged commit c7a5cf7 into master Mar 23, 2020

dhmlau deleted the validation-example branch March 23, 2020 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(example-validation-app): add validation example #4895

feat(example-validation-app): add validation example #4895

dhmlau commented Mar 16, 2020 •

edited

Loading

dhmlau commented Mar 18, 2020

emonddr Mar 18, 2020

jannyHou left a comment

jannyHou Mar 18, 2020

dhmlau Mar 18, 2020

jannyHou Mar 18, 2020

dhmlau Mar 18, 2020

dhmlau Mar 18, 2020

raymondfeng Mar 18, 2020

jannyHou Mar 19, 2020

dhmlau Mar 19, 2020

raymondfeng Mar 18, 2020

raymondfeng Mar 20, 2020

dhmlau commented Mar 19, 2020

Arathy-sivan commented Mar 20, 2020

emonddr Mar 20, 2020

dhmlau Mar 20, 2020

dhmlau Mar 20, 2020

hacksparrow Mar 20, 2020

dhmlau commented Mar 20, 2020

nabdelgadir Mar 20, 2020

dhmlau Mar 20, 2020

nabdelgadir Mar 20, 2020

Arathy-sivan commented Mar 21, 2020

Arathy-sivan commented Mar 21, 2020 •

edited

Loading

achrinza commented Mar 21, 2020

nabdelgadir left a comment

nabdelgadir Mar 23, 2020

dhmlau commented Mar 23, 2020

		@@ -0,0 +1,57 @@
		// Copyright IBM Corp. 2020. All Rights Reserved.

feat(example-validation-app): add validation example #4895

feat(example-validation-app): add validation example #4895

Conversation

dhmlau commented Mar 16, 2020 • edited Loading

Checklist

dhmlau commented Mar 18, 2020

Choose a reason for hiding this comment

jannyHou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhmlau commented Mar 19, 2020

Arathy-sivan commented Mar 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhmlau commented Mar 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Arathy-sivan commented Mar 21, 2020

Arathy-sivan commented Mar 21, 2020 • edited Loading

achrinza commented Mar 21, 2020

nabdelgadir left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhmlau commented Mar 23, 2020

dhmlau commented Mar 16, 2020 •

edited

Loading

Arathy-sivan commented Mar 21, 2020 •

edited

Loading