feat: Store documentation file on a configurable directory by hans-thomas · Pull Request #152 · RonasIT/laravel-swagger

hans-thomas · 2024-12-13T09:45:54Z

Hi, it adds the ability to store the generated documentation file in a configurable directory.

TODO list:

Local and Storage drivers should have new config, named directory with the default value documentations
the production_path config for Local and Storage drivers should be renamed to base_file_name with the default value documentation (without .json extension)
documentation file extension should be automatically extended by the .json extension
Local driver should check if directory exists and create it if not (on documentation saving)
Storage driver should create storage directory only for non Cloud drivers (on documentation saving)
Config version should be increased to force developers republish package config

DenTray · 2024-12-16T06:40:16Z

@hans-thomas I've updated task description, please add required changes accordingly the new task description (please feel free to open few PRs if you can divide task for small subtasks without breaking the package work)

DenTray · 2024-12-16T06:53:22Z

@hans-thomas please do not use autoclosing issues as we need to test task before closing

The `production_path` key renamed to `base_file_name` for both local and storage drivers;

…xistance;

src/Drivers/StorageDriver.php

hans-thomas · 2025-01-08T08:26:11Z

Hi @DenTray, would you please review this PR?

src/Drivers/LocalDriver.php

src/Drivers/StorageDriver.php

tests/AutoDocControllerTest.php

tests/LocalDriverTest.php

DenTray · 2025-01-27T08:16:28Z

src/Drivers/LocalDriver.php

+        $this->config = config('auto-doc.drivers.local');

-        if (empty($this->prodFilePath)) {
+        $directory = str_ends_with($this->config['directory'], DIRECTORY_SEPARATOR)


Suggested change

$directory = str_ends_with($this->config['directory'], DIRECTORY_SEPARATOR)

$directory = (str_ends_with($this->config['directory'], DIRECTORY_SEPARATOR))

I made this change but I can't understand the difference.

src/Drivers/LocalDriver.php

DenTray · 2025-01-27T08:21:30Z

src/Drivers/StorageDriver.php

+        $this->disk = Storage::disk($this->config['disk']);

-        if (empty($this->prodFilePath)) {
+        $directory = str_ends_with($this->config['directory'], DIRECTORY_SEPARATOR)


as you already did the same in the local driver, let's move this logic to the BaseDriver class

Suggested change

$directory = str_ends_with($this->config['directory'], DIRECTORY_SEPARATOR)

$directory = (str_ends_with($this->config['directory'], DIRECTORY_SEPARATOR))

src/Drivers/StorageDriver.php

tests/AutoDocControllerTest.php

Force set documentation directory as empty for AutoDocControllerTest;

src/Drivers/LocalDriver.php

DenTray · 2025-02-03T06:36:44Z

src/Drivers/StorageDriver.php

+        $this->disk = Storage::disk($this->config['disk']);

-        if (empty($this->prodFilePath)) {
+        $directory = (str_ends_with($this->config['directory'], DIRECTORY_SEPARATOR))


not sure this logic required for the Storage driver as it works correctly for all next cases:

Storage::put('file.txt', 'content')

Storage::put('/path/file.txt', 'content')

Storage::put('path/file.txt', 'content')

Storage::put('/file.txt', 'content')

Storage::put('//file.txt', 'content')

That logic was preventing this case:

Storage::put('documentations//documentation.json', 'content')

However, it's handling this case correctly, too.

tests/AutoDocControllerTest.php

tests/LocalDriverTest.php

DenTray · 2025-02-03T06:46:37Z

tests/LocalDriverTest.php

+        $driver = new LocalDriver();
+        $driver->saveTmpData(self::$tmpData);
+
+        $this->assertFileExists(self::$tmpDocumentationFilePath);


Suggested change

$this->assertFileExists(self::$tmpDocumentationFilePath);

$this->assertFileExists(storage_path('documentations/documentation.json'));

These static properties are used several times in this class and I believe they are beneficial to keep tests simpler and improve the readability.

tests/LocalDriverTest.php

tests/StorageDriverTest.php

sonarqubecloud · 2025-02-08T17:02:43Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

hans-thomas · 2025-03-09T07:06:23Z

Hi @DenTray, please review this PR. If everything is okay, I will cover the new lines.

Copilot

Pull Request Overview

This PR introduces configurable directory support for documentation file storage, allowing users to specify where generated documentation files are saved. The changes refactor both Local and Storage drivers to support directory configuration while maintaining backward compatibility.

Adds a new directory configuration option with default value documentations
Renames production_path to base_file_name with automatic .json extension handling
Implements directory creation logic for the Local driver when directories don't exist

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
config/auto-doc.php	Updates configuration structure with new `directory` and `base_file_name` options, increments config version
src/Drivers/LocalDriver.php	Implements directory handling logic and automatic directory creation
src/Drivers/StorageDriver.php	Refactors to use new configuration structure with directory support
tests/LocalDriverTest.php	Updates tests to use new configuration and adds directory creation test coverage
tests/StorageDriverTest.php	Updates tests for new configuration structure and adds directory separator handling test
tests/RemoteDriverTest.php	Updates temporary file path to use storage_path helper
tests/AutoDocControllerTest.php	Updates tests to use new configuration options

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-28T09:38:54Z

src/Drivers/StorageDriver.php

+        $this->disk = Storage::disk($this->config['disk']);

-        if (empty($this->prodFilePath)) {
+        $directory = $this->config['directory'] . DIRECTORY_SEPARATOR;


The directory separator is unconditionally added, which will create paths like 'documentations//' when the directory already ends with a separator. This should check if the directory already ends with a separator first, similar to the LocalDriver implementation.

Suggested change

$directory = $this->config['directory'] . DIRECTORY_SEPARATOR;

$directory = $this->config['directory'];

if (!str_ends_with($directory, DIRECTORY_SEPARATOR)) {

$directory .= DIRECTORY_SEPARATOR;

}

Copilot · 2025-08-28T09:38:55Z

src/Drivers/LocalDriver.php

    public function saveData(): void
    {
-        file_put_contents($this->prodFilePath, json_encode($this->getTmpData()));
+        $documentationDirectory = storage_path($this->config['directory']);


The directory path construction is inconsistent with the mainFilePath construction. When config['directory'] already ends with DIRECTORY_SEPARATOR, this will create the wrong directory path. Should use the same normalized $directory variable from lines 19-21.

Copilot · 2025-08-28T09:38:55Z

tests/LocalDriverTest.php

+        $documentationDirectory = 'test_directory';
+        if (is_dir($documentationDirectory)) {


The test uses a hardcoded directory name that doesn't match the actual configuration being tested. This should use the same directory configuration as other tests for consistency and to properly test the actual functionality.

Suggested change

$documentationDirectory = 'test_directory';

if (is_dir($documentationDirectory)) {

$documentationDirectory = config('auto-doc.drivers.local.directory');

if (is_dir(storage_path($documentationDirectory))) {

Copilot · 2025-08-28T09:38:55Z

tests/LocalDriverTest.php

+    {
+        $documentationDirectory = 'test_directory';
+        if (is_dir($documentationDirectory)) {
+            rmdir(storage_path($documentationDirectory));


Using rmdir() without checking if the directory is empty will fail if the directory contains files. Should use a recursive directory removal or ensure the directory is empty before attempting to remove it.

hans-thomas added 5 commits December 13, 2024 13:05

Add documentation_directory key to the config file;

748d424

Update drivers;

cc6d056

Fix tests;

4e1970f

Increase config version;

7d08de0

Fix No such file or directory error on running tests;

a97158e

DenTray assigned hans-thomas Dec 16, 2024

hans-thomas added 5 commits December 20, 2024 19:41

The directory config key added to local and storage drivers;

10705ea

The production_path key renamed;

8d25c0f

The `production_path` key renamed to `base_file_name` for both local and storage drivers;

Add .json extension to base_file_name attr and check target dir e…

db649e9

…xistance;

Update tests;

9dd4951

Wip;

0fa0b4b

hans-thomas commented Dec 28, 2024

View reviewed changes

src/Drivers/StorageDriver.php Outdated Show resolved Hide resolved

DenTray requested changes Jan 21, 2025

View reviewed changes

Making suggested changes;

4a387c2

hans-thomas requested a review from DenTray January 21, 2025 08:23

Apply changes;

b665010

DenTray requested changes Jan 27, 2025

View reviewed changes

hans-thomas added 2 commits February 2, 2025 21:13

Apply changes;

ebef239

Set empty directory in AutoDocControllerTest;

cef046b

Force set documentation directory as empty for AutoDocControllerTest;

hans-thomas force-pushed the doc-file-on-configurable-dir branch from ee90fc7 to cef046b Compare February 2, 2025 17:53

hans-thomas requested a review from DenTray February 2, 2025 17:54

Cover new lines;

83b43b3

DenTray requested changes Feb 3, 2025

View reviewed changes

Apply new changes;

999f2c2

hans-thomas requested a review from DenTray February 20, 2025 06:56

astorozhevsky requested a review from Copilot August 28, 2025 09:37

Copilot AI reviewed Aug 28, 2025

View reviewed changes

	$directory = str_ends_with($this->config['directory'], DIRECTORY_SEPARATOR)
	$directory = (str_ends_with($this->config['directory'], DIRECTORY_SEPARATOR))

	$this->assertFileExists(self::$tmpDocumentationFilePath);
	$this->assertFileExists(storage_path('documentations/documentation.json'));

-        $directory = $this->config['directory'] . DIRECTORY_SEPARATOR;
+        $directory = $this->config['directory'];
+        if (!str_ends_with($directory, DIRECTORY_SEPARATOR)) {
+            $directory .= DIRECTORY_SEPARATOR;
+        }

		$documentationDirectory = 'test_directory';
		if (is_dir($documentationDirectory)) {

Conversation

hans-thomas commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DenTray commented Dec 16, 2024

Uh oh!

DenTray commented Dec 16, 2024

Uh oh!

Uh oh!

hans-thomas commented Jan 8, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DenTray Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

hans-thomas Feb 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DenTray Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DenTray Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

hans-thomas Feb 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DenTray Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

hans-thomas Feb 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Feb 8, 2025

Quality Gate passed

Uh oh!

hans-thomas commented Mar 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

hans-thomas commented Dec 13, 2024 •

edited

Loading