Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/src/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ public function registerHandler(string $path, RequestHandlerInterface $handler):
$this->handlers[$path] = $handler;
}

/**
* @param ServerWithPathPrefix&RequestHandlerInterface $server
*/
public function registerServer(ServerWithPathPrefix $server): void
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have intersection types here, but they are supported since php8.1, and this will require to drop php7.4 & php8.0 support.

Suggested change
public function registerServer(ServerWithPathPrefix $server): void
public function registerServer(ServerWithPathPrefix&RequestHandlerInterface $server): void

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option that ServerWithPathPrefix should extend RequestHandlerInterface.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given 8.0 has been EOL for almost two years now, I don't think dropping support would be a huge issue.

I'm not against extending the request handler interface either, just make sure it's implemented by the generated servers separately as well (to follow the concept we agreed to in the other PR)

Copy link
Contributor Author

@antonkomarev antonkomarev Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sagikazarmark I suggest to move forward and drop old PHP versions support. It will help us to use all features of the language.

In a 4 days PHP8.5 will be released.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sagikazarmark and I propose to remove deprecated \Twirp\Server class, since it's deprecated for a 4 years already.

{
$this->handlers[$server->getPathPrefix()] = $server;
}

public function handle(ServerRequestInterface $request): ResponseInterface
{
foreach ($this->handlers as $path => $handler) {
Expand Down
15 changes: 15 additions & 0 deletions lib/src/ServerWithPathPrefix.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace Twirp;

interface ServerWithPathPrefix
{
/**
* Returns the base service path, in the form: "/<prefix>/<package>.<Service>/"
* that is everything in a Twirp route except for the <Method>. This can be used for routing,
* for example, to identify the requests that are targeted to this service in a mux.
*/
public function getPathPrefix(): string;
}
5 changes: 4 additions & 1 deletion protoc-gen-twirp_php/templates/service/Server.php.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@ use Twirp\BaseServerHooks;
use Twirp\Context;
use Twirp\ErrorCode;
use Twirp\ServerHooks;
use Twirp\ServerWithPathPrefix;

/**
* @see {{ .Service | phpServiceName .File }}
*
* Generated from protobuf service <code>{{ .Service.Desc.FullName }}</code>
*/
final class {{ .Service | phpServiceName .File }}Server implements RequestHandlerInterface
final class {{ .Service | phpServiceName .File }}Server implements
RequestHandlerInterface,
ServerWithPathPrefix
{
/**
* A convenience constant that may identify URL paths.
Expand Down
Loading