Skip to content
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

A few questions about schema validation #22

Open
carlost opened this issue Jun 20, 2024 · 4 comments
Open

A few questions about schema validation #22

carlost opened this issue Jun 20, 2024 · 4 comments

Comments

@carlost
Copy link

carlost commented Jun 20, 2024

I ran across a few things I was a little unsure about while integrating easy_talk into our app. I was hoping you have the time to answer a few questions I had:

JSON Validation

The readme mentions that "EasyTalk does not yet perform JSON validation". Can you confirm that this is no longer the case?

I think this was resolved when you added json-schema in release 0.1.2: 7416466#diff-db2f18104e8129484ced3b2cbe64479d338d247094016629bc8c2682b5cfd93dR85-R87

I just want to make sure I am understanding this correctly.

Unexpected root level attributes raises ActiveModel::UnknownAttributeError

Using spec/easy_talk/schema_validation_spec.rb as a reference, this passes:

it 'errors on a non-existent property' do
expect do
user.new(surprise: 'yah-got-me', name: 'Jim', age: 30, height: 5.9, email: { address: '[email protected]', verified: false })
end.to raise_error(ActiveModel::UnknownAttributeError, /unknown attribute 'surprise'/)
end

This occurs even if the "user" Class has additionalProperties explicitly set to true.

However, this does not occur if the unexpected attribute is nested. For instance:

user.new(name: 'Jim', age: 30, height: 5.9, email: { surprise: 'nested k/v', address: '[email protected]', verified: false })

Does not raise an error, and its validity is dependent on additionalProperties.

Is this behavior expected? The behavior feels a little surprising because it isn't implied by the API/docs and goes against the expectation when explicitly setting "additionalProperties true". This feels like it is just an unintended consequence of including ActiveModel::API in EasyTalk::Model, which replaces the default initializer with AR's initializer. ActiveModel::API#initialize freaks out if it is passed an unexpected root level attribute.

validation errors for errors and validation_context when additionalProperties is set to false

When additionalProperties is set to false, it looks like the JSON validation is treating the validation_context and errors attributes added by ActiveModel::Validations as if they are values submitted by JSON.

["Validation context object property at /validation_context is a disallowed additional property", "Errors object property at /errors is a disallowed additional property"]

This feels like a bug. Is this behavior expected?

Is there a way to nest the JSON schema definition for the contents of arrays?

When the JSON schema assigns an object to a property, you can specify the the schema for the nested object inline as:

property :email, :object do
property :address, String
property :verified, T::Boolean
end

Is it possible to do this when the property is an array of objects? Something like:

property :emails, T::Array do
property :address, String
property :verified, T::Boolean
end

I also wanted to just thank you for publishing easy_talk. We were able to finally get AWS Bedrock reliably generating the JSON output we needed because of it!

@sergiobayona
Copy link
Owner

Thanks for this useful feedback Carlos. Regarding json schema validation, the latest version of the gem (v0.2.2) does validate json schema. It piggybacks on the json-schemer gem and uses the ActiveModel api to report back the errors. Take a look at:
https://github.com/sergiobayona/easy_talk/blob/main/spec/easy_talk/schema_validation_spec.rb
https://github.com/sergiobayona/easy_talk/blob/main/spec/easy_talk/activemodel_integration_spec.rb

@sergiobayona
Copy link
Owner

regarding the second part of your message:

"validation errors for errors and validation_context when additionalProperties is set to false"

It looks like you found a bug. I'm creating a bug issue and will look into it soon.

@sergiobayona
Copy link
Owner

as far as nesting an object as an array, yes it possible. You'd define the nested object as a model. Then reference it as a T::Array[Object] where the Object is the name of the model. Like:

class Email
  include EasyTalk::Model

  define_schema do
    property :address, String
    property :verified, T::Boolean
  end
end

class User
  include EasyTalk::Model

  define_schema do
    property :emails, T::Array[Email]
  end
end

Here is a similary spec https://github.com/sergiobayona/easy_talk/blob/main/spec/easy_talk/examples/user_model_spec.rb#L107-L122
for reference.

@carlost
Copy link
Author

carlost commented Jun 26, 2024

@sergiobayona thanks for looking into this. The only other question I had was related to the ActiveModel::UnknownAttributeError raised when passing the model's initializer a hash with an unexpected root level attribute. The difference in how the model treats an unexpected root level attribute by raising an error while an unexpected nested attribute is flagged as a validation error is a little surprising.

Is this behavior expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants