[katello-devel] Apipie and validation.

Wed Dec 5 12:20:25 UTC 2012


----- Original Message -----
| 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.
| 
What do you mean by that?

| 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
| 
| _______________________________________________
| katello-devel mailing list
| katello-devel at redhat.com
| https://www.redhat.com/mailman/listinfo/katello-devel
|