fix(loopback4-example-shopping): add unique email constraint to user …

[dougal83]

…model

add unique index to email in user model in file user.model.ts

fix #103

[bajtos]

Thank you for the pull request. Can you add a test please?

[dougal83]

@bajtos Sorry, I will add tests and actually commit the controller additions. My mistake. Quick question before I resubmit. What would be your implementation recommendation, should I rely on the database to enforce constraint(as I originally intended) or should I search for existing email in controller?

[dougal83]

Since this is an example I opted to add check to the controller. Seemed simpler than to involve the db migrate functionality.

[bajtos]

We should rely on the database, that's the only reliable option.

The solution you are proposing at controller level has two problems:

(1) It's possible to write code that's calling this.userRepository.create (e.g. in a new controller method, or perhaps a script seeding database) and bypassing your check. At minimum, the check needs to be implemented in UserRepository.

(2) But even then, there is a race condition. If two requests to create a new user arrive at the same time, it's possible that they are handled in the following order:

1. Request1 arrives, sends count query to the database
2. Request2 arrives, sends count query to the database
3. count returns 0, Request1 proceeds to create a new user
4. count returns 0, Request2 proceeds to create a new user
5. BOOM. We have two users with the same email.

See also loopbackio/loopback-next#2331 where we are discussing the problem of referential integrity & unique constraints, and related items like loopbackio/loopback-next#2606 & loopbackio/loopback-next#2712.

[bajtos]

IMO, the solution in 994ba09 was the right one.

IIRC, we are using MongoDB to store the data and it is able to define unique constraints. So let's call automigrate from a before hook to ensure the database is properly set up for the tests.

[dougal83]

@bajtos PTAL

Few issues:

• Is it OK to assume the MongoError 11000 duplicate key is email at this point? (EDIT: added extra check)
• Is the test OK for double post or is there a better way to double post the same user?
• Would it be helpful to add migrate.ts and associated package script too or should I leave that out of PR? (EDIT: added extra commit JIC)

Also thank you for collating all the relevant posts. Very helpful.

[dougal83]

Right I'll stop fiddling, lack of precision was irking me. Refactored to use a named index (if there is a better way please enlighten me): 3c76834.

[bajtos]

Awesome 👏