twirphp / twirp

PHP port of Twitch's Twirp RPC framework
https://twirphp.github.io
MIT License
155 stars 19 forks source link

Path prefix doesn't work #126

Open ilzrv opened 1 year ago

ilzrv commented 1 year ago

Preflight Checklist

Version

v.0.9.1

PHP version

7.4

Go version

No response

Expected Behavior

When the path prefix is changed, the library continues to work.

Actual Behavior

When changing the path prefix, the library does not work.

Steps To Reproduce

  1. Add to the .proto file package apis.inter.services;
  2. Register a handler
    $this->router->registerHandler(
    MyServer::PATH_PREFIX,
    new MyServer($this->service)
    );
  3. After generating the MyServer class in the PATH_PREFIX constant:
    final class MyServer implements RequestHandlerInterface
    {
    const PATH_PREFIX = '/twirp/apis.inter.services.My/';
    //...
  4. But below the code (MyServer) we have a switch:

    switch ($req->getUri()->getPath()) {
    case '/twirp/apis.inter.services.My/Create': // <-- full path?
        return $this->handleCreate($ctx, $req);
    
    default:
        return $this->writeError($ctx, $this->noRouteError($req));
    }

    This switch does not use path prefix from constructor in ANY way, which we specify when registering the handler. Looks like a serious mistake. Why pass a prefix if it's always the same?

Additional Information

No response

sagikazarmark commented 1 year ago

This is probably a regression we introduced when upgrading from v6 spec to v7. v7 made the prefix optional...I guess we did not change all necessary code to reflect that.

Let me look into it.

sagikazarmark commented 1 year ago

Actually, @ilzrv are you sure you are using 0.9.1?

As far as I can tell, the prefix change was merged way before that version.

Also, I'm looking at code generated by that version (downloaded from the releases page) and the routing mechanism looks completely different.

Can you please confirm your plugin version by running protoc-gen-twirp_php --version?

Thank you!

ilzrv commented 1 year ago

The result of executing the ./proto/bin/protoc-gen-twirp_php --version command is: 0xc0000296b0