Proposal to use `DB jsonb` with the JSON schema validation
Proposal
We started recently adding first columns with jsonb
into GitLab Database.
We do prefer a well structured data: columns with know types.
However, there are cases where jsonb
is more suitable. One of such examples is
a complex semi-unstructured data, or key-value type of storage. In such cases it
might be more advisable to use jsonb
as it offers significantly more flexibility.
However, this moves us into unstructured documents as JSON can hold any data.
I believe we should ensure that each jsonb
column has a Rails JSON schema validation
to ensure that we always store a document of a known format instead of gibberish.
Even, if such validation errors would not be presented to user, but for example to Sentry.
This is based on premise that knowing the structure makes it easier for us later to:
- Ensure that this structure is consistent
- We do version JSON schema being used, likely adding new fields
- Makes it easier to translate JSON into a Ruby object if this requires that
- Allows any other person to see a structure supported looking at schema validation
For that purpose we can use the same approach that is taken by an existing json request matcher that we use for validating structure of json endpoints.
Proposal
- Each new
jsonb
column if used by model should have associatedjson_schema
matcher
Check-list
-
Make sure this MR enables a static analysis check rule for new usage but ignores current offenses -
Mention this proposal in the relevant Slack channels (e.g. #development
,#backend
,#frontend
) -
The MR doesn't have significant objections, and is getting a majority of 👍 vs👎 (remember that we don't need to reach a consensus) -
Create a follow-up issue to fix the current offenses as a separate iteration: ISSUE_LINK -
Follow the review process as usual -
Once approved and merged by a maintainer, mention it again: -
In the relevant Slack channels (e.g. #development
,#backend
,#frontend
) -
(Optional depending on the impact of the change) In the Engineering Week in Review
-