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.

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

Already on GitHub? Sign in to your account

fix: use full url for Swagger Ui bundles urls #29

Merged

Conversation

nathanaelhoun
Copy link
Contributor

Hello! I wanted to use your package on an application hosted in a subdirectory (like https://example.net/my-laravel-application/), but it failed to load the Swagger bundle because the link is generated from the root of the application.

This proposed change fixes this behaviour by using the url() helper to generate a fully qualified link to the openapi file.

I failed to add a unit test 馃槶

I tried to add a unit test to test for this behaviour, but I couldn't get it to work correctly.

  • By default, the url() helper doesn't use the config from app.url in the CLI (as far as I can say)
  • So I added URL::forceRootUrl(config('app.url'));, but then the request gives a 404 as the test request now includes the subdir in it's path
  • The last solution was to override the prepareUrlForRequest() method to remove the url() helper call from it, but I'm not keen to override a framework method. It works though.
    /** @test */
    public function it_supports_application_sub_dir_and_swagger_sub_path()
    {
        config()->set('app.url', 'http://localhost/application-subdir/');

        // https://github.com/laravel/framework/issues/18613#issuecomment-342033733
        URL::forceRootUrl(config('app.url'));

        $this->assertEquals('http://localhost/application-subdir/path/with/multiple/segments/swagger-with-versions', url('path/with/multiple/segments/swagger-with-versions'));

        $this->get('/path/with/multiple/segments/swagger-with-versions')
            ->assertStatus(200)
            ->assertSee('url: \'http://localhost/application-subdir/path/with/multiple/segments/swagger-with-versions/v1\'', false)
            ->assertSee('url: \'http://localhost/application-subdir/path/with/multiple/segments/swagger-with-versions/v2\'', false);
    }

    /**
     * @overrides Do not use the `url()` helper, because the router doesn't use the setting from config('app.url')
     */
    protected function prepareUrlForRequest($uri)
    {
        if (str_starts_with($uri, '/')) {
            $uri = substr($uri, 1);
        }

        return trim($uri, '/');
    }

@nathanaelhoun
Copy link
Contributor Author

Hi @gdebrauwer, is there anything that I can do to help getting this PR reviewed?

@gdebrauwer gdebrauwer merged commit 2e0dd61 into nextapps-be:master Mar 20, 2024
15 checks passed
@gdebrauwer
Copy link
Member

Lost track of this PR 馃槃

Fixed in v0.9.2

@nathanaelhoun nathanaelhoun deleted the fix-application-in-subpath branch March 20, 2024 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants