View Effects implementation

[johanandren]

On top of #230

Refs #52

Dropped annotations, handcoded Provider and Handlers in one sample and the TCK (which passes)

[patriknw]

I merged #230 so this can be rebased now

[patriknw]

looking good
I thought you said that you had codegen for this also, not pushed yet, or I misunderstood?

[johanandren]

Started on codegen but nothing pushable or at least reviewable yet, and got stuck with other things today so haven't made any progress there.

[johanandren]

One thing left now, I think: how to deal with views without transform updates. I'm thinking if we should generate a separate Provider for those that can be used with .register without needing a handler/actual view etc. Alternative could be that we generate an empty View implementation (no update handlers) and everything else looks the same but then there'd still be a dummy emptyState etc.

[patriknw]

looking good

[patriknw]

Alternative could be that we generate an empty View implementation (no update handlers) and everything else looks the same but then there'd still be a dummy emptyState etc.

I like this alternative. Would be good also for the upcoming transform_results combination.
emptyState can be as it is with UnsupportedOperationException since it will not be called.

[johanandren]

I think just picking the first command output type as the state type is not quite safe, one scenario where it will break that I can think of is a view with no transform and only a query that "projects" the result into some other type (unless I remember wrong that we support that).

I don't have a great idea for a more foolproof way to figure out the state type though.

[patriknw]

we have a validation for the return types, if that is the case you think about: AS-00109 All View update methods must have the same return type

[johanandren]

Hmm, maybe I'm misinterpreting that the commands here will include both queries and update method - was thinking that the first command (not sure how they are ordered) could be the query, and that could be a projection into some non-state type.

[patriknw]

ah, I understand, you are right if commands includes both update methods and query methods. it should look for a update method

[johanandren]

Did something about that now, looking only at the first update method (with some test coverage). Not sure if there may still be cases I have not thought about.

[johanandren]

I found one issue that is messed up but that I don't think surfaces with the current enabled samples compiled/tested and that is the de-duplication of names where it tries to add Impl or such to the name to avoid it clashing with the classes generated by protobuf.

Right now that turns service ShoppingCartViewService into ShoppingCartViewServiceView in the java-eventing-shopping-cart sample, and if I rename it to ShoppingCartView which makes sense, I get ShoppingCartViewImplView generated which is even weirder.

[patriknw]

related to the View/Impl naming is #238

[johanandren]

Ready for final review. java-eventing-shopping-cart is broken, but it also is regardless of this PR, and not part of what's validated by CI for the sdk-codegen-dev-branch so let's fix that later all at once.

[patriknw]

LGTM