Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to ServiceManager 4 #834

Open
TomHAnderson opened this issue Mar 5, 2024 · 16 comments
Open

Upgrade to ServiceManager 4 #834

TomHAnderson opened this issue Mar 5, 2024 · 16 comments
Milestone

Comments

@TomHAnderson
Copy link
Member

No description provided.

@TomHAnderson TomHAnderson added this to the 7.0.0 milestone Mar 5, 2024
@TomHAnderson
Copy link
Member Author

This and #833 are on hold pending laminas-cache 4.0.0

@boesing
Copy link
Contributor

boesing commented Jun 23, 2024

@TomHAnderson
Copy link
Member Author

TomHAnderson commented Jul 6, 2024

https://github.com/laminas/laminas-cache-storage-adapter-redis/blob/3.1.x/composer.json#L12

There is a release for the redis adapter but not on the filesystem and memory adapters that DoctrineModule uses. This major relesae is delayed until we can get a working composer.json

Here's how that looks today: https://github.com/TomHAnderson/DoctrineModule/blob/feature/composer-versions/composer.json

@boesing
Copy link
Contributor

boesing commented Jul 7, 2024

Id recommend not requiring explicit adapters, but only laminas-cache. recent major does not require an Adapter anymore. let projects decide which adapters they need.

There will be support in those adapters at some time but that should not affect this repo imho

@TomHAnderson
Copy link
Member Author

The approach I'd like to take for a new major release is modifying the application rather than rewriting it. I appreciate the ideas for cache adapters. But there are more places in Laminas that don't support one another.

- laminas/laminas-i18n 2.27.1 requires laminas/laminas-servicemanager ^3.21.0 -> found laminas/laminas-servicemanager[3.21.0, 3.22.0, 3.22.1] but it conflicts with your root composer.json require (^4.2.0).

- laminas/laminas-session 2.21.0 requires laminas/laminas-servicemanager ^3.22 -> found laminas/laminas-servicemanager[3.22.0, 3.22.1] but it conflicts with your root composer.json require (^4.2.0).

These are just two examples. You can see the whole list by removing the cache adapters from the composer.json and trying to update vendors. @boesing what approach would you recommend? I want this library to stay true to Laminas but Laminas is a slow-moving framework that doesn't keep up with itself.

@boesing
Copy link
Contributor

boesing commented Jul 8, 2024

I want this library to stay true to Laminas but Laminas is a slow-moving framework that doesn't keep up with itself.

Laminas is slow moving as there was a lack of contributions. If u have a need for these libraries to support SM v4, you could start with contributing that migration. Frameworks live from contributions.

Regarding laminas-i18n and laminas-session:

As far as I can see, neither internationalization nor session stuff is frequently used in mezzio and since laminas-mvc will get abandoned in a foreseeable future, I doubt that these components will get bumped to SM v4 at all. The doctrine module depends on laminas-mvc - so this module will most probably not support SM v4 as MVC most probably won't support it.

A few suggestions from my side:

  • Remove laminas-mvc as a dependency (why is it even required, I do not see any namespace usage in src/ directory)
    • it is required in tests but I do not see any usage of the mocked classes - there are method calls configured but since any method calls are allowed, tests do not fail... might be time to drop these
    • Refactor ServiceManagerFactory to remove module loader magic and instead use config-aggregator to merge multiple ConfigProvider from those "modules" configured in that default config
  • Remove laminas-modulemanager as a dependency, when providing support for mezzio and MVC, the modulemanager is not recommended as it only provides features to MVC.
    • By having a method Module#getConfig, the modulemanager will still merge the config even tho the ConfigProviderInterface is not implemented. See laminas-cache Module.php for example
  • Remove laminas-i18n as a dependency (same question, why is it required if there is no namespace requirement)
  • Remove laminas-log from dev dependencies (same question, why is it required if there is no namespace requirement)
  • Remove laminas-cache-storage-adapter-* (let projects decide themselves and/or detect availability using composer runtime API and only add those adapters to the config once installed)
  • Move laminas/laminas-form to dedicated satellite such as doctrine/laminas-form-elements or sth. similar
    • Was not even aware that this is a thing, used this module for quite some time (I quit my job recently so I am not working with this module anymore and in mezzio context I even switched to the roave one as it is more lightweight). However, there is a dedicated ORM module, so I guess there could be a more specialized module for boundaries such as form, etc. which is not providing doctrine specific features but the other way around.
    • I do not think that there will be a new major version of laminas-form in the forseeable future, there might be other components which will get migrated first and thats why I think it would be good to extract these "features" from this component
  • Remove laminas-session as a dependency (its just a transitive dependency due to the usage of laminas-authentication)
  • Remove DoctrineModule\Authentication\Storage\Session as a default storage adapter in the Authentication options, require this option instead to let projects explicitly selecting the storage (that helps removing the laminas-session dependency tho)

This is just an idea on how I'd try to tackle this problem. From my perspective, this component should totally focus on providing easy implementation of doctrine into a project.
So the focus should be:

  • provide a way to configure doctrine as outlined in the docs
  • provide a way to fetch entity managers from ContainerInterface#get by using doctrines logic (i.e. ContainerInterface#get('doctrine.entitymanager.orm_whatever')
  • provide decorators to have doctrine caches using laminas-cache

