Simplify Server registration v2

[antonkomarev]

Overview

Replacement for the:

• Simplify Server registration v1 #205

Simplify registration of server, because PATH_PREFIX constant declares:

Should be used with caution, it only matches routes with the default "/twirp" prefix and default CamelCase service and method names. Use EmployeeServiceServer::getPathPrefix instead.

What this PR does / why we need it

Instead of this:

$this->router->registerHandler(
    EmployeeServiceServer::PATH_PREFIX,
    new EmployeeServiceServer(svc: $this->employeeService),
);

Or this:

$employeeServiceServer = new EmployeeServiceServer(
    svc: $this->employeeService,
);

$this->router->registerHandler(
    $employeeServiceServer->getPathPrefix(),
    $employeeServiceServer,
);

We can do:

$this->router->registerServer(
    new EmployeeServiceServer(
        svc: $this->employeeService,
    )
);

Special notes for your reviewer

I tend to add an *Interface suffix to the interface files, but didn't see that prefix in current local interfaces, so omitted it.

Does this PR introduce a user-facing change?

New ability to register server without manually define server path.

[antonkomarev]

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

[antonkomarev]

Another option that ServerWithPathPrefix should extend RequestHandlerInterface.

[sagikazarmark]

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)

[antonkomarev]

@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.

[antonkomarev]

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

[antonkomarev]

@sagikazarmark @SpencerMalone what do you think about proposed interface name?

And do we add *Interface suffix at the end of interface name, or not? PSR packages has this suffix for example.

[SpencerMalone]

Idiomatic PHP I think is to have the suffix, even though I don't personally love it (but that's the golang dev in me talking). If it were me, I would probablyyyy leave the suffix off, but honestly don't feel strongly enough to push one way or another.

[SpencerMalone]

Not a maintainer but this LGTM

[antonkomarev]

@sagikazarmark what's your opinion? I'd better go with Interface suffix

[sagikazarmark]

I always thought that Interface suffix is an ugly code smell people invented because they fundamentally misunderstand interfaces.

Normally I wouldn't fight it, because I know it's a standard in the PHP community, but the rest of the interfaces in the library don't use suffixes (looks like I wrote this lib during one of my rebel periods :) ), so for that reason alone (consistency), I'd say drop the suffix.

Otherwise, LGTM

[antonkomarev]

@sagikazarmark renamed interface

[sagikazarmark]

Thanks @antonkomarev

Sorry it took some time for me to come around