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

Add support for non-pretty URLs #8

Open
cboulanger opened this issue Feb 17, 2020 · 11 comments
Open

Add support for non-pretty URLs #8

cboulanger opened this issue Feb 17, 2020 · 11 comments
Assignees
Labels

Comments

@cboulanger
Copy link
Contributor

cboulanger commented Feb 17, 2020

Hi, I am just evaluating your library, to replace the no-longer maintained cranetm/yii2-json-rpc-2.0.

I am running functional tests with codeception, making the JSORPC calls to the backend.

Unfortunately, I haven't been successful yet, I am getting Method georgique\\yii2\\jsonrpc\\Action::run() does not exist without any usable stack trace.

Do you have an idea what could be wrong?

@cboulanger
Copy link
Contributor Author

cboulanger commented Feb 17, 2020

Looking through the source, I can see that in https://github.com/georgique/yii2-jsonrpc/blob/master/JsonRpcRequest.php#L157 , the route is re-parsed even though the method has already been determined. If I replace $routeParsed with $route in https://github.com/georgique/yii2-jsonrpc/blob/master/JsonRpcRequest.php#L187 , the error goes away (I have a different error now, but that's a different question). Why would you call the action on the original route, which is to the JsonRpcController instead of on the new route that is parsed from the JsonRpc method call?

@georgique
Copy link
Owner

Hi @cboulanger, I don't think your conclusion is correct. Line 157 is needed for splitting the route from query parameters, so can't really say it's "reparsed". Would you be able to show values for your $routeParsed and $route at the moment of line 187 execution?

@cboulanger
Copy link
Contributor Author

cboulanger commented Feb 18, 2020

Hi, thanks for looking into this! Here's a screenshot of my IDE - I am using my fork which replaces $routeParsed with $route(because it wouldn't work otherwise). Thw screenshot shows the content of both variables: $routeParsed (incorrectly) contains /json-rpc, whereas $route (correctly) contains access/username.

Bildschirmfoto 2020-02-18 um 09 01 19

@cboulanger
Copy link
Contributor Author

It might have something to do that these are codeception functional tests that work without a webserver. But they should work anyways. Can you explain what $routeParsed is supposed to contain that is not already in $route? Because I couldn't figure that out...

@georgique
Copy link
Owner

@cboulanger Basically $routeParsed has to contain URI before "?" mark. I.e. for $route = "user/view?id=1", $routeParsed should become "user/view" and $params should become ['id' = 1].
From the screenshot I can see that it doesn't work properly for you. I guess it's because of codeception.
I would look into it, but would you be able to chare your codeception configuration so I can reproduce the problem on my side? Also please provide your Yii version.

@cboulanger
Copy link
Contributor Author

cboulanger commented Feb 18, 2020

ok, I think I have an idea what could be the problem. Since my Yii2 app is an API backend and not a html web application which shows the URL, I am not using pretty urls (domain.com/endpoint/json-rpc), but the simple domain.com/endpoint/?r=json-rpc configuration. I assume your jsonrpc implementation assumes a pretty url syntax ...

@cboulanger
Copy link
Contributor Author

versions: yii ^2.0.32, codeception ^4.0, codeception config is here. But very likely the problem is the non-pretty-url stuff. I can try and enable it to see if the problem goes away.

@georgique georgique self-assigned this Feb 18, 2020
@georgique
Copy link
Owner

Non-pretty urls explanation sounds like it. If you can try with pretty urls and let me know - that'd be helpful.

@cboulanger
Copy link
Contributor Author

I just enabled pretty urls and after reverting the changes in my fork, my tests started to pass with your original code! It would be great if you could also support the "default URL format". I have one request to add a catch block for yii\web\UnauthorizedHttpException, I'll submit a PR for that.

@georgique georgique changed the title Method georgique\\yii2\\jsonrpc\\Action::run() does not exist Add support for non-pretty URLs Feb 18, 2020
@georgique georgique added the bug label Feb 18, 2020
@georgique
Copy link
Owner

@cboulanger I'll work on it. Will try to do as soon as possible

@cboulanger
Copy link
Contributor Author

I am still having problems, even with pretty URLs turned on. This time using acceptance tests, where the codeception test suite makes a POST request to the web server. The JSONRPC method is correctly translated into $route, but $routeWithParams = $app->request->resolve(); then reverts to the /json-rpc Yii2 controller path. I still don't understand why you need to re-read the route from the original request when you already have it in $route? Unfortunately, I have to go back to my fork :-( which replaces $routeParsed with $route... I wonder what I am doing wrong.

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

No branches or pull requests

2 participants