Skip to content
This repository has been archived by the owner on Mar 16, 2022. It is now read-only.

TCK: order of state action updates for addItem by the value entity ShoppingCart #490

Closed
marcellanz opened this issue Nov 19, 2020 · 4 comments · Fixed by #492
Closed

TCK: order of state action updates for addItem by the value entity ShoppingCart #490

marcellanz opened this issue Nov 19, 2020 · 4 comments · Fixed by #492

Comments

@marcellanz
Copy link
Contributor

The com.example.valueentity.shoppingcart.ShoppingCart implements addItem in a way that returns state action updates in the order so that the updated line-item, if it has existed, is positioned at the end of the state action updates list:

This is originated because a shopping cart line-item that already exists is removed and then re-added with the newly updated quantity:

https://github.com/cloudstateio/cloudstate/blob/master/samples/java-shopping-cart/src/main/java/io/cloudstate/samples/shoppingcart/ShoppingCartEntity.java#L58-L59

    Domain.Cart cart = ctx.getState().orElse(Domain.Cart.newBuilder().build());
    Domain.LineItem lineItem = updateItem(item, cart);
    List<Domain.LineItem> lineItems = removeItemByProductId(cart, item.getProductId());
    ctx.updateState(Domain.Cart.newBuilder().addAllItems(lineItems).addItems(lineItem).build());

So adding 2 times product:2 then 11 of product:1 and then 31 of product:2 emits the following state as ValueEntityUpdate:

command: entity_id:"cart:1" id:3 name:"AddItem" payload:{[type.googleapis.com/com.example.valueentity.shoppingcart.AddLineItem]:{user_id:"cart:1" product_id:"product:2" name:"Product2" quantity:2}}, state: [type.googleapis.com/com.example.valueentity.shoppingcart.persistence.Cart]:{items:{productId:"product:1" name:"Product1" quantity:1} items:{productId:"product:2" name:"Product2" quantity:2}}
command: entity_id:"cart:1" id:4 name:"AddItem" payload:{[type.googleapis.com/com.example.valueentity.shoppingcart.AddLineItem]:{user_id:"cart:1" product_id:"product:1" name:"Product1" quantity:11}}, state: [type.googleapis.com/com.example.valueentity.shoppingcart.persistence.Cart]:{items:{productId:"product:2" name:"Product2" quantity:2} items:{productId:"product:1" name:"Product1" quantity:12}}
command: entity_id:"cart:1" id:5 name:"AddItem" payload:{[type.googleapis.com/com.example.valueentity.shoppingcart.AddLineItem]:{user_id:"cart:1" product_id:"product:2" name:"Product2" quantity:31}}, state: [type.googleapis.com/com.example.valueentity.shoppingcart.persistence.Cart]:{items:{productId:"product:1" name:"Product1" quantity:12} items:{productId:"product:2" name:"Product2" quantity:33}}

with the items: ordered as shown. This results in the TCK testing this order by the specifics of this implementations detail:
https://github.com/cloudstateio/cloudstate/blob/master/tck/src/main/scala/io/cloudstate/tck/CloudStateTCK.scala#L1390-L1412

We could

  • mandate the TCK addItem test case for the shopping cart to be mandated like this
  • allow the TCK to match list items independently of their order
  • mandate that the order of the carts state for addItem, and any other, is the order in which the items where added and or removed.

I think I would opt-in for the last option, whereas the second option also could be helpful for certain instances, where the order is not important at all and would not fit a languages idiomatic implementation.

@pvlugter
Copy link
Member

Yes, agree that there should either be an order mandated, or the test should allow for out-of-order. Order used could be insert order, or could be sorted by product id.

For the CRDT tests in the TCK, I've added a requirement that responses with current state for Sets and Maps have these sorted (and there's a simplification in the tests that Set values and Map keys are strings). This makes it easy for the tests to just compare received message values directly. And then for the added or removed sets in the deltas themselves, these get transformed on the receiving side, to sort these before doing simple direct comparisons — found it easier to transform deeper parts of the message into an expected order, than do order-independent comparisons across parts of the message.

@pvlugter
Copy link
Member

I've added a quick fix for this in #492. Requires ordered by product id now.

@marcellanz
Copy link
Contributor Author

Cool, I agree.

@ralphlaude
Copy link
Contributor

Cool with the order in shopping cart by product id and more generally for the TCK tests

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants