Skip to content

Conversation

@rdohms
Copy link
Contributor

@rdohms rdohms commented Jun 19, 2020

Alright, taking previous work further, opening this as a Draft to start validating steps.

For now I isolated all definition creation to use the class/interface and add an alias to the previous name based on applicationName.

Next up I'll look what is static enough to be extracted to an XML.

For reference:

$aliases[$this->applicationName . '.http.route.' . $route['route_name']]  = 'http.route.' . $route['route_name'];
$aliases[$this->applicationName . '.http.middleware_container']           = MiddlewareContainer::class;
$aliases[$this->applicationName . '.http.middleware_factory']             = MiddlewareFactory::class;
$aliases[$this->applicationName . '.http.middleware_pipeline']            = MiddlewarePipe::class;
$aliases[$this->applicationName . '.http.router']                         = FastRouteRouter::class;
$aliases[$this->applicationName . '.http.uri_generator']                  = UriGeneratorInterface::class;
$aliases[$this->applicationName . '.http.route_collector']                = RouteCollector::class;
$aliases[$this->applicationName . '.http.middleware.route']               = RouteMiddleware::class;
$aliases[$this->applicationName . '.http.middleware.implicit_head']       = ImplicitHeadMiddleware::class;
$aliases[$this->applicationName . '.http.middleware.content_negotiation'] = ContentTypeMiddleware::class;
$aliases[$this->applicationName . '.http.request_handler_runner']         = RequestHandlerRunner::class;
$aliases[$this->applicationName . '.http_expressive']                     = Expressive::class;
$aliases[$this->applicationName . '.http']                                = ApplicationInterface::class;

@rdohms rdohms added the Improvement Improvement of existing feature label Jun 19, 2020
@rdohms rdohms requested a review from lcobucci June 20, 2020 17:51
@rdohms
Copy link
Contributor Author

rdohms commented Jun 21, 2020

Preliminary into what can go to xml

// dynamic                                                                                        
$aliases[$this->applicationName . '.http.route.' . $route['route_name']]  = 'http.route.' . $route
$aliases[$this->applicationName . '.http.middleware_container']           = MiddlewareContainer::c
$aliases[$this->applicationName . '.http.middleware_pipeline']            = MiddlewarePipe::class;
$aliases[$this->applicationName . '.http.route_collector']                = RouteCollector::class;
                                                                                                  
// depends on legacy paramter                                                                     
$aliases[$this->applicationName . '.http.router']                         = FastRouteRouter::class
$aliases[$this->applicationName . '.http.middleware.content_negotiation'] = ContentTypeMiddleware:
                                                                                                  
// static                                                                                         
$aliases[$this->applicationName . '.http.middleware_factory']             = MiddlewareFactory::cla
$aliases[$this->applicationName . '.http.uri_generator']                  = UriGeneratorInterface:
$aliases[$this->applicationName . '.http.middleware.route']               = RouteMiddleware::class
$aliases[$this->applicationName . '.http.middleware.implicit_head']       = ImplicitHeadMiddleware
$aliases[$this->applicationName . '.http.request_handler_runner']         = RequestHandlerRunner::
$aliases[$this->applicationName . '.http_expressive']                     = Expressive::class;                                                                                                

@rdohms rdohms linked an issue Feb 5, 2021 that may be closed by this pull request
@rdohms
Copy link
Contributor Author

rdohms commented Feb 6, 2021

Gonna try and pick this up once more tomorrow hopefully.

Base automatically changed from master to 0.4.x February 7, 2021 14:23
@rdohms rdohms force-pushed the feature/remove-multi-application-registration-v2 branch 2 times, most recently from 75584d9 to 5aed642 Compare February 7, 2021 16:16
@rdohms
Copy link
Contributor Author

rdohms commented Feb 7, 2021

Rebased on 0.4.0

Let's pair later to understand what else we want to do and remove the scaffolding.
and I'll organize the commits.

@rdohms
Copy link
Contributor Author

rdohms commented Feb 11, 2021

From research these are the services i see being used by name/alias:

  • ??.http
  • ??.command_bus
  • ??.query_bus
  • ??.http.router

Parameters:

  • ??.allowed_formats

Replacing all with a Class reference is welcome, its just BC so we need to decide how to handle deprecating it.

@lcobucci lcobucci added this to the 0.5.0 milestone Feb 21, 2021
@lcobucci lcobucci force-pushed the feature/remove-multi-application-registration-v2 branch from 5aed642 to ebe9d13 Compare September 16, 2021 20:14
@lcobucci lcobucci changed the base branch from 0.4.x to 0.5.x September 16, 2021 20:14
@lcobucci
Copy link
Member

I've just rebased this on top of 0.5.x... I'll apply the changes to the Mezzio implementation so we can rely on the functional tests to make sure we don't break stuff and to say "buh bye" to Expressive.

In order to simplify the container generation process, let's stop allowing the
registration of multiple apps in a container and stick to a single one.

A side effect of these changes is that we can statically create the XML files
for the services created in the compiler passes. We looked into it but we
couldn't do it in a time-efficient manner.
@lcobucci lcobucci force-pushed the feature/remove-multi-application-registration-v2 branch from ebe9d13 to 1f3534a Compare November 30, 2021 20:29
All service Ids that use ApplicationName are now replaced with the
proper class or interface, and an alias is set to the previous name.
@lcobucci lcobucci force-pushed the feature/remove-multi-application-registration-v2 branch from 1f3534a to 5de5fd5 Compare November 30, 2021 21:06
Making sure everything still works as expected on the functional tests.
The Expressive implementation is getting in the way of keeping a sane
amount here.

We'll review these requirements once the deprecated component is
removed.
@lcobucci lcobucci force-pushed the feature/remove-multi-application-registration-v2 branch from 5de5fd5 to 1334575 Compare November 30, 2021 21:33
@lcobucci lcobucci changed the title Remove Multi-Application support Remove Multi-Application support from HTTP registration Nov 30, 2021
@lcobucci lcobucci self-assigned this Nov 30, 2021
Copy link
Member

@lcobucci lcobucci left a comment

Choose a reason for hiding this comment

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

I've reorganised a few things here and there and made the test suite pass with the Mezzio implementation.

The current failures will be fixed in another PR (as remove zend expressive and upgrade dependencies).

@lcobucci lcobucci marked this pull request as ready for review November 30, 2021 21:39
@lcobucci
Copy link
Member

I'll merge this so we can continue the clean-up.
Thanks @rdohms

@lcobucci lcobucci merged commit 2d8436d into 0.5.x Nov 30, 2021
@lcobucci lcobucci deleted the feature/remove-multi-application-registration-v2 branch November 30, 2021 21:40
@lcobucci lcobucci modified the milestones: 0.5.0, 1.0.0 Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Improvement Improvement of existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove multi-application registration

4 participants