Skip to content

Bug with using json_schema validation with jsonb_accessor

Summary

Validation of a model's jsonb_accessor attributes does not behave as expected when attempting to validate with json_schema. Since the json_schema validation uses the entire attribute, values are coerced before validation is run which causes validation to pass when an invalid data type is passed.

Another issue is that some attributes that are exposed via jsonb_accessor have rails validators defined. Maintaining two different validators for these settings (JSON schema and traditional rails validators) seems to add unnecessary complications.

Steps to reproduce

[38] pry(main)> ::Gitlab::CurrentSettings.nuget_skip_metadata_url_validation
=> false
[39] pry(main)> ::Gitlab::CurrentSettings.update!(nuget_skip_metadata_url_validation: 'not-a-bool')
=> true
[40] pry(main)> ::Gitlab::CurrentSettings.nuget_skip_metadata_url_validation
=> true

Example Project

What is the current bug behavior?

Type validation is not performed when validating with JSON schema because jsonb_accessor coerces values before the validation step.

What is the expected correct behavior?

A validation error should occur when variable types do not match those defined in JSON schema.

[45] pry(main)> ::Gitlab::CurrentSettings.update!(nuget_skip_metadata_url_validation: 'not-a-bool')
ActiveRecord::RecordInvalid: Validation failed: Package registry value at `/nuget_skip_metadata_url_validation` is not a boolean

Possible fixes

  1. Removing the data type definition results in jsonb_accessor not performing type coercion and allows validation to behave as expected when validating with JSON schema. We could write a cop to ensure that type definitions are not included when adding attributes to jsonb_accessor and require validation to be handled by JSON schema. This would also allow us to cleanup a lot of the validations in the models and move them to the json schema validation file.

  2. Remove json_schema validation for jsonb_accessors in models and instead rely on regular validators for the attributes. This solution would result in a lot more code in the model files so I would prefer keeping the json schema validation over this option.

  3. We could do away with using jsonb_accessor altogether and use attribute and store_accessor to interact with JSON objects similar to what is done for UserDetail. The downside is we would lose the ability to define a default so more engineering work would be needed to create a solution to allow for default values when using store_accessor. This would essentially re-create the solution in option 1 so I don't think it is worth the extra effort.

Edited by Ian Anderson