Discussion: make ProductData getCurrent() return Optional

[schleichardt]

• [ ] clarify what differs from the API in the SDK

ProductCatalogData return not an Optional for the current product data. If I create a product in the backend the current part is already filled with the staged stuff but the flag published is false. I think getCurrent() should only be used if isPublished is true, therefore it may be better to make getCurrent() return an Optional?

What do you think @gogregor @lauraluiz ?

[lauraluiz]

So it is going to be a different behaviour from the API? Would you also return absent when the product was unpublished?

[schleichardt]

exactly: if isPublished => the content if no isPublished or unpublished => Optional.empty(), even if the JSON for current was delivered.

[lauraluiz]

It makes sense if I think about the system alone. But what I don't like is the different behaviour that the SDK is offering versus the API. Should we "correct" the API behaviour in some way? Or should we limit us to do what the API does in the first layer of the SDK (the 1:1 layer)? And this question is general for any future case.

[schleichardt]

I want the SDK very API near*, but I want to prevent users from traps. And using current projection without is published is a trap.

• Sometimes I provide classes/methods with a more intuitive alias.

[lauraluiz]

Yes, I think some harmless interpretation is not bad. I want to agree on the level of interpretation though. Also it would be good to clearly state in the SDK documentation in what the methods/classes differ from those of the API, to help API users that move to a SDK.

[schleichardt]

Okay, But I still want to read @gogregor thinks about it.

[gogregor]

In general, the SDK can differ from our HTTP API data models.

In this case I also agree that it would make sense for current to return empty optional if a product is not published.

[schleichardt]

needs documentation