Having more specialized features such as laminas-form, laminas-authentication, laminas-paginator, laminas-validator could be dedicated packages. Not all projects using this module even use laminas-authentication or laminas-paginator, so why force them to have these components installed? In professional scopes, devs want to decide themselves what to install - why not let them opt-in for these features by providing doctrine/laminas-form-extension, doctrine/laminas-authentication-extension, doctrine/laminas-paginator-extension, doctrine/laminas-validator-extension? (names just randomly picked)


I am not available for further discussion as I do start new job with symfony and thus do not have a need for this. Just wanted to give some ideas while my knowledge is still "fresh" (as stated, I used to use this component).
I understand that you want to prevent BC breaks but thats how it majors work and you should not be scared to introduce these if they provide benefits for future development. I also understand that this is a bunch of changes which needs to be addressed, but laminas will drop a bunch of components at some point as it makes no sense to have framework agnostic components when there are other components out there (such as dropping laminas-db in favor of doctrine, etc.). This will most probably also happen to a bunch of other components (laminas-log already security-only, laminas-mvc in favor of mezzio, ...) and thus, directly depending on stuff is always a PITA when it comes to support stuff. Thats why I think its better to have satellites for each component rather than having one big component requiring all.

Thus said, I wish u all the best. In case you need some more assistance, etc. - feel free to head out in laminas slack or on github (issues are always welcome).

@TomHAnderson
Copy link
Member Author

Validator is now stopping a clean composer.json. https://github.com/laminas/laminas-validator/milestone/5 is in active development.

@MatthiasKuehneEllerhold
Copy link
Contributor

Regarding laminas-i18n and laminas-session:

As far as I can see, neither internationalization nor session stuff is frequently used in mezzio and since laminas-mvc will get abandoned in a foreseeable future, I doubt that these components will get bumped to SM v4 at all.

Hello @boesing can you elaborate or point to a source for the abandoning of laminas-mvc?

@boesing
Copy link
Contributor

boesing commented Jul 30, 2024

Regarding laminas-i18n and laminas-session:

As far as I can see, neither internationalization nor session stuff is frequently used in mezzio and since laminas-mvc will get abandoned in a foreseeable future, I doubt that these components will get bumped to SM v4 at all.

Hello @boesing can you elaborate or point to a source for the abandoning of laminas-mvc?

Not exactly as it is not 100% decided right now. but I recommend following TSC meetings in laminas slack for upcoming decisions.
From my PoV as a former active TSC member, I do not see the point in developing two different types of application frameworks given the fact that there is absolutely 0 contributions in the laminas org regarding non-TSC members.
The focus will most probably be on mezzio and thus the only logical consequence will be abandoning mvc.

@cbichis
Copy link

cbichis commented Jul 31, 2024

Hi,

@boesing Don't get me wrong, I know the amount of work involved for laminas-mvc.

However, for many of us (I am around since ZF 0.2) it would be a huge hassle to find out suddenly not just that they cant upgrade to SM 4 (quite important upgrade) but also that laminas-mvc is gonna be abandoned.

Basically the migration out of laminas-mvc needs to be hurried a lot if this decision is made before SM4 upgrade.

Better so we know it after 4.x is released. Not upgrading will hold off a lot of other packages to older versions.

@MatthiasKuehneEllerhold
Copy link
Contributor

Thanks @boesing . That is very sad to hear, because I quite like laminas-mvc and how its working. Sadly if it gets abandoned we have to migrate away from it.

@froschdesign
Copy link

@cbichis @MatthiasKuehneEllerhold
Please note:

Not exactly as it is not 100% decided right now.

This means: Nothing has been decided yet!
And work on version 4 of laminas-mvc is in progress.

@froschdesign
Copy link

By the way, I have a few suggestions that go in a different direction:

1. Kill the dependency on module manager

The direct dependency on the module manager can be killed because the interface does not necessarily have to be used:

use Laminas\ModuleManager\Feature\ConfigProviderInterface;
/**
* Base module for integration of Doctrine projects with Laminas applications
*/
final class Module implements ConfigProviderInterface

See:
https://github.com/laminas/laminas-modulemanager/blob/8df7b237d75c04a1bc17b8f7d01eeb601cd7b7e3/src/Listener/ConfigListener.php#L97-L102

2. Kill the dependency on service manager

The direct dependency on the service manager can be killed because the interfaces for the factories do not necessarily have to be used.

3. Kill the dependency on laminas-mvc

Use laminas-cli to create the Doctrine CLI support and laminas-mvc is no longer needed as a direct dependency.


A config provider class is already available so that support for Mezzio is provided. However, the MVC components must still be installed. If these dependencies are removed, the full functionality for laminas-mvc is still retained.
Furthermore, nothing stands in the way of the development of this package.

(Please correct me if I have overlooked something.)

@TomHAnderson
Copy link
Member Author

There are strong ties to laminas form. We will require a new release: https://github.com/laminas/laminas-form/milestone/16

@froschdesign
Copy link

Before laminas-form can be update, the following components must be reworked:

See also: laminas/laminas-servicemanager#216


Help is welcome at any time! 😃 👍🏻

@tasselchof
Copy link

@TomHAnderson this milestone was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants