Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 60 commits into from
Feb 17, 2020
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
1b7f7b2
chore: adds docker container with php7.4
Jan 17, 2020
aa4fe6c
feat: adds generator to swagger definitions section
Jan 17, 2020
d9f6b04
style: removes extra "\" from test routes definitions
Jan 17, 2020
0d8fe42
feat: changes php version according to travis ci definitions
Jan 17, 2020
d257c3b
refactor: adds action extraction methods to Route class
Jan 20, 2020
605958d
refactor: obtains model relations from helper instead of class
Jan 20, 2020
fb2482a
refactor: disables generate definition example
Jan 20, 2020
71c1b4f
refactor: changes model extraction from Generator to Route
Jan 20, 2020
9ae6f11
feat: adds generator to success and error responses
Jan 20, 2020
833a448
feat: adds definitions and responses to generated docs
Jan 20, 2020
ba07e5a
docs: adds docs to generate definitions and responses
Jan 20, 2020
6c82814
docs: updates docs details
Jan 20, 2020
805dddd
feat: adds factory to generate example field on parameters.
Jan 21, 2020
947efec
feat: fixes form request name class extraction
Jan 21, 2020
f979832
feat: adds view to show swagger docs
Jan 21, 2020
ebce3da
feat: adds swagger ui dependency
Jan 21, 2020
23e6c82
feat: adds swagger ui route and view
Jan 22, 2020
7d6aec3
test: adds tests to GenerateSwaggerDocCommand
Jan 22, 2020
75d59ad
refactor: changes to filter routes before generate docs
Jan 22, 2020
5e11250
docs: changes generate swagger command usage
Jan 22, 2020
e6ee2bc
feat: adds routes versions to swagger ui view
Jan 22, 2020
36ddef3
feat: adds select to choose api version on Swagger UI
Jan 23, 2020
c90c3fc
feat: adds method to route return form request from params
Jan 23, 2020
ed5a2ad
feat: adds errors response generation with laravel defaults responses
Jan 23, 2020
2029b79
feat: adds cast to array on get throws from route docblock
Jan 23, 2020
bd7637c
feat: adds handlers to generate definitions to error responses
Jan 24, 2020
c7d5e4f
fix: fixes to create one definition for each form request
Jan 24, 2020
404da85
chore: adds xdebug to Dockerfile
Jan 24, 2020
1a8e175
test: adds tests to return of getAppends method
Jan 24, 2020
8f825f0
feat: throws exception when response has no definition configured
Jan 24, 2020
56a4658
test: adds test when find not existent version
Jan 24, 2020
f82c559
feat: adds swagger file name generator to make it more automatic
Jan 27, 2020
0c5979c
fix: fixes definition generation when has no model defined
Jan 27, 2020
91ed0b5
style: fixes return type from getModel method
Jan 27, 2020
f109f0c
fix: fixes check model on success response generator
Jan 27, 2020
1ac2f44
feat: adds jwt security definitions generator
Jan 28, 2020
2a2cfb6
fix: fixes to get all middleware registered for action
Jan 29, 2020
29d3b44
test: adds test for route
Jan 29, 2020
3e11718
fix: removes output parameter from command
Jan 29, 2020
81365ca
feat: add trait to use when the model has appends
Jan 29, 2020
83492bc
docs: adds info and examples how to use the laravel swagger
Jan 29, 2020
ea54419
test: fixes passport routes calling
Jan 30, 2020
5716e27
chore: adds docker-sync to improve performance on mac
sfelix-martins Jan 30, 2020
a99a9d4
chore: adds gitattributes
sfelix-martins Jan 30, 2020
8d87fee
feat: changes parseSecurity and parseDocsBlock from version to global
sfelix-martins Feb 5, 2020
5e25242
feat: removes param --all-versions from command
sfelix-martins Feb 5, 2020
ea2a76e
refactor: changes the model annotation obtaining
sfelix-martins Feb 5, 2020
344187d
refactor: changes getActionClassInstance to use action from $this
sfelix-martins Feb 5, 2020
b8ee2e7
refactor: removes alias to check if can generate definitions
sfelix-martins Feb 5, 2020
c6e83a0
refactor: removes not required filter to check if allows generate
sfelix-martins Feb 5, 2020
2964244
conflict fix
mtrajano Feb 7, 2020
bad6685
moved configs around for better versioning
mtrajano Feb 9, 2020
316215d
more clean up and tests
mtrajano Feb 10, 2020
46a4254
more fixes and tests
mtrajano Feb 10, 2020
0ab8825
relations logic do not call method before checking return typehint
mtrajano Feb 12, 2020
dd5e5e6
test: changes to assert example is not empty instead of just null
sfelix-martins Feb 12, 2020
9f8aa0b
config for parsing relationships and example data, disable by default
mtrajano Feb 13, 2020
511eda6
Merge branch 'master' of github.com:coopersystem-fsd/laravel-swagger …
mtrajano Feb 13, 2020
428989e
fixed documentation
mtrajano Feb 17, 2020
77b52ce
skipping flaky tests for now
mtrajano Feb 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 12 additions & 29 deletions src/Definitions/DefinitionGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,11 @@
namespace Mtrajano\LaravelSwagger\Definitions;

use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;
use Illuminate\Support\Str;
use InvalidArgumentException;
use Mtrajano\LaravelSwagger\DataObjects\Route;
use Mtrajano\LaravelSwagger\Definitions\ErrorHandlers\DefaultDefinitionHandler;
use ReflectionException;
use RuntimeException;

class DefinitionGenerator
{
Expand Down Expand Up @@ -41,7 +38,7 @@ public function __construct(Route $route, array $errorsDefinitions)
}

/**
* @throws ReflectionException
* @throws \ReflectionException
*/
public function generate(): array
{
Expand Down Expand Up @@ -76,7 +73,7 @@ private function getPropertyDefinition($column)
* Get model searching on route and define the found model on $this->model
* property.
*
* @throws ReflectionException
* @throws \ReflectionException
*/
private function setModelFromRouteAction(): void
{
Expand Down Expand Up @@ -136,25 +133,18 @@ private function getDefinitionName(): string
private function getModelFake(): ?Model
{
try {
DB::beginTransaction();

return factory(get_class($this->model))->create();
return factory(get_class($this->model))->make();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} catch (InvalidArgumentException $e) {
return null;
} finally {
DB::rollBack();
}
}

/**
* Identify all relationships for a given model
*
* @param Model $model Model
* @return array
*
* @throws ReflectionException
* @throws \ReflectionException
*/
public function getAllRelations(Model $model)
public function getAllRelations(Model $model): array
{
return get_all_model_relations($model);
}
Expand All @@ -175,12 +165,12 @@ private function generateFromCurrentModel()
}

/**
* @param Model $model
* @return $this
*/
private function setModel(Model $model)
{
$this->model = $model;

return $this;
}

Expand Down Expand Up @@ -219,29 +209,22 @@ private function mountColumnDefinition(string $column)

/**
* Set property data on specific definition.
*
* @param string $definition
* @param string $property
* @param $data
*/
private function setPropertyOnDefinition(
string $definition,
string $property,
$data
) {
private function setPropertyOnDefinition(string $definition, string $property, $data)
{
$this->definitions[$definition]['properties'][$property] = $data;
}

/**
* Generate the definition from model Relations.
*
* @throws ReflectionException
* @throws \ReflectionException
*/
private function generateFromRelations()
{
$baseModel = $this->model;
$relations = $this->getAllRelations($this->model);

$baseModel = $this->model;
foreach ($relations as $relation) {
$this->setModel($baseModel);

Expand Down Expand Up @@ -288,7 +271,7 @@ private function definitionExists()
/**
* Generate definition from errors.
*
* @throws ReflectionException
* @throws \ReflectionException
*/
private function generateFromErrors()
{
Expand All @@ -307,7 +290,7 @@ private function generateFromErrors()
/**
* @param $exception
* @return DefaultDefinitionHandler|null
* @throws ReflectionException
* @throws \ReflectionException
*/
private function findExceptionDefinitionHandler($exception): ?DefaultDefinitionHandler
{
Expand Down
73 changes: 14 additions & 59 deletions src/helpers.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,88 +38,43 @@ function get_annotations(string $from, string $annotationName): array
/**
* Identify all relationships for a given model
*
* @todo Create unit test fot this method.
* @param Model $model Model
* @param string $heritage A flag that indicates whether parent and/or child
* relationships should be included
* @return array
* @throws ReflectionException
*/
function get_all_model_relations(Model $model = null, $heritage = 'all') {
$modelName = get_class($model);
function get_all_model_relations(Model $model, $heritage = 'all'): array {
$types = ['children' => 'Has', 'parents' => 'Belongs', 'all' => ''];
$heritage = in_array($heritage, array_keys($types)) ? $heritage : 'all';

$filter = $types[$heritage] ?? $types['all'];
$reflectionClass = new ReflectionClass($model);
$traits = $reflectionClass->getTraits();
$traitMethodNames = [];
foreach ($traits as $name => $trait) {
$traitMethods = $trait->getMethods();
foreach ($traitMethods as $traitMethod) {
$traitMethodNames[] = $traitMethod->getName();
}
}

// Checking the return value actually requires executing the method.
// So use this to avoid infinite recursion.
$currentMethod = collect(explode('::', __METHOD__))->last();
$filter = $types[$heritage];
// The method must be public
$methods = $reflectionClass->getMethods(ReflectionMethod::IS_PUBLIC);

$methods = collect($methods)
->filter(
function (ReflectionMethod $method) use (
$modelName,
$traitMethodNames,
$currentMethod
) {
$methodName = $method->getName();
/**
* - The method must not originate in a trait
* - It must not be a magic method
* - It must be in the self scope and not inherited
* - It must be in the this scope and not static
* - It must not be an override of this one
*/
if (!in_array($methodName, $traitMethodNames)
&& strpos($methodName, '__') !== 0
&& $method->class === $modelName
&& !$method->isStatic()
&& $methodName != $currentMethod
) {
$parameters = (
new ReflectionMethod($modelName, $methodName)
)->getParameters();
// If required parameters exist, this will be false and
// omit this method
return collect($parameters)->filter(
function (ReflectionParameter $parameter) {
// The method must have no required parameters
return !$parameter->isOptional();
}
)->isEmpty();
}
return false;
->map(function (ReflectionMethod $method) use ($filter, $model) {

if (!$method->getReturnType() || !$method->getReturnType()->getName()) {
return null;
}
)
->map(function (ReflectionMethod $method) use ($model, $filter) {

$methodName = $method->getName();
/** @var Relation|mixed $relation */
$relation = $model->$methodName();
$returnTypeClass = $method->getReturnType()->getName();

// Must return a Relation child. This is why we only want to do
// this once
if (is_subclass_of($relation, Relation::class)) {
$type = (new ReflectionClass($relation))->getShortName();
if (is_subclass_of($returnTypeClass, Relation::class)) {
$relation = $method->invoke($model);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// If relation is of the desired heritage
if (!$filter || strpos($type, $filter) === 0) {
if (!$filter || strpos($returnTypeClass, $filter) === 0) {
return [
'method' => $methodName,
'related_model' => $relation->getRelated(),
'relation' => get_class($relation),
];
}
}

return null;
})
// Remove elements reflecting methods that do not have the desired
Expand Down
Loading