-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhance SelectionSet
annotation to ignore fields in the return type
#27
Comments
@fabiocarvalho777 Can you specify how the user will specify ignore for inner values? For eg: if response type is For eg can we specify ignore to be complexField somehow which should ignore all its inner values? |
{
booleanVar
complexField {
innerBooleanVar
innerIntVar
innerStringVar
}
intVar
stringVar
} Given the response type above, to ignore
Yes, we are. If we want to ignore Let me know if you have other questions. Thanks. |
Thanks for clarifying @fabiocarvalho777 |
Why don't we simply not allow this instead of exposing our users to runtime exceptions? Is a different annotation design (e.g. 2 different annotations) not an option? I don't know that much about 'selection sets', so forgive my ignorance.. Just trying to help with the PR review. |
Having two different annotations could be an option. There are some cons in this approach too. For example, what if the user uses both annotations at the same time? Wouldn't we have to check and prevent that in runtime anyways? How would we name both annotations? Could it be confusing for users to have two instead of one? Could it make the logic that process the annotations more complex? So, when I think about the pros and cons, I think having one annotation sounds better, although not ideal. What do you think? |
I definitely think we can run with this as-is and let users complain. Of your questions, the first one gives me the most pause. I am assuming we could come up with good names, but that worries me a bit as well. Honestly, I was thinking more about types (classes) when I made those comments. I haven't built many things where I leveraged annotations. Anyhow, if we ever revisit, here are some options that could be considered:
Again, I don't think this is required. I have a mild itch to investigate, but sadly time will likely not permit it. |
Add a new use case for
SelectionSet
annotation where users can still let Mocca resolve the selection set automatically, using the response DTO declared in the method return type, however, ignoring fields specified in a new attribute, calledignore
, to be added toSelectionSet
.SelectionSet would then behave like this:
value
attribute is set, Mocca automatic selection set resolution is turned off, andSelectionSet
value
is used to define the selection set. In this case ifignore
value is also set, then that is not used by Mocca, and a warning is logged.value
attribute is NOT set, butignore
is, then Mocca automatic selection set resolution is turned ON, andSelectionSet
ignore
is used to pick which response DTO fields to ignore from the selection set.value
andignore
attributes are NOT set, then aMoccaException
is thrown.Important Note
The fields marked to be ignored must NOT be present in the selection set defined in the request message! This might sound obvious but it is important to emphasize it to make sure the code changes pushed for this issue don't remove those fields only after the server already returned a response with them.
Acceptance Criteria
The text was updated successfully, but these errors were encountered: