note previously we were inconsistent, with NON_NULL used some places,
NON_EMPTY elsewhere, and no exclusions elsewhere, and in many places
without much thought. so we have three options:
1 - dogmatic - remove all `@Include(NON_*)` annotations
2 - compatibility-first - restore anything changed since 0.12.0
3 - forward-looking - leave as is, with documentation added that
empty/null fields may not be included in json response objects, and
release note that this has been applied in more places
(note that 1 is technically a breaking change if i had code that
expected not to see a field unless it was non-empty ... it will probably
break tests)
i tend to think API consumers impacted by this are small and can take
the pain, and better to have a nice set of API response objects for new
users. and when i see lots of nulls and empties in an api response
object i think the designers care more about dogma than human users. i
don't expect the v2 rest api will land any time soon.
so i am strongly for option 3, taking the ui pain now, but i'll defer if
i'm a singleton
--a
On 08/11/2017 11:35, Geoff Macartney wrote:
My tuppence worth - agree with Thomas, it looks like a change best done in
a V2. Since it's actually a real breakage for the API it would be at least
worth passing through a deprecation cycle, but as Alex points out there's
no efficient way to do that. The cost of restoring ugliness in the response
objects at least keeps the pain away from client API consumers.
G
On Wed, 8 Nov 2017 at 11:22 Graeme Miller <gra...@cloudsoft.io> wrote:
Hello,
I agree with Thomas here. This seems like an API breaking change, and
should be reserved for V2 if it at all.
I lean towards reverting.
Regards,
Graeme
On 8 November 2017 at 10:01, Thomas Bouron <
thomas.bou...@cloudsoftcorp.com>
wrote:
Hi all.
I'm not a fan of excluding fields from the JSON payload, if empty, for
few
reasons:
1. this is a breaking change for the UI and CLI which will be time
consuming to fix (very fiddly to guard against this in JS for example)
2. this makes it harder to design clients, because you need to guard
against the presence of those fields. The swagger page displays all
fields
leading the user/dev to think that they are always returned in the
payload
3. I don't think it saves bandwidth to remove a `constraints: []` from
the
final JSON. If you want to have a smaller payload, I would much prefer to
set a query string flag to restrict (or expand) the full body, something
like `?summary=true` => returns only necessary information.
Now, my comments apply for the API endpoints under /v1 only. I'm all in
favour of break things with a v2 API though.
Best.
On Mon, 6 Nov 2017 at 15:17 Alex Heneveld <
alex.henev...@cloudsoftcorp.com
wrote:
Hi All-
Until recently our REST API returned full records in most cases,
including often lots of empty lists and maps and sometimes nulls --
such
as `constraints: []` on all config keys.
The widespread preference in REST / JSON community seems to be to omit
these unless there is a very good reason for having them, e.g. Google
JSON Style Guide [1].
Recently in replacing deprecated FasterXML annotations with newer ones
many fields were changed to be included only if NON_EMPTY, in line with
that preference, but this is technically a breaking change. And it's
not just technical, as there are a few places in UI code which assume
fields exist which now do break. These are easy to fix once they're
encountered at runtime but tedious to ensure no problems at design
time.
Thus we have two choices:
* Yes, make this break now. This (v1.0) is probably the best time.
There is no efficient way to pass this through a deprecation cycle.
Cost
is adding to release nodes and REST API client breakages and fixes.
* No, revert any `Include(NON_EMPTY)` annotations recently introduced
to
be strict about backwards-compatibility here. Cost is restoring old
behaviour and bloat/ugliness in the API response objects.
I have a preference for "Yes", roll-forward and tolerate some breakages
with the shift to 1.0.
Best
Alex
[1]
https://google.github.io/styleguide/jsoncstyleguide.
xml#Empty/Null_Property_Values
--
Thomas Bouron • Senior Software Engineer @ Cloudsoft Corporation •
https://cloudsoft.io/
Github: https://github.com/tbouron
Twitter: https://twitter.com/eltibouron
