Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Adds factories for all middleware and a ConfigProvider #50

Conversation

weierophinney
Copy link
Member

This patch accomplishes three things:

  • It moves all middleware into a subnamespace, Zend\Expressive\Router\Middleware. This is done due to the large number of middleware now provided (six!), and the fact that each will have a factory.
  • Adds factories for all provided middleware. In some cases, these depend on virtual services (e.g., response prototypes, callable factories). In those cases, per discussion in Suggestion for '<Dispatch|Route>Middleware::class' in 3.0alpha zend-expressive-skeleton#220, I've added package-level constants, and have the factories look for services named after those constant values, raising an exception if missing.
  • Adds a ConfigProvider class that defines factory entries for each middleware, and registers the provider in the package.

We have a lot of middleware at this point, so they need to be moved to a
subnamespace.
A number of factories depend on "virtual" services: responses, stream
factories, etc. To define these services, I have created namespaced
constants that define each, and provide documentation for what they
represent. The individual factories then refer to these, and raise
exceptions if they are missing.

All middleware defined by this package now have factories present.
Defines a `ConfigProvider` class that registers factory entries for all
provided middleware, and adds it to the package definition.
@geerteltink
Copy link
Member

Interesting solution to use the constants file. I thought you would use constants in the factories but this is a better solution. I only find the uppercase names ugly. But as they are constants you should, otherwise you still have confusion.

Copy link
Member

@michalbundyra michalbundyra left a comment

Choose a reason for hiding this comment

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

LGTM 👍


public function getDependencies() : array
{
// @codingStandardsIgnoreStart
Copy link
Member

Choose a reason for hiding this comment

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

Why? Are lines too long?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. :)

@weierophinney
Copy link
Member Author

Interesting solution to use the constants file. I thought you would use constants in the factories but this is a better solution. I only find the uppercase names ugly. But as they are constants you should, otherwise you still have confusion.

Yeah, I prototyped originally with constants in the factories, and realized that was a poor solution when NOT using those factories. The PSR-1/2 and PSR-12 guides require constants be all-caps, underscore_separated, so I went with that style, and the values... well, seems like having them be equivalent to the constant name made as much sense as anything. I was happy with the end-result, particularly as these will essentially be inlined in the autoloader in production.

@weierophinney weierophinney merged commit 8ad664e into zendframework:release-3.0.0 Feb 13, 2018
weierophinney added a commit that referenced this pull request Feb 13, 2018
@weierophinney weierophinney deleted the feature/factories-and-config-provider branch February 13, 2018 20:45
weierophinney added a commit to weierophinney/zend-expressive-router that referenced this pull request Mar 14, 2018
Combines it with the entries for zendframework#47 and zendframework#50.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants