Repository navigation
task: using a beanparam for client listing options - #49074
Conversation
There was a problem hiding this comment.
Thanks Steve! I think this is a step in the right direction as more and more parameters are being added.
I may be missing something, thus the suggestion below is just a comment (and not a request change) to help myself to explain things better. Two questions:
BuilderextendsListOptionsmeaning theBuilderis aListOptions. Thebuild()method then returns this (theBuilder), not a new immutableListOptionsinstance. Doesn't this defeats the purpose?- a caller can do
builder.build()and then still callbuilder.withFields(...). Doesn't this mutates the same object? (it's the same instance, right?)
It got lost in the shuffle somewhere, but I thought I had already made a comment on the issue or the pr that this was just a "dummy" builder, and that I'd like to see if we could just use lombok, FreeBuilder, or even sundrio instead of crafting one. As for the convention for the static method and the usage of the with prefix, I have no strong opions. I mostly wanted to have us get something in, before we started down the path of adding a bunch of default methods or null arguments.
Edit: sorry for the stream of consciousness here - I'm making things more complicated that it needs to be. I don't think we need to ensure that ListOptions are immutable and a full blown source generating annotation while nice isn't entirely needed. How about just adding fluent methods, which align with @Pepo48's suggestions? Also I had forgotten just much boilerplate sundrio brings in, so that is a non-starter. |
closes: keycloak#48650 Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Signed-off-by: Steve Hawkins <shawkins@redhat.com>
* task: using a beanparam for client listing options closes: keycloak#48650 Signed-off-by: Steve Hawkins <shawkins@redhat.com> * just adding fluent methods Signed-off-by: Steve Hawkins <shawkins@redhat.com> --------- Signed-off-by: Steve Hawkins <shawkins@redhat.com>
closes: #48650
Uses a beanparam with a builder, rather than adding more direct parameters to the getClients end point. Ideally this will be reused across all of our resource types for projection, querying, offset, and ordering. I'm not sure about limit as we may need a different default or hard maximum per resource type and may want that metadata to on the parameters, not the method.
If we try to more kube or scim like, the ClientApi, and ClientsApi would similarly be abstracted and parameterized around the type. However that also implications for generating the metadata or providing discovery services which are not currently in scope for admin api v2.