-
Notifications
You must be signed in to change notification settings - Fork 197
feat: add an api to add items to a cart atomically #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,14 +4,69 @@ | |
| // License text available at https://opensource.org/licenses/MIT | ||
|
|
||
| import {DefaultKeyValueRepository} from '@loopback/repository'; | ||
| import {ShoppingCart} from '../models/shopping-cart.model'; | ||
| import {ShoppingCart, ShoppingCartItem} from '../models/shopping-cart.model'; | ||
| import {RedisDataSource} from '../datasources/redis.datasource'; | ||
| import {inject} from '@loopback/context'; | ||
| import {promisify} from 'util'; | ||
|
|
||
| export class ShoppingCartRepository extends DefaultKeyValueRepository< | ||
| ShoppingCart | ||
| > { | ||
| constructor(@inject('datasources.redis') ds: RedisDataSource) { | ||
| super(ShoppingCart, ds); | ||
| } | ||
|
|
||
| /** | ||
| * Add an item to the shopping cart with optimistic lock to allow concurrent | ||
| * `adding to cart` from multiple devices | ||
| * | ||
| * @param userId User id | ||
| * @param item Item to be added | ||
| */ | ||
| addItem(userId: string, item: ShoppingCartItem) { | ||
| const addItemToCart = (cart: ShoppingCart | null) => { | ||
| cart = cart || new ShoppingCart({userId}); | ||
| cart.items = cart.items || []; | ||
| cart.items.push(item); | ||
| return cart; | ||
| }; | ||
| return this.checkAndSet(userId, addItemToCart); | ||
| } | ||
|
|
||
| /** | ||
| * Use Redis WATCH and Transaction to check and set against a key | ||
| * See https://redis.io/topics/transactions#optimistic-locking-using-check-and-set | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @raymondfeng IIUC, the algorithm outlined in the linked page is expecting to re-run the transaction if a race condition was detected, and repeat those re-run until the transaction succeed.
I don't see that logic implemented here. Are we expecting the REST clients to retry the operation? Could you please explain how are we signaling the race condition to them? |
||
| * | ||
| * Ideally, this method should be made available by `KeyValueRepository`. | ||
| * | ||
| * @param userId User id | ||
| * @param check A function that checks the current value and produces a new | ||
| * value. It returns `null` to abort. | ||
| */ | ||
| async checkAndSet( | ||
| userId: string, | ||
| check: (current: ShoppingCart | null) => ShoppingCart | null, | ||
| ) { | ||
| const connector = this.kvModelClass.dataSource!.connector!; | ||
| // tslint:disable-next-line:no-any | ||
| const execute = promisify((cmd: string, args: any[], cb: Function) => { | ||
| return connector.execute!(cmd, args, cb); | ||
| }); | ||
| /** | ||
| * - WATCH userId | ||
| * - GET userId | ||
| * - check(cart) | ||
| * - MULTI | ||
| * - SET userId | ||
| * - EXEC | ||
| */ | ||
| await execute('WATCH', [userId]); | ||
| let cart: ShoppingCart | null = await this.get(userId); | ||
| cart = check(cart); | ||
| if (!cart) return null; | ||
| await execute('MULTI', []); | ||
| await this.set(userId, cart); | ||
| await execute('EXEC', []); | ||
| return cart; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the partial needed in this case since this isn't a model directly exposed over
RESTand won't face issues of trying to create an instance without aproductId?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a big deal as the
dataitself is optional. So we can create a empty instance and populate the data afterward. Therequiredconstraint should be enforced by model validation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raymondfeng Please note that KeyValue repository does not perform validation right now! See lib/kvao/set.js. I think this is an oversight we should eventually fix.