[katello-devel] Apipie and validation.

[Petr Chalupa]

I see the model validation and API params validation in controller as 
two different things. -1 for removing the validation from apipie and 
controllers. I agree with the lzap.

However I felt the duplication in simple crud controller actions when 
writing api documentation. +1 to fix [1] (the issue inecas mentioned) to 
get rid of the duplication.

-1 for disabling the apipie validations by default. When adding a 
feature a developer should disable the apipie validation (if needed) to 
see ActiveRecord errors and to simplify developing. Then apipie 
validation should be re-enabled and tested if it is up to date. I think 
task keeping-API-doc-up-to-date should not be done separately from 
feature development.

Petr

[1] - https://github.com/Pajk/apipie-rails/issues/67

On 03.12.12 15:54, Eric Helms wrote:
> We are in the situation of having two different entry points into our
> model - API and UI controllers. This fact alone makes me feel that we
> need as much as possible validations at the model layer, otherwise we
> are just repeating ourselves and opening holes for miss-matches in
> expectations around our data layer.  Further, our data layer should be
> the end-all, know-all for the /data/.  Thus, I would say:
>
> - If the validation deals with a direct property of the model (either an
> attribute or an association), the validation should live in the model
> - If the validation deals with a value expected specifically by the
> controller that is somehow involved in massaging of the attributes sent
> down to the model it should live in the controller
>
> I agree our models feel quite large, there is lot of logic and
> functionality sitting in them currently.  That shouldn't be the reason
> we shy away from putting data validation in that layer, that should be
> the motivation to organize our models smarter.  In the PulpV2 branch we
> have made some strides to help with this by breaking out all
> authorization (app/models/authorization) and elasticsearch
> (app/models/glue/elasticsearch/) functionality into their own files
> [1].  A good example file to look at is repository.rb [2]
>
> https://github.com/Katello/katello/tree/pulpv2/src/app/models
>
> https://github.com/Katello/katello/blob/pulpv2/src/app/models/repository.rb
>
> - Eric
>
> On 12/03/2012 08:44 AM, Ivan Necas wrote:
>>
>> ----- Original Message -----
>>>
>>> ----- Original Message -----
>>> | On 30/11/12 03:41 PM, Lukas Zapletal wrote:
>>> | > On Fri, Nov 30, 2012 at 03:02:15PM +0000, Dmitri Dolguikh wrote:
>>> | >> Repeat after me: validation logic belongs in the model.
>>> | > Only in an ideal world, Dmitri. ;-)
>>> | >
>>> | > Not all our controllers follows REST pattern, and you know that.
>>> | >
>>> | This has nothing to do with rest - just common sense. If you start
>>> | doing
>>> | validations in controllers, you will inevitable start moving
>>> | business
>>> | logic to them.
>>> |
>>> | In our case, instead of perpetuating the problem, let's fix it
>>> | instead.
>>> | Step #1 - let's get rid of controller validations.
>>> |
>>>
>>> The good thing about the validations is that you can pass them along,
>>> for example to a client, so it does not need to make the round trip
>>> to figure out he needs another param.
>>>
>>> I agree that I would prefer the model to raise the error, so should
>>> probably turn it off by default?
>> +1 for turning it off by default, for this reason.
>>
>> However, I wouldn't recommend to getting rid of them completely. Being
>> able to describe the params with the code has much more value in it than
>> just with plain strings/comments. This was the original intention of the
>> apipie gem. We can just run it with validations turned on when we want to
>> find inconsistencies between documentation and actual code, perhaps
>> automatically
>> with some test suite for that.
>>
>> -- Ivan
>>
>>> I would also look at
>>> https://github.com/bcardarella/client_side_validations for
>>> inspirations of how to get the model validations.
>>>
>>> Ohad
>>> |
>>> | _______________________________________________
>>> | katello-devel mailing list
>>> | katello-devel at redhat.com
>>> | https://www.redhat.com/mailman/listinfo/katello-devel
>>> |
>>>
>>> _______________________________________________
>>> katello-devel mailing list
>>> katello-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/katello-devel
>>>
>> _______________________________________________
>> katello-devel mailing list
>> katello-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/katello-devel
>
>
>
>
> _______________________________________________
> katello-devel mailing list
> katello-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/katello-devel
>