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

How the error information is represented if the resource validation fails? #82

Closed
millisami opened this issue Dec 3, 2013 · 21 comments
Closed

Comments

@millisami
Copy link

Since I didn't find any place to ask and its not a bug, just want to know the way out.

I am using roar with roar-rails in my rails app with the HAL+JSON media type.

My question is, if a resource save fails with the validation errors, how to represent it in HAL? Or maybe I am not understanding the Hypermedia in true sense.
Can anyone put some light on this?
Sample code in context of a bare bone Rails app would be much better?

@andresf
Copy link
Contributor

andresf commented Dec 3, 2013

There are many ways in which you can represent errors; it ultimately depends on what your use case is.

Jørn Wildt wrote a great piece about REST and error handling, which should guide you in the right direction.

In the context of Roar and Rails, I've modified ActiveModel::Errors to include more information about validation errors, such as the key that failed and the error code. I just realized I'm including HAL into the representers, but the output is not really HAL at all -- it's a different media type just for errors. Feel free to take whatever is useful from that approach.

@apotonick
Copy link
Member

What if we had a feature Errors that you include into your representer to achieve displaying errors conditionally in a standardized manner?

module SongRepresenter
  include Roar::Representer::JSON

  include Roar::Representer::Feature::Errors

The Errors feature would roughly do the following (pseudo-code):

  collection :errors, if: errors.any?, extend: JSON::ErrorsRepresenter

We can basically implement our own errors specification. What do you include into your errors representation, @andresf ?

@andresf
Copy link
Contributor

andresf commented Dec 3, 2013

Right now I'm including a lot of fluff into the error messages, mostly because we need to support internationalization, and we have external developers coding against the API. The actual representable model is at the bottom of this gist, but we basically include:

At the error collection level:

  • A copy of the http status code (flash and others can't read http codes directly)
  • A timestamp (for internal logging / tracking)

At the individual error level:

  • Localized, end-user message ("The email provided is invalid")
  • Localized, end-user attribute ("Email")
  • Attribute key ("user_email")
  • Error code for the developer, so he can decide how to proceed (can be a URI that identifies the error + gives documentation when followed, can be an API-specific error code like "insufficient funds", etc)
  • Details of the error intended for the developer (anything that can help him debug or figure out why the error happened -- in practice this is not used much)

IMO, at a minimum, an error representation should include:

  • A user-facing error message
  • The field that failed validation
  • An error code that identifies the validation that failed (in Rails: :too_short, :too_long, :invalid)

...unfortunately Rails atm does not give you access to the validation error code, so I opened a PR fixing the problem based on this conversation. It was unfortunately largely ignored, but to be honest I haven't pushed for it either. Perhaps I should go do that?

At any rate, 👍 to the idea of having a default Feature::Errors to build on top of. This is definitely a friction point around HAPIs, so anything that helps is time well-spent.

@apotonick
Copy link
Member

That looks great, I believe we can reuse a lot of your code.

@millisami
Copy link
Author

Guys, thanks for looking this up.
@andresf I looked at the gist you provided and I guess its already included by the roar-rails gem.

But can you put me an example on how to use that feature?
Should I include it or by just calling resource.errors will give me that output?

It would be better if you can put up on what I've right now.

module Api::V1::MachineRepresenter
  include Roar::Representer::JSON::HAL

  link :self do
    api_machine_url(self)
  end

  property :id
  property :name, from: 'name'
  property :sub_type
  property :manufacturer
  property :model
  property :year
  property :vin
  property :license

end

@apotonick
Copy link
Member

Where is that in roar-rails? haha

@andresf
Copy link
Contributor

andresf commented Dec 4, 2013

@millisami sorry to confuse you, I did not explain myself correctly.

The patch I linked above is not part of roar-rails; it must be included as an initializer in rails in order to overwrite ActiveModel::Errors. What the patch does is allow you to call to_representable on any ActiveModel::Errors object, which can then be used by the sample representers I provided to return errors.

A complete (if silly) example:

def update
  stuff = Stuff.find(params[:id])

  if stuff.update_attributes(params)
    render json: stuff.extend(StuffRepresenter).to_json
  else
    render json: stuff.errors.extend(Representers::Errors::Collection).to_json
  end
end

In practice I don't recommend actually exposing your database models as your API.

@andresf
Copy link
Contributor

andresf commented Dec 4, 2013

It's also worth noting that you do not need the ActiveModel::Errors extension if you don't care about the "code" of the validation that failed in your error representation. The representer would have to change, though, to match the interface of the Errors objects.

@bethesque
Copy link

Something I've been wondering about recently is the difference between API validation and model validation. For example, the field names of your model may not map exactly to the field names of your API - I tend to expose keys in camel case, rather then the ruby underscore because JSON should be language agnostic, and I feel that underscores are quite a ruby specific thing. It's also quite likely that the database representation (which a model often maps to) is different from the way you want to represent the object in the API. In both of these cases, the model errors do not necessarily give you a direct mapping back to the fields that caused the errors.

So this leads me to ask - would it make design sense to put validation in the Roar mappings themselves?

module SongRepresenter
  include Roar::Representer::JSON
  property :first_name, as: :firstName, read_required: true
  property :age, read_required: true, read_type: Fixnum
  property :dob, as: dateOfBirth, read_pattern: /\d\{2}-\d{2}-\d{4}/
end

I say this because I'm using Webmachine with Roar (both libraries that I think are fantastic), but the API validation has kind of fallen in the cracks between them, and I'm not sure where it should belong. It doesn't make sense to me to create a 3rd place (Model, Representor, Validator) that a field should be listed in, which leads me to think that the Representor may be an appropriate place. However, I am also aware that a library should do one thing and one thing well, and not try to be all things to all people, or it will do nothing well!

@apotonick
Copy link
Member

Very good point!

The solution is to use an Operation from Trailblazer (usable in Webmachine, of course) or do it yourself using a Reform form with included JSON support.

This will allow you parsing and rendering models via the form/operation. The cool part here is that the form contains validations, and representable does the rendering/parsing part. I hope this is what you're after.

class SongForm::JSON < Reform::Form
  include JSON
  property :title

  validates :title, presence: true
end

It doesn't exactly work like the above, yet, but that's a matter of days (as always 😉 ).

You then parse the JSON via the form.

SongForm::JSON.new(song).validate("....json...")

@bethesque
Copy link

I think the Reform solution would fit best with the design I was thinking of. A bit of a drag that each attribute has to be mentioned in 3 places (form, representor, model) but I can see the reason for the separation of concerns. Muddling the concerns is that made Rails the mess for long term development that it has turned in to.

Where should I look for an announcement that the "matter of days" work has been done? I have a codebase that is under active development that I am using as an example of how to use Webmachine+Roar, and I would like to use Reform when it is available, as I am not happy with my current model validations being used in the Webmachine resource.

@apotonick
Copy link
Member

Oh, you absolutely do not repeat representers. I invested quite some days into making representers and forms interchangeable to the max, meaning you can define structure schemas in modules and then include them in representers and forms (and fine-tune them, of course).

I will start working on gemgem's API functionality soon, and then push some changes to Reform. Sharing forms and representer structures already works, it's just a bit clumsy ATM.

As a side-note, I believe a lot of thoughts and work will go into this interchangeability (what a word) since this is the future of Trailblazer: We all love the explicitness of the framework, so I want to make sharing and customizing schemas as simple and streamlined as possible.

Follow me on Twitter/add me on Skype for annoucements.

@bethesque
Copy link

I like the sound of reuse via modules. Looking forward to seeing it all come together. Have added you on twitter.

@apotonick
Copy link
Member

I'd LOVE you to start playing with it now! The sooner I get feedback, the better.

For your understanding: The form simply uses representable in #validate, populates the object graph and then runs the validation. It is cleanly separated although it looks as if mixed together from the outside.

@bethesque
Copy link

Sure, am I working from master on https://github.com/apotonick/reform?

@apotonick
Copy link
Member

AAaargh.... yes... sorry I didn't think of that! Also, representable/master. I know you coder kids love livin' on the edge! 😉

@bethesque
Copy link

Checked out All The Repos, had a go: bethesque/pact_broker@486ba07

I'm using Decorators, not Representors, will the logic be sharable between a Decorator and a Contract/Form? Btw, I'm a big fan of Decorators. Maintaining a gem myself, I have come to know the pain caused by other libraries mixing in things in unexpected ways (and stomping all over things in unexpected ways. Here's looking at you ActiveSupport). Wrapper classes are so much cleaner and more reliable.

@apotonick
Copy link
Member

...and faster! I love decorator way better than the modules.

I am pretty sure you can simply go with your form classes and then do rendering and parsing from the form (which will delegate to representable).

If you want to have different forms and representers per concept, you can define shared schema in a module and then include that schema in representer and form (where the representer is a Decorator).

@bethesque
Copy link

So, does this mean that the Decorator is not needed any more? Does Form do Decorator+Validation now? Got a gist?

@apotonick
Copy link
Member

The form will internally set up a decorator for you using the schema you're providing! I'll try to make the example in gemgem more clear/knock a simple example together for you.

@bethesque
Copy link

Thanks. Great work.

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

4 participants