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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reopening #748: UrlGenerator warming up issue #784

Closed
gazzoy opened this issue Dec 21, 2023 · 8 comments
Closed

Reopening #748: UrlGenerator warming up issue #784

gazzoy opened this issue Dec 21, 2023 · 8 comments
Assignees

Comments

@gazzoy
Copy link
Contributor

gazzoy commented Dec 21, 2023

Octane Version

2.2.3

Laravel Version

10.38.1

PHP Version

8.2.13

What server type are you using?

Swoole

Server Version

v2023.2.2

Database Driver & Version

No response

Description

See #748.

In addition to that, I'm trying to reproduce this issue on the test as below but I can't. As for some reason $app['router']->bind() doesn't work. Could you tell me where I should change please?

    public function test_url_generator_creates_correct_urls_across_subsequent_with_bind()
    {
        [$app, $worker, $client] = $this->createOctaneContext([
            Request::create('/dashboard/foo', 'GET'),
            Request::create('/dashboard/bar', 'GET'),
        ]);

        $app['router']->bind('param', function (string $value)
        {
            return URL::defaults(['param' => strtoupper($value)]);
        });

        $app['router']->get('dashboard/{param}', function (Application $app)
        {
            return URL::getDefaultParameters()['param'];
        })->name('dashboard');

        $worker->run();

        $this->assertEquals('FOO', $client->responses[0]->getContent());
        $this->assertEquals('BAR', $client->responses[1]->getContent());
    }

Steps To Reproduce

See #748.

@gazzoy
Copy link
Contributor Author

gazzoy commented Dec 27, 2023

I was trying to write the test for this too, but everything is working fine in tests.
It seems that the application is loaded differently in tests than in real-world apps.

Hi @michael-rubel, Would it be possible to you to share the test code you mentiond like above on #748 ?

Just wondering what kind of test you had as I've been stucking to reproduce the issue.

@michael-rubel
Copy link

Hi @gazzoy

I removed the branch already. I was applying similar code as in other Octane tests, but this issue cannot be covered because of the way the instance is bootstrapped. Octane needs an integration test suite. The one that will boot the real Swoole/Roadrunner/FrankenPHP instance.

@gazzoy
Copy link
Contributor Author

gazzoy commented Dec 28, 2023

@michael-rubel Thank you! I see, seems a bit complicated to reproduce the issue then.

Actually my problem was I couldn't even write a successful test case for this, but most likely I'm doing something wrong so will have another look.

@gazzoy
Copy link
Contributor Author

gazzoy commented Jan 6, 2024

Hi @nunomaduro

As @michael-rubel suggested, most likely we need to set up an integration test suite to reproduce this issue. However, I'm not too sure how to do it in octane repository. Do you happen to have any ideas? Using sail or testbench-dusk doesn't seem a good option.

@nunomaduro
Copy link
Member

@gazzoy, could you confirm if the scenario described in #807 is the one causing issues in your application? Meaning that the URL::defaults state is being shared between requests?

@gazzoy
Copy link
Contributor Author

gazzoy commented Jan 9, 2024

Thank you @nunomaduro. Correct me if I'm wrong, but I don't believe #807 is the case I've been facing, as in my case I define Router::bind with calling URL::defaults along with Router::get.

To be more specific, my case is as follows. I believe the original issue #748 was too:
https://github.com/gazzoy/octane/blob/fix/url-same-instance/tests/UrlGeneratorStateTest.php

The problem here is in above test both requests returns BAR and WIBBLE as expected, but in the real world Octane always returns BAR which seems URL::defaults keeps its state.

Please let me know if you have any questions.

@nunomaduro
Copy link
Member

Got it! In that case, please provide an open-source repository with a fresh Laravel installation with Octane, including one or two routes that face this issue. I need something I can clone locally to reproduce your problem. Thank you for your patience with this matter. Once you've set it up, create a new issue and include the repository link and instructions.

@gazzoy
Copy link
Contributor Author

gazzoy commented Jan 11, 2024

Got it! In that case, please provide an open-source repository with a fresh Laravel installation with Octane, including one or two routes that face this issue. I need something I can clone locally to reproduce your problem. Thank you for your patience with this matter. Once you've set it up, create a new issue and include the repository link and instructions.

No worries. Will do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants