#270 support prefix for producer transaction id and consumer group

[MagnusSmith]

Currently producer transaction id and consumer group are treated as literals. This PR addresses #270 by allowing a trailing * to denote a resource pattern type prefix

    consumers:
      - principal: "User:greta"
        group: "greta*"

and

    producers:
      - principal: "User:greta"
        transactionId: "greta*"

Since this is not changing the structure then there should not be any braking changes. Have added test cases.

[purbon]

Thanks a lot for your contribution! I appreciate it a ton! left only small cosmetic comments 👍

[purbon]

Nope! I mean have a master constructor where you set all class attributes and a secondary constructor that can call this main one.

…

-- Pere Missatge de Magnus Smith ***@***.***> del dia dc., 5 de maig 2021 a les 16:16:

***@***.**** commented on this pull request. ------------------------------ In src/main/java/com/purbon/kafka/topology/model/users/Consumer.java <#279 (comment)>: > @@ -18,6 +18,11 @@ public Consumer(String principal) { group = Optional.empty(); @purbon <https://github.com/purbon> just to confirm are you saying to remove the additional constructor Consumer(String principal, String group) so instead of Consumer consumer = new Consumer("User:foo", "foo*"); to call Consumer consumer = new Consumer("User:foo"); consumer.setGroup(Optiona.of("foo*"); or do you mean something else? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#279 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAQXPDAB6TPJ7M27PV6E63TMFHMJANCNFSM44DA4FDQ> .

-- Pere Urbon-Bayes Software Architect https://twitter.com/purbon https://www.linkedin.com/in/purbon/

[MagnusSmith]

@purbon In the Producer and Consumer class I see Optional field values. I think these could be better expressed by for example

public class Producer extends User {

  @JsonProperty("transactionId")
  String transactionId;

...

  @JsonIgnore
  public Optional<String> getTransactionId() {
    return Optional.ofNullable(transactionId);
  }
  
  public void setTransactionId(String transactionId) {
    this.transactionId = transactionId;
  }

What do you think?

[purbon]

I wonder about your proposal in the use of JsonProperty and JsonIgnore, can you explain why you use them? thanks a lot

[MagnusSmith]

I wonder about your proposal in the use of JsonProperty and JsonIgnore, can you explain why you use them? thanks a lot

It allows to separate the underlying instance variable from the getter by telling jackson to map the instance variable rather than the getter/setter. This allows us to keep the Optional getter but removes the need for Optional parameters which tend to be bad because they cause the caller to do unnecessary boxing, can still be null anyway, and don't work well with higher order functions forcing the caller to fallback to if statements. This might be something to consider later on as part of a wider code style refactoring.

[purbon]

nice, we can certainly do this in a clean-up! do you mind doing a new PR with this, and might be other nice improvements? I will, for now, merge this one, thanks a lot.

[purbon]

thanks a lot for your contribution!