#78: Store tmp doc data in file instead of static class property to use it in Artisan push-documentation command #80

vitgrams · 2023-03-07T05:11:48Z

…use Artisan command to push documentation;

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules

…use Artisan command to push documentation;

vitgrams · 2023-03-07T07:43:50Z

@DenTray Done as discussed. Could you please review this code?

DenTray · 2023-03-09T03:13:23Z

src/Drivers/StorageDriver.php

+    public function saveTmpData($data)
    {
-        self::$data = $tempData;
+        $this->disk->put($this->tempFilePath, json_encode($data));


tmp data should be stored in the local file, but storage disc may be set as remote storage which may cause a slow testing process. Please use file_put_contetn as well here

vitgrams · 2023-03-09T14:54:03Z

@DenTray Review conversations have been resolved.

DenTray · 2023-03-13T00:55:31Z

composer.json

    "php": ">=7.3.0",
    "laravel/framework": ">=5.3.0",
-    "phpunit/phpunit": ">=7.0|<=10.0"
+    "phpunit/phpunit": ">=7.0|<10.0"


Suggested change

"phpunit/phpunit": ">=7.0|<10.0"

"phpunit/phpunit": ">=7.0"

@DenTray We cannot remove this restriction, because we will have code that depends on code that does not exist in version 10 of PhpUnit lib.

@vitgrams as we discussed, let's add this restriction after the swagger 3 will be released

DenTray · 2023-03-13T00:57:12Z

src/Drivers/StorageDriver.php

    public function getTmpData()
    {
-        return self::$data;
+        if (file_exists($this->tempFilePath)) {


it seems we can move such duplicated logic to the parent BaseDriver class

DenTray · 2023-03-13T00:58:50Z

@vitgrams please don't forget to rebuild composer.lock

…added MissedRemoteDocumentationUrlException Abstract BaseDriver has been added to unify saveTmpData, getTmpData methods semantic.

DenTray · 2023-03-14T01:11:13Z

config/auto-doc.php

    |
    | The name of driver, which will collect and save documentation
    | Feel free to use your own driver class which should be inherited from
-    | `RonasIT\Support\AutoDoc\Interfaces\SwaggerDriverInterface` interface,


I believe we shouldn't force developers to use BaseDriver as the parent class.

Let's just make BaseDriver implementing SwaggerDriverInterface interface

DenTray · 2023-03-14T01:11:27Z

src/Drivers/BaseDriver.php

+
+namespace RonasIT\Support\AutoDoc\Drivers;
+
+abstract class BaseDriver


Suggested change

abstract class BaseDriver

abstract class BaseDriver implements SwaggerDriverInterface

DenTray · 2023-03-14T01:12:40Z

src/Drivers/LocalDriver.php

    public function __construct()
    {
        $this->prodFilePath = config('auto-doc.drivers.local.production_path');
+        $this->tempFilePath = storage_path('temp_documentation.json');


could we also move it to the parent class

DenTray · 2023-03-14T01:13:30Z

src/Drivers/LocalDriver.php

+        if (file_exists($this->tempFilePath)) {
+            unlink($this->tempFilePath);
+        }


let's move it to the new parent class method clearTmpData

Return removed SwaggerDriverInterface.

vitgrams · 2023-03-14T08:24:03Z

@DenTray Fixed. Could you please take a look?

DenTray · 2023-03-21T02:03:32Z

src/Exceptions/InvalidDriverClassException.php

    public function __construct(string $driver)
    {
-        parent::__construct("The driver '{$driver}' is not implements the SwaggerDriverInterface.");
+        parent::__construct("The driver '{$driver}' does not extend the BaseDriver class.");


please revert this one

DenTray · 2023-03-21T02:05:36Z

src/Exceptions/MissedRemoteDocumentationUrlException.php

+{
+    public function __construct($message = null, $code = 0, Exception $previous = null)
+    {
+        $message = $message ?? 'Remote documentation url missed in config';


let's add the information about the env variable name which should be filled to fix it

DenTray · 2023-03-21T02:15:44Z

composer.json

    "php": ">=7.3.0",
    "laravel/framework": ">=5.3.0",
-    "phpunit/phpunit": ">=7.0|<=10.0"
+    "phpunit/phpunit": ">=7.0|<10.0"


@vitgrams as we discussed, let's add this restriction after the swagger 3 will be released

sonarqubecloud · 2023-03-21T09:49:09Z

SonarCloud Quality Gate failed.

2 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

No Coverage information
0.0% Duplication

vitgrams added 3 commits March 7, 2023 11:09

feat: store tmp doc data in file instead of static class property to …

6b022e4

…use Artisan command to push documentation;

tests: added tests;

4453ec7

fix: remove tmp data after saving production data;

76f09fc

vitgrams requested a review from DenTray March 7, 2023 07:43

vitgrams assigned DenTray Mar 7, 2023

vitgrams added type: feature state: code review labels Mar 7, 2023

DenTray requested a review from NikitaYakovlev March 7, 2023 10:01

NikitaYakovlev approved these changes Mar 8, 2023

View reviewed changes

DenTray requested changes Mar 9, 2023

View reviewed changes

DenTray assigned vitgrams and unassigned DenTray Mar 9, 2023

vitgrams force-pushed the 78_save_temp_doc_in_file_for_local_and_storage_drivers branch from 11a1d58 to 76f09fc Compare March 9, 2023 14:51

fix: resolved review conversations;

b613a2b

vitgrams assigned DenTray and unassigned vitgrams Mar 9, 2023

fix: minor fix;

676887a

DenTray requested changes Mar 13, 2023

View reviewed changes

DenTray assigned vitgrams and unassigned DenTray Mar 13, 2023

vitgrams force-pushed the 78_save_temp_doc_in_file_for_local_and_storage_drivers branch from 9a3eb0b to 676887a Compare March 13, 2023 13:13

fix: replaced SwaggerDriverInterface with abstract BaseDriver class, …

2384677

…added MissedRemoteDocumentationUrlException Abstract BaseDriver has been added to unify saveTmpData, getTmpData methods semantic.

DenTray requested changes Mar 14, 2023

View reviewed changes

vitgrams added 2 commits March 14, 2023 12:40

fix: removed composer.lock from .gitignore and rebuilt this one;

f862f2b

fix: resolved review conversations;

b90d8d9

Return removed SwaggerDriverInterface.

vitgrams assigned DenTray Mar 14, 2023

vitgrams removed their assignment Mar 14, 2023

DenTray requested changes Mar 21, 2023

View reviewed changes

fix: resolved review remarks;

7e3b829

vitgrams assigned DenTray and unassigned DenTray Mar 21, 2023

DenTray approved these changes Mar 21, 2023

View reviewed changes

DenTray merged commit 1acfdaa into master Mar 21, 2023

DenTray deleted the 78_save_temp_doc_in_file_for_local_and_storage_drivers branch September 18, 2023 11:40


		namespace RonasIT\Support\AutoDoc\Drivers;

		abstract class BaseDriver

	abstract class BaseDriver
	abstract class BaseDriver implements SwaggerDriverInterface

#78: Store tmp doc data in file instead of static class property to use it in Artisan push-documentation command #80

#78: Store tmp doc data in file instead of static class property to use it in Artisan push-documentation command #80

Uh oh!

Conversation

vitgrams commented Mar 7, 2023

Description

Type of change

Checklist:

Uh oh!

vitgrams commented Mar 7, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vitgrams commented Mar 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DenTray commented Mar 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vitgrams commented Mar 14, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Mar 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants