jhthorsen / mojolicious-plugin-openapi

OpenAPI / Swagger plugin for Mojolicious
54 stars 42 forks source link

Decouple the validation and the web context #158

Closed Ovid closed 4 years ago

Ovid commented 4 years ago

I'm not sure how feasible the following request is, but it would be lovely if it could be officially supported.

In the tutorial, we have something similar to this:

package Myapp::Controller::Pet;
use Mojo::Base "Mojolicious::Controller";

sub list {
  my $c = shift->openapi->valid_input or return;

  my $input = $c->validation->output;
  my $age   = $c->param("age"); # same as $input->{age}
  my $body  = $c->req->json;    # same as $input->{body}

  my $output = {pets => [{name => "kit-e-cat"}]};
  $c->render(openapi => $output);
}

1;

In this controller method, we have both validation and data generation. When I'm building MVC applications, I like to have the controller merely take view data and dispatch that to the model (Jan, I know you are aware of this, but for anyone else, you can read about the reasoning here, but I don't want to clutter the ticket with this).

Thus, it would be nice if the controller looked conceptually similar to this:

sub list {
  my $c = shift->openapi->valid_input or return;
  $c->render(openapi => $c->model('Pet')->list);
}

And something like MyApp::Model::Pet would have no need to know about the web context., but would still be able to validate against the schema to ensure that input and output data conform to requirements.

Aside from a proper separation of concerns and allowing me to build an app that can be more easily used outside of a web context, it also means that I can write tests targeting the data directly and not going through Test::Mojo. While the latter module is great, it has the same issue that all similar modules have: when having to have a "browser" object, the tests run much slower and the error messages can easily be obscured. It's trivial to get a 500 error and not immediately see what caused it.

By having a clean model that doesn't require a "browser" object, tests run much faster and it's far easier to debug what's going on.

Admittedly it's hard to completely decouple these things because OpenAPI is itself tightly coupled to the Web, but I figured I could take a first stab at this using JSON::Validator::OpenAPI::Mojolicious. Except that the docs say very clearly:

Do not use this module directly. Use Mojolicious::Plugin::OpenAPI instead.

So I'm unclear how to best achieve this goal, but it would be nice to do so.

jhthorsen commented 4 years ago

I'm not sure if I understand, but...

As long as MyApp::Model::Pet::list() returns either a plain data structure or something that can TO_JSON() then you can absolutely make the controller as thin as you want.

You can also extract the validated data from $c->req->json (if it's a posted JSON document) or from $c->validation->output if you want the POST/GET parameters as a hash instead of fiddling around with $c->param(...).

If you on the other side want to do validation inside your model, you can do that using https://metacpan.org/pod/JSON::Validator, which does not care about $c. You can also feed JSON::Validator parts of your OpenAPI spec, if you want to reuse the "body" parameter schema rules.

When it comes to testing, then I would urge you to both have tests for the API server and for the model. The reason is that there's no guaranty that you've written a spec that makes sense, until you actually test your application. I've surprised myself many times when trying to write failing (400 Bad Request) tests, and then observing they all get 200 OK 🙈

Ovid commented 4 years ago

When it comes to testing, then I would urge you to both have tests for the API server and for the model.

Absolutely agreed. But I primarily want my server tests to know the server dispatches correctly between the view and the model. I can thus keep those tests as thin as the controller methods.

And yes, I want to do validation inside of the model, but perhaps that's output validation and not input validation (since that's coupled to the web; I'll investigate). However, I'll need to tread lightly because I don't want double-validation of both input and output (for example, if the model validates output, and I call $c->render(openapi => $data) in the controller, it would be validated a second time).

jhthorsen commented 4 years ago

I do have validation in some of my models. The validation must be called explicitly by whatever application that uses the model though. Example:

#!perl
use MyModel;
use Mojo::JSON 'decode_json';
my $data = decode_json(shift @ARGV);
MyModel->new($data)->validate_or_die->insert_into_database;

The validate_or_die() method extracts whatever JSON-Schema rules that I've already defined in my OpenAPI spec. That way, I know (famous last words) that my script follow the same rules as my web app.

Example web app:

use Mojolicious::Lite;
use MyModel;

post "/cool-stuff" => sub {
  my $c = shift->openapi->valid_input or return;
  my $obj = MyModel->new($c->req->json);
  $obj->insert_into_database_p->then(sub { $c->render(openapi => $obj) });
}, 'CoolOperationId';

I do however don't worry too much about my models, and what input they get. I worry about what my world wide accessible application might receive.

jhthorsen commented 4 years ago

Also, I absolutely don't worry about what kind of data the model outputs, since other parts of my program (or other parts of the system) should not be slowed down, just because I want the internal representation of "123" (string) to be 123 (integer) instead. I only care that my JavaScript doesn't suddenly start returning "361" instead of "37" when I do something like json.age + 1.

jhthorsen commented 4 years ago

I wonder if you should join #perl-openapi on irc.freenode.net instead at this point.

jhthorsen commented 4 years ago

I’m closing this, since the issue seems have gone stale.