[katello-devel] Apipie and validation.

Wed Dec 5 10:11:49 UTC 2012

On 05/12/12 09:07 AM, Petr Chalupa wrote:
> I agree that domain validation/consistency checks belongs to the 
> models. Nevertheless definition of the API is other thing entirely. It 
> should be defined and validated on its own with re-usage of 
> information provided by models (domain) to avoid duplications. It 
> should not be delegated to model to deal with it by adding other 
> responsibility.

This statement is only valid if we are talking about cross-concerns 
(think authentication, security-related stuff, etc). Everything else 
will result in duplicated, non-consistent validations. Take a look at 
current state of validations in foreman api.

I don't want to be jumping through hoops, and fixing/adding 
code/meta-data in multiple places, so just the documentation is 
up-to-date. It's a process/organizational issue, and tools do not fix those.

And, once again, validations belong in the model. Let's get rid of 
validations in apipie, fix issues we have with domain logic leaking into 
controllers.

>
> So I strongly suggest to fix the duplication issue [1] and keep things 
> separate.
> * Apipie for API validations.
> * Model validations for domain logic.
>
> ## Fix Apipie
>
> Enable Apipie to read model definitions and validations to set param 
> validations automatically for parts of request representing a domain 
> model, see [1].

Agree with defining docs based on model (and I would suggest routes). 
Disagree with everything else. No point in intercepting validations, the 
most you could do in the controller is map exceptions thrown by the 
model to http errors/statuses.

>
>
> + api documentation generated from api specification
> + api specification is kept up to date (has to be because validation 
> is turned on)
> + api validation/specification is separated from domain validations/logic
>
> ## Remove validations from Apipie
>
> + api documentation generated from api specification
> - api specification is not kept up to date (has to be ensured in an 
> extra task)
> - no api validation, it's partially handled by models, errors can be 
> ugly, models have another responsibility

Again, tools are not the way to enforce process deficiencies. The last 
statement is FUD.

>
>
> [1] - https://github.com/Pajk/apipie-rails/issues/67
>
> ## Other notes
>
> > The second issue is that apipie validations are stricter than AR 
> ones: array validation assumes that there's an array in place, while 
> AR would accept a nil. These two combined lead to even more false 
> positives during apipie validation.
>
> I think be more consistent and strict is a good thing, it prevents 
> confusion. Empty Array is an empty Array not a nil.

It's not good, because depending on how you use foreman, you'll get 
different results. What's considered valid/correct by foreman proper 
should be valid by any other means, period.

-d

>
> Petr
>
> On 04.12.12 22:47, Ivan Necas wrote:
>>
>>
>> ----- Original Message -----
>>> On Tue 04 Dec 2012 05:10:28 AM MST, Dmitri Dolguikh wrote:
>>>> On 03/12/12 09:36 AM, Lukas Zapletal wrote:
>>>>>> Step #1 - let's get rid of controller validations.
>>>>> Again, you can only do this for CRUD actions in the controllers.
>>>>> For all
>>>>> other methods we need validations. And we have those, not all our
>>>>> controllers are plain REST-driven. We do have procedure-like
>>>>> actions.
>>>>> Maybe more then we should have, but this is a fact.
>>>>>
>>>>> You say all the validations should be done in models, thus you
>>>>> essentially say let's put all our code logic there. Because it
>>>>> does not
>>>>> make sense to do validations in models while doing anything else
>>>>> (that
>>>>> also requires some level of validations) in controllers or a logic
>>>>> layer. A fact: you need to do validations also outside of models.
>>>>
>>>> Domain logic belongs in the model layer. We can discuss how to
>>>> better
>>>> factor model layer code, but not where to put it.
>>>>
>>>> -d
>>>>
>>>> A lot of rationalizations to keep validations in apipie have been
>>>> put
>>>> forward, *all* of them attempting to deal with various smells, code
>>>> or
>>>> process ones. Let's fix the root issues.
>>>>
>>>> Again, Let's get rid of validations in apipie. Refactor the
>>>> controllers when needed. If apipie is not useful without those
>>>> validations, let's suspend/stop use of it.
>>
>> Am I reading it correctly you suggesting getting rid of API 
>> documentation?
>>
>> -- Ivan
>>
>>>>
>>>> -d
>>>>
>>> +1
>>>>
>>>>>
>>>>> It is wrong to put all your code logic into models. And
>>>>> unfortunately we
>>>>> have most of it there. Rails follows MVC pattern and what we have
>>>>> is
>>>>> those
>>>>> messy Models-for-everything approach. If you mix this with our
>>>>> orchestration, it is deadly combination. Difficult testability,
>>>>> huge
>>>>> objects with no responsibility split, unable to mock things and so
>>>>> on.
>>>>> Code logic in models works well for small projects and it is
>>>>> de-facto
>>>>> standard in Rails world. People just blindly follow this wrong
>>>>> pattern.
>>>>>
>>>>> So I don't agree with this simplification you offer. We will
>>>>> likely
>>>>> start to refactor our orchestration creating new layer of code
>>>>> logic,
>>>>> which is the right approach to do things. And we should move
>>>>> complex
>>>>> methods like activation key subscription from models there. Then
>>>>> we will
>>>>> need validations on the controller side (or on the newly created
>>>>> logic
>>>>> layer or orchestration layer - depends on how are we gonna name
>>>>> it).
>>>>>
>>>>> So yes, it has something to do with REST. My counter proposal is:
>>>>>
>>>>> #2 Let's get rid of validations where they duplicate model
>>>>> validators,
>>>>> but let's keep them for (usually non-CRUD) methods which need
>>>>> then. We
>>>>> will likely be extending those soon.
>>>>>
>>>>
>>>> _______________________________________________
>>>> katello-devel mailing list
>>>> katello-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/katello-devel
>>>
>>>
>>>
>>> -- 
>>> Jason E. Rist
>>> Senior Software Engineer
>>> Systems Management and Cloud Enablement
>>> Red Hat, Inc.
>>> +1.919.754.4048
>>> Freenode: jrist
>>> github/identi.ca: knowncitizen
>>>
>>> _______________________________________________
>>> 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