Added schema definitions, responses, support to JWT auth, Swagger UI and Docker environment #24

sfelix-martins · 2020-01-29T18:08:54Z

Added

Added schema definitions for models (Defined on @model annotation on methods or controller).
Added responses based on routes http methods, @throws annotations and auth middleware.
Added support to JWT authentication.
Added Swagger UI with docs.
Added docker environment to development support.

Changed

Changed to get middlewares from controller too, instead of only from routes.
Changed command usage to don't print on console or set in file. Automatically will generate on public path and the docs will be available in /docs route.

Configuring

To download the package from fork just add the repository in your composer.json:

   "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/coopersystem-fsd/laravel-swagger"
        }
    ],

and change the package version to dev-master:

"require": {
        "mtrajano/laravel-swagger": "dev-master",
}

Run composer update and the code from fork should be downloaded.

Usage

Follow the instructions on README.

Closes #18 , #7

Co-authored-by: Odilon Garcez <[email protected]> Co-authored-by: Alan Christian <[email protected]>

Co-authored-by: Odilon Garcez <[email protected]>

Changes the configs to generate and show Swagger UI to many API versions. Changes the generate swagger docs command to generate the swagger files to specific versions on public folder to access from SwaggerUI. Co-authored-by: Odilon Garcez <[email protected]> Co-authored-by: Alan Christian <[email protected]>

Co-authored-by: Odilon Garcez <[email protected]> Co-authored-by: Alan Christian <[email protected]>

Co-authored-by: Odilon Garcez <[email protected]>

mtrajano · 2020-02-10T01:12:48Z

config/laravel-swagger.php

-                // 'http',
-                // 'https',
-            ],
+            'basePath' => '/',


didn't want to assume the basePath of the application, especially since I don't know if most people using this will have their application be versioned but it's there as an option for them

src/Definitions/DefinitionGenerator.php

mtrajano · 2020-02-10T01:16:45Z

src/Generator.php

-        return Str::replaceFirst($this->config['basePath'], '', $this->route->uri());
+        $uri = Str::replaceFirst($this->config['basePath'], '', $this->route->uri());
+
+        return Str::start($uri, '/');


fixed bug where if the user added / as basePath it would strip out the leading / from the path in the swagger docs

sfelix-martins · 2020-02-10T01:31:56Z

@mtrajano Thanks for good changes.

I saw that occurred and error on travis build. Here has an example that can help you.

mtrajano · 2020-02-10T02:13:50Z

@sfelix-martins oh nice yes that is a much nicer solution, I'll update ci

mgamal92 · 2020-02-10T10:16:45Z

Can I help in changes to make the process fast ? because I need this PR to be merged to can use it

sfelix-martins · 2020-02-10T12:18:55Z

@mtrajano Please, resolve the review conversations that you already handled, to be more easy to see the really pending reviews.

mtrajano · 2020-02-10T17:36:54Z

@mgamal92 @sfelix-martins hey sorry will resolve the issues I tackled already. The 2 things missing that are the most pressing are: 1. changing the example data logic from create to make and add tests for those (can also kill the transaction logic). 2. for the logic of getting the model relationships we should definitely not be calling random model methods in order to not cause any side effects (can potentially increment things on the db, add/remove data, etc..). Potential solutions would be to either rely on the return type hints to be of instance HasOne etc.. (prefer this one) or rely on a new annotations, and again just adding tests for these.

Some other ones I was hoping to get to before merging are: 1. making ignoredRoutes also take regex since this was a feature before and we're removing it in this pr, 2. Making the response logic more flexible (being able to config these etc..), am currently working on this one and adding tests. Finally some general clean up and fixing the README documentation.

Sorry again for the time it's taking to merge this one, I was actually away at a conference last week so I only had a few hours here and there to get stuff out. I have a lot more time until Wednesday so I was hoping to merge it by then. But yes if you guys can tackle some of these I'd really appreciate it and it'd speed the process a lot. Thanks!

mgamal92 · 2020-02-10T17:42:46Z

Please clarify what problem about response ?

mtrajano · 2020-02-11T02:05:55Z

@mgamal92 nothing per se, it would just be nice if the consumer could configure which route methods map to which response codes and what their descriptions are. But again, that could come at a future release. I already fixed the example logic and I'm working on the model relationships logic so the only thing left to do would be to fix the docs to match the new config structure and adding the ability to filter by regex (not just route name). This should hopefully be done by tomorrow 🤞

mgamal92 · 2020-02-11T09:06:50Z

So I understood from that, the PR will be finished merged tomorrow, right?

…

On Tue, Feb 11, 2020, 4:05 AM Mauricio Trajano ***@***.***> wrote: @mgamal92 <https://github.com/mgamal92> nothing per se, it would just be nice if the consumer could configure which route methods map to which response codes and what their descriptions are. But again, that could come at a future release. I already fixed the example logic and I'm working on the model relationships logic so the only thing left to do would be to fix the docs to match the new config structure and adding the ability to filter by regex (not just route name). This should hopefully be done by tomorrow 🤞 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24?email_source=notifications&email_token=AHWBLWAEQSNSF2UAGICSLK3RCIBYJA5CNFSM4KNJELW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELK7WGY#issuecomment-584448795>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHWBLWDNECFJ2KDREKMJGD3RCIBYJANCNFSM4KNJELWQ> .

mtrajano · 2020-02-12T03:08:00Z

src/Definitions/DefinitionGenerator.php

-            DB::beginTransaction();
-
-            return factory(get_class($this->model))->create();
+            return factory(get_class($this->model))->make();


Make is much safer, only creates an instance without making a connection to the db

I don't know if it's more safer. Please, think about cases like that:

$factory->define(App\Post::class, function ($faker) { return [ 'title' => $faker->title, 'content' => $faker->paragraph, 'user_id' => function () { return factory(App\User::class)->create()->id; } ]; });

If think we should to use transactions to avoid surprises...

I checked here other reasons to use create instead of make. We done it to generate the example data to timestamps and id fields too. I don't know if it sufficient... bus was one of the reason.

The test that check the example response is checking just if the result is not null. But I think that the correct behavior should check if is not empty. If apply this change the build will fail with the make method, because empty values in created_at, updated_at na id properties.

Ahh good point on the user_id example, I guess we can't avoid a transaction here. I'll change this back to what it was. Reasons I was trying to avoid create even with the transaction are 1. to avoid having to make a connection to the db to generate static data, 2. it doesn't necessarily leave the db in an unchanged state (one example being the AUTO_INCREMENT value that remains unchanged even with a rollback) so it might just be some extra side effects that the user might not be expecting when running this. We might also want to add a config to disable this part by default and let the user manually turn it on if he chooses to, along with some prominent warnings not to run this in a prod environment (which should be obvious but we can't always guarantee that).

I allow. I think we can maybe too ask user a different DB connection just to generate the docs... what about it?

mtrajano · 2020-02-12T03:09:55Z

src/helpers.php

-                if (is_subclass_of($relation, Relation::class)) {
-                    $type = (new ReflectionClass($relation))->getShortName();
+                if (is_subclass_of($returnTypeClass, Relation::class)) {
+                    $relation = $method->invoke($model);


still not ideal but slightly safer than before. It at least checks the return typehint of the method before invoking it. Am also going to add a config to disable this by default and add a warning on the docs that this will invoke methods in the model to get the relation name.

mtrajano · 2020-02-12T03:13:14Z

Pushed some more changes out, only thing missing is just to fix the README and add a config called parseModels to disable invoking the model methods by default. Thanks for bearing with me all

mtrajano · 2020-02-12T14:24:26Z

tests/Definitions/DefinitionGeneratorTest.php

+    {
+        return $this->belongsTo('Mtrajano\LaravelSwagger\Tests\Definitions\Customer');
+    }


going to clean up these tests and move them to the stubs namespace during a later refactor after the merge

…into master-pr

mtrajano · 2020-02-13T03:36:52Z

src/Definitions/DefinitionGenerator.php

+     */
+    private $shouldParseRelationshipDefinitions;
+
+    public function __construct(Route $route, array $errorsDefinitions = [], $shouldGenerateExampleData = false, $shouldParseRelationshipDefinitions = false)


@sfelix-martins let me know what you think. If this works for you then I just need to revert that other change, fix the documentation and we can finally merge this soon 🙌

@mtrajano To me it looks good to leave it in the user's hands

mtrajano · 2020-02-17T04:35:23Z

@sfelix-martins Sorry for the late response to this. I fixed the docs to match the match the new config layout. However there are still a couple of flaky tests ever since you changed the assertion to check that the example property is empty vs null. It always passes locally but seems to fail every now and then in CI, I tried sshing into a 7.2/7.3 php docker container and tried running the test there but it was still passing pretty consistently. So I chose to skip those tests for now.

I don't want to keep this pr open much longer and keep adding changes to this since I think it's getting a little out of control so I opened a v0.7 branch and I think we can merge to that in the meantime. I'll cut a dev release for 0.7 that way people who want to use/test the new features are able to do so and we're able to clean up, fix the tests, etc.. and when it's stable we'll merge it into master, does that sound ok? I'll also add you guys as collaborators as well, thank you so much for this awesome pr and sticking around!

sfelix-martins · 2020-02-17T09:32:26Z

@mtrajano ok man. It looks good. Thank you for develop this very good package. I think we can finally merge.

Samuel and others added 30 commits January 17, 2020 15:20

chore: adds docker container with php7.4

1b7f7b2

Co-authored-by: Odilon Garcez <[email protected]> Co-authored-by: Alan Christian <[email protected]>

feat: adds generator to swagger definitions section

aa4fe6c

Co-authored-by: Odilon Garcez <[email protected]> Co-authored-by: Alan Christian <[email protected]>

style: removes extra "\" from test routes definitions

d9f6b04

Co-authored-by: Odilon Garcez <[email protected]> Co-authored-by: Alan Christian <[email protected]>

feat: changes php version according to travis ci definitions

0d8fe42

Co-authored-by: Odilon Garcez <[email protected]> Co-authored-by: Alan Christian <[email protected]>

refactor: adds action extraction methods to Route class

d257c3b

Co-authored-by: Odilon Garcez <[email protected]>

refactor: obtains model relations from helper instead of class

605958d

refactor: disables generate definition example

fb2482a

refactor: changes model extraction from Generator to Route

71c1b4f

feat: adds generator to success and error responses

9ae6f11

feat: adds definitions and responses to generated docs

833a448

docs: adds docs to generate definitions and responses

ba07e5a

docs: updates docs details

6c82814

feat: adds factory to generate example field on parameters.

805dddd

Co-authored-by: Odilon Garcez <[email protected]>

feat: fixes form request name class extraction

947efec

Co-authored-by: Odilon Garcez <[email protected]>

feat: adds view to show swagger docs

f979832

Co-authored-by: Odilon Garcez <[email protected]>

feat: adds swagger ui dependency

ebce3da

Co-authored-by: Odilon Garcez <[email protected]>

test: adds tests to GenerateSwaggerDocCommand

7d6aec3

Co-authored-by: Odilon Garcez <[email protected]> Co-authored-by: Alan Christian <[email protected]>

refactor: changes to filter routes before generate docs

75d59ad

Co-authored-by: Odilon Garcez <[email protected]> Co-authored-by: Alan Christian <[email protected]>

docs: changes generate swagger command usage

5e11250

Co-authored-by: Odilon Garcez <[email protected]>

feat: adds routes versions to swagger ui view

e6ee2bc

Co-authored-by: Odilon Garcez <[email protected]>

feat: adds select to choose api version on Swagger UI

36ddef3

Co-authored-by: Odilon Garcez <[email protected]>

feat: adds method to route return form request from params

c90c3fc

feat: adds errors response generation with laravel defaults responses

ed5a2ad

feat: adds cast to array on get throws from route docblock

2029b79

Co-authored-by: Odilon Garcez <[email protected]>

feat: adds handlers to generate definitions to error responses

bd7637c

Co-authored-by: Odilon Garcez <[email protected]>

fix: fixes to create one definition for each form request

c7d5e4f

chore: adds xdebug to Dockerfile

404da85

test: adds tests to return of getAppends method

1a8e175

feat: throws exception when response has no definition configured

8f825f0

mtrajano reviewed Feb 10, 2020

View reviewed changes

src/Definitions/DefinitionGenerator.php Outdated Show resolved Hide resolved

mtrajano reviewed Feb 10, 2020

View reviewed changes

mtrajano force-pushed the master branch from 78c1fe5 to 62b8020 Compare February 10, 2020 01:56

more clean up and tests

316215d

mtrajano force-pushed the master branch from 62b8020 to 316215d Compare February 10, 2020 02:06

more fixes and tests

46a4254

relations logic do not call method before checking return typehint

0ab8825

mtrajano reviewed Feb 12, 2020

View reviewed changes

test: changes to assert example is not empty instead of just null

dd5e5e6

mtrajano reviewed Feb 12, 2020

View reviewed changes

mtrajano added 2 commits February 12, 2020 22:33

config for parsing relationships and example data, disable by default

9f8aa0b

Merge branch 'master' of github.com:coopersystem-fsd/laravel-swagger …

511eda6

…into master-pr

mtrajano reviewed Feb 13, 2020

View reviewed changes

mtrajano added 2 commits February 16, 2020 19:32

fixed documentation

428989e

skipping flaky tests for now

77b52ce

mtrajano changed the base branch from master to v0.7 February 17, 2020 04:36

mtrajano merged commit bd42122 into mtrajano:v0.7 Feb 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added schema definitions, responses, support to JWT auth, Swagger UI and Docker environment #24

Added schema definitions, responses, support to JWT auth, Swagger UI and Docker environment #24

sfelix-martins commented Jan 29, 2020 •

edited

Loading

mtrajano Feb 10, 2020

mtrajano Feb 10, 2020

sfelix-martins commented Feb 10, 2020

mtrajano commented Feb 10, 2020

mgamal92 commented Feb 10, 2020

sfelix-martins commented Feb 10, 2020

mtrajano commented Feb 10, 2020 •

edited

Loading

mgamal92 commented Feb 10, 2020

mtrajano commented Feb 11, 2020

mgamal92 commented Feb 11, 2020 via email

mtrajano Feb 12, 2020

sfelix-martins Feb 12, 2020

sfelix-martins Feb 12, 2020

sfelix-martins Feb 12, 2020

mtrajano Feb 12, 2020

sfelix-martins Feb 12, 2020

mtrajano Feb 12, 2020

mtrajano commented Feb 12, 2020

mtrajano Feb 12, 2020

mtrajano Feb 13, 2020

sfelix-martins Feb 13, 2020

mtrajano commented Feb 17, 2020 •

edited

Loading

sfelix-martins commented Feb 17, 2020

Added schema definitions, responses, support to JWT auth, Swagger UI and Docker environment #24

Added schema definitions, responses, support to JWT auth, Swagger UI and Docker environment #24

Conversation

sfelix-martins commented Jan 29, 2020 • edited Loading

Added

Changed

Configuring

Usage

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfelix-martins commented Feb 10, 2020

mtrajano commented Feb 10, 2020

mgamal92 commented Feb 10, 2020

sfelix-martins commented Feb 10, 2020

mtrajano commented Feb 10, 2020 • edited Loading

mgamal92 commented Feb 10, 2020

mtrajano commented Feb 11, 2020

mgamal92 commented Feb 11, 2020 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtrajano commented Feb 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtrajano commented Feb 17, 2020 • edited Loading

sfelix-martins commented Feb 17, 2020

sfelix-martins commented Jan 29, 2020 •

edited

Loading

mtrajano commented Feb 10, 2020 •

edited

Loading

mtrajano commented Feb 17, 2020 •

edited

Loading