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

Laminas migration (master) #392

Closed
wants to merge 3 commits into from

Conversation

basz
Copy link
Collaborator

@basz basz commented Jan 15, 2020

No description provided.

@coveralls
Copy link

coveralls commented Jan 15, 2020

Coverage Status

Coverage increased (+0.4%) to 93.344% when pulling 9617304 on basz:laminas-migration-master into 8a28d65 on ZF-Commons:master.

@basz basz changed the title laminas-migration migrate Laminas migration (master) Jan 15, 2020
@basz basz mentioned this pull request Jan 16, 2020
Copy link

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

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

Thanks again for working on this -- just a few nitpicky comments about comments that you can act on or ignore as you see fit... just thought I'd point these things out in case you're feeling perfectionist. :-)

@@ -8,22 +8,21 @@
[![Scrutinizer Quality Score](https://scrutinizer-ci.com/g/ZF-Commons/zfc-rbac/badges/quality-score.png?s=685a2b34dc626a0af9934f9c8d246b68a8cac884)](https://scrutinizer-ci.com/g/ZF-Commons/zfc-rbac/)
[![Total Downloads](https://poser.pugx.org/zf-commons/zfc-rbac/downloads.png)](https://packagist.org/packages/zf-commons/zfc-rbac)

ZfcRbac is an access control module for Zend Framework 2, based on the RBAC permission model.
ZfcRbac is an access control module for Laminas (Zend Framework 2), based on the RBAC permission model.

Choose a reason for hiding this comment

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

Perhaps change to Laminas (formerly Zend Framework)...

// Create your authentication service!
}
]
]
];
```
The identity given by `Zend\Authentication\AuthenticationService` must implement `ZfcRbac\Identity\IdentityInterface`. Note that the default identity provided with ZF2 does not implement this interface, neither does the ZfcUser suite.
The identity given by `Laminas\Authentication\AuthenticationService` must implement `ZfcRbac\Identity\IdentityInterface`. Note that the default identity provided with ZF2 does not implement this interface, neither does the ZfcUser suite.

Choose a reason for hiding this comment

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

Might want to do a search for the abbreviation ZF2 and replace that with something else -- I see it here, and I imagine it is found elsewhere too.

@@ -80,7 +80,7 @@ return [
];
```

Because of the way Zend Framework 2 handles config, you can without problem define some rules in one module, and
Because of the way Laminas Framework 2 handles config, you can without problem define some rules in one module, and

Choose a reason for hiding this comment

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

"Laminas Framework 2" is another phrase that might be worth searching and replacing -- perhaps with simply "Laminas."

@@ -466,7 +466,7 @@ class IpGuardFactory implements FactoryInterface, MutableCreationOptionsInterfac
}
```

The `MutableCreationOptionsInterface` was introduced in Zend Framework 2.2, and allows your factories to accept
The `MutableCreationOptionsInterface` was introduced in Laminas Framework 2.2, and allows your factories to accept

Choose a reason for hiding this comment

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

Maybe "Laminas version 2.2" makes more sense than "Laminas Framework 2.2" -- not sure how to deal with the name change retroactively, but as it is, it definitely doesn't feel quite right.

@@ -108,15 +108,15 @@ return [
## Creating custom strategies

Creating a custom strategy is rather easy. Let's say we want to create a strategy that integrates with
the [ApiProblem](https://github.com/zfcampus/zf-api-problem) Zend Framework 2 module:
the [ApiProblem](https://github.com/laminas-api-tools/api-tools-api-problem) Laminas Framework 2 module:

Choose a reason for hiding this comment

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

Another instance of "Laminas Framework 2" that could probably just be abbreviated to Laminas.

@@ -75,7 +75,7 @@ public function testAuthorizationServiceIsInjectedWithDelegator()
$serviceManager = ServiceManagerFactory::getServiceManager();

if (method_exists($serviceManager, 'build')) {
$this->markTestSkipped('this test is only vor zend-servicemanager v2');

Choose a reason for hiding this comment

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

Might be possible to remove tests/support for servicemanager v2 -- I can't imagine people are going to want to move to Laminas without upgrading first. Of course, maybe I'm wrong! Probably not something you want to do in this PR, of course, but an idea for the future.

@demiankatz
Copy link

Two other general notes:

1.) I checked out this branch and dropped it into the vendor directory of my own project, which I have migrated to Laminas, and it seems to work just fine.

2.) I imagine there may also be a need to rename this whole project and organization... but again, that's a separate issue from what's going on in this PR.

Thanks again -- this is a great help!

@demiankatz
Copy link

...and I guess there's one final question, which is: after this gets merged, do we immediately mint a new release, or do we wait until naming discussions are sorted out. I'm fine with either approach -- I don't need to finish my own Laminas migration for a few months, so I can afford to be patient. I'm just interested to know which approach will be taken so I can plan accordingly.

@svycka
Copy link
Contributor

svycka commented Jan 16, 2020

I am not sure how you will release this. this is considered a BC break and should be released as major version but we already have one so not sure what to do :D

@demiankatz
Copy link

@svycka, would it make sense to fork this into two different projects -- LaminasRbacModule for 2.x-based logic and LaminasRbac for 3.x-based logic? My understanding (and please correct me if I'm wrong) is that 3.x is a pretty significant rewrite that changes the scope of the project. For those of us who still want to use this is a drop-in module for laminas-mvc, perhaps it makes more sense to separate the two things while everything is getting reorganized anyway.

And, of course, if that's too much trouble, I'm also open to the answer of "this is deprecated, so please stop using it." But if the 2.x line still has some life left in it, we should figure out a way to keep it available.

@basz
Copy link
Collaborator Author

basz commented Jan 16, 2020

Hmm, don't think any of the laminas component have been as a new version? It should be just a namespace change. Shouldn't it just work? Not sure...

@demiankatz
Copy link

@basz, Laminas components have kept the same version numbers but changed their names. If we keep the same project name, this is a backward-breaking change, because users who are still on Zend will run composer update and suddenly run into a bunch of Laminas namespaces they aren't expecting.

@svycka
Copy link
Contributor

svycka commented Jan 16, 2020

I think better just do not migrate it will work in both cases. And will work until laminas will release major versions that would mean that we will have to update something anyway and will lead to new versions.
And about moving 3.0 to separate repository maybe, not sure about this because most contributors already use 3.0 version(I guess. At least me and @basz)

@demiankatz
Copy link

@svycka / @basz, I think the first thing to figure out is the naming issue. If we can follow the Zend pattern of creating a new repo with a new name and then marking this one as obsolete, we could retain current versioning, merge both current pull requests, and then I think everyone gets what they want. I think that's probably worth doing since the name has lost a lot of meaning with the transition to Laminas. In the meantime, as @svycka points out, it works as-is, so it's not an urgent crisis. I think if we can achieve the rename, though, there are some benefits to updating the namespace even in the legacy version, since I have to imagine that there's at least a tiny bit of overhead in the autoloading name remapping.

@basz
Copy link
Collaborator Author

basz commented Jan 16, 2020

Aha... I think I understand now. So to solve the naming issue a new organization would need to be created. Laminas-Commons?

The only thing is that this commons org does not contain many active projects anymore... is it worth the effort? are there any people of the organization around as we would need to archive projects... maybe we could reuse some of the tooling to do such a migration...

@demiankatz
Copy link

@basz, I'm not sure what the history of ZF-Commons is, but yes, I think we would either need a new Laminas-Commons organization, or else a conversation about what it would take to move this into the official Laminas organization (though I assume this stayed outside of official Zend organization in the first place for a reason).

If this is the only project in ZF-Commons that people wish to maintain, and it doesn't make sense to move it into the official Laminas territory or create a new organization, I'd also be happy to offer up space in my vufind-org organization, which has some other Zend/Laminas-adjacent libraries in it. I don't claim that this is a great option, or the most appropriate, but if you just want a place to park a repo, I certainly wouldn't mind having this living near my project that depends on it.

@FabianKoestring
Copy link

Any news on laminas support? We can´t update to laminas because of this module. 😨

@demiankatz
Copy link

@FabianKoestring, I have successfully upgraded to Laminas while still using the existing version of this module; the Zend-Laminas bridge takes care of making things work. I'd still much rather see a Laminas version of the code, so I would support making forward progress on this pull request... but in the meantime, this doesn't have to be a blocker for you.

@basz
Copy link
Collaborator Author

basz commented Mar 6, 2020

I don’t know how to increment the major versions. Especially for the 2.0 version... that why this is blocking.

In the mean time I have read that because there are no api changes and only dependecy changes a jump in major isn’t required. Perhaps @Ocramius can advise us here?

@demiankatz
Copy link

@basz, as discussed above, one possible solution would be to fork the repository and treat this as a new project. This is the model that core Laminas components have followed. That way, we could keep existing 2.x and 3.x versions, and people could migrate by changing the package name in their composer.json. I think the only problem with that approach is deciding where the repository should live. I've volunteered space in the vufind-org repository if that helps, but it might make more sense to establish a new LaminasCommons organization.

@basz
Copy link
Collaborator Author

basz commented Mar 6, 2020

@demiankatz that would be fine by me. However no one from the zfcommon organisation has responded to previous call... I think this org is somewhat abandoned...
Perhaps we should just do the fork anywhere...

@demiankatz
Copy link

I'm happy to fork into vufind-org if that helps. Do you have a list of people who will need access to the repository beyond yourself? If you can share usernames, I can go ahead with the fork and then give everyone appropriate ownership.

@demiankatz
Copy link

I guess the other important question is whether to continue calling this zfc-rbac, or if it's better to call it laminas-commons-rbac, or something else.

@FrankHouweling
Copy link

Hi @basz ! We are using zfc-rbac and would like to upgrade to Laminas. What is the status of the migration of this library? Is there anything I can do?

@demiankatz
Copy link

On my end, things are still working just fine with the Zend Framework to Laminas Bridge, but this is now the last outstanding piece of my application that contains Zend references, so I share @FrankHouweling's desire to get a Laminas migration completed. My offer still stands to fork this into the vufind-org organization if that's the easiest way to produce some forward progress, but if there's a more logical path forward, I'm open to anything!

@basz
Copy link
Collaborator Author

basz commented Jun 25, 2020

A new org has been created it seems https://github.com/Laminas-Commons/LmcRbac

@demiankatz
Copy link

That's great news; thanks, @basz. I'll try that out soon. It looks like that particular repo is based on zfc-rbac 2.x, which is exactly what I need for my purposes. It's not clear to me what the future is for the 3.x code, though.

@svycka
Copy link
Contributor

svycka commented Jun 26, 2020

@basz but seems different people, can they maintain that many projects? also projects does not officially announce that they moved I think its to soon to move there

@demiankatz
Copy link

@svycka, is it worth reaching out to the developers maintaining the Laminas-Commons organization and see if they're willing to weigh in here with their plans? I'm happy to attempt to make contact if there's not already somebody from that group who is monitoring this thread.

@FrankHouweling
Copy link

FrankHouweling commented Jun 26, 2020

@basz @svycka @demiankatz I would also prefer to start the discussion about merging the two projects, and put our joint efforts in maintaining this project and keeping it stable. As I mentioned earlier, I am very happy to help in anyway possible, so if there is any lack of resources/anything that needs to be done development-wise please let me know.
We use this library intensively in our products at Senet, and it's keeping us from removing the Zend Framework to Laminas (namespace) bridge for now.

@demiankatz
Copy link

@FrankHouweling, which version are you using, 2.x or 3.x? As discussed above, I think there may be two different paths forward, depending on whether users rely on the MVC-oriented (2.x) way of doing things, or the new (3.x) lighter-weight approach. I think there are a few questions to answer:

1.) Is the (2.x-oriented) work at https://github.com/Laminas-Commons/LmcRbac intended to be an official continuation of this work? Do the developers there need more help? Is there a desire to make that the official successor of the 2.x line of code here and document it in some way in this repo?

2.) What is the future of the 3.x work? Does that continue here (as 4.x, to reflect the backward-incompatible name change), become part of https://github.com/Laminas-Commons/LmcRbac somehow, or move elsewhere?

I still need the 2.x code for the moment, as I don't currently have the bandwidth to rethink my application for the lighter 3.x approach. If others have an interest in keeping that alive, I'm happy to support those efforts as best I can, wherever they take place. But I agree with @FrankHouweling that we should try to get on the same page to be sure we're all working together on the same project, and that it's clear to others where the latest and greatest code lives. Unnecessary fragmentation will only slow us all down.

Let me know if there's anything else I can do to help!

@FrankHouweling
Copy link

FrankHouweling commented Jun 26, 2020

@demiankatz Thanks for the elaborate comment. We are currently working on the 2.x branch.

As the 3.x branch is still unstable, I think there are two paths going forward, both of them should start with merging our efforts with Laminas-Commons.

Option A:
We make sure to release a new 4.x version that is simply the 2.x version with support for Laminas namespacing (but because of that backwards incompatible). This library goes into maintanance-only mode, and we communicate it as such in the README. The 3.x branch has never gone beyond alpha, and is therefor deprecated.

Option B:
This is simply a continuation of Option A, but if there is still an interest in the version 3 that is currently in alpha, it is further developed. To keep versioning clear, I would suggest to make a 4.x version that is simply 2.x with a backward compatibility break, and a 5.x version which contains all the new features of the current 3.x version. In this way, we make it clear that the 5.x version is 'newer' than the 4.x version, while still maintaining semantic versioning.

As I have not personal interest in the 3.x version, if it comes to my effort I would suggest Option A. Of course we could also always go for Option A now, and create the 5.x version from option B later.

@svycka
Copy link
Contributor

svycka commented Jun 26, 2020

I am using both 2.x and 3.x in production I would say 3.x is stable just not sure how many uses it

@demiankatz
Copy link

@FrankHouweling, this sounds reasonable to me, though I question whether there is a need to create a 4.x branch here at all. If we follow the precedent set by laminas itself, and if we can all agree to make Laminas-Commons the official successor of ZF-Commons, we can simply mark this repo as maintenance-only as it is, without doing any Laminas porting here, and we can move all Laminas-related work over to the new repository.

Since the new repository has a different name from this repository, we can keep the existing 2.x and 3.x version numbers, because those numbers are applied to a different package name, so there's no meaningful conflict. If we keep the main branch as the latest ported 2.x code, and we create a 3.x branch with a Laminas port of the current alpha code (if demand justifies), then we've successfully ported things without changing the overall state of the library, and everyone should have what they need. There are probably still conversations to be had about the future of the two threads of the code, but that's a separate conversation from the move to Laminas, and we can have that in another place.

If everyone prefers the 4.x/5.x approach, I offer no objections to that -- I'm just suggesting that there may be a simpler approach that might require slightly less effort.

@FrankHouweling
Copy link

FrankHouweling commented Jun 26, 2020

@demiankatz I agree, no need to go to new version numbers if we keep the new name and organization (Laminas instead of ZF). Didn't think about that option!

@svycka I have no experience or opinion about the stability of the 3.x branch, but as it is an alpha release I assumed work needs to be done before we can promote it to be a 'stable' release version-wise. Anyhow, I would still like to have a version of the 2.x branch that is Laminas compatible.

@basz
Copy link
Collaborator Author

basz commented Jun 26, 2020

using v3 in production for over a year. Just need to be released as 1.0.0

That 2.x->4.x & 3.x -> 5.x solution could work, but two repositories would be a lot clearer.

If LaminasCommon would be open to accommodate that I think the following would be simplest.

MVC version at Laminas-Commons/LmcRbac, which is copy and continuation of this repo

And new repo named Laminas-Commons/LaminasRbac or Laminas-Commons/LmsRbac or Laminas-Commons/LRbac with the v3 branch from this repo, to be released as 1.0.0 when ready.

@demiankatz
Copy link

Thanks, @basz, I think that's definitely the clearest way forward... and then we can always choose to deprecate the LmsRbac repo in the future if there's a widespread move to the newer code, but we don't have to as long as people are still happy with it.

So who are the key stakeholders here? Are there others from Laminas-Commons who need to weigh in on this conversation? Are there people from ZF-Commons who would like shared ownership to Laminas-Commons but don't already have it? (I'm not in either category -- just trying to help facilitate the transition). :-)

@FrankHouweling
Copy link

@demiankatz @basz I think that from the ZF-Commons organization, only @weierophinney is still relatively active. Am I correct?

@basz
Copy link
Collaborator Author

basz commented Jun 26, 2020

No one from the org is still active in the org, so I think we should simply go ahead with whatever we need. We've given them multiple opportunities to weigh in...

(edit) not meant to be snarky. I think the original commoners have simply outgrown and are expecting us to take point by now)

@demiankatz
Copy link

Yes, we've now done so in a very public place, so people can always catch up later if they suddenly realize this has happened and want to make further suggestions.

@basz, are you in a position to begin implementing your proposed plan? Can others do anything more to help?

@visto9259
Copy link

Hi,
@FrankHouweling reached out to me.
@matwright and I started Laminas-Commons as an organization to house ZF-Commons components that are migrated to Laminas. So far, @matwright has migrated zfc-user and I have migrated zf-rbac. We both needed updated zfc packages for our applications.

LmcRbac:dev-master is the same as zfc-rbac v2.6.3 with the breaking change that the zfc-rbac config key has been changed to lmc-rbac to keep consistency in naming.

I looked at the different branches and releases and saw the work on 3.x which seems a departure from an MVC-oriented module towards a more middleware implementation. Which brings me to now to question myself as to what direction to take.

The intent is to keep alive the work of ZF-Commons into Laminas-Commons.

@basz
Copy link
Collaborator Author

basz commented Jun 26, 2020

@demiankatz lets wait on Laminas-Commons/LmcRbac#11 and see what they will say

@demiankatz
Copy link

demiankatz commented Jun 26, 2020

@visto9259, as discussed above, I think @basz's suggestion of having separate repositories for the MVC-oriented 2.x code and the new direction 3.x code makes the most sense, since the project seems to have diverged to potentially serving two significantly different use cases, both of which remain valid (at least for the moment).

@weierophinney
Copy link
Member

I think that from the ZF-Commons organization, only @weierophinney is still relatively active.

I've never been active in this organization. 😄

I was added as an "honorary member" shortly after it launched, but I've only ever contributed a few patches and issues; I've had plenty of other OSS work to do to be able to commit any time here.

My suggestion is to create components that will work in either paradigm, and then, if needed, provide bridge packages for the specific contexts (MVC, middleware). This approach helps you focus on the specific problems (e.g., user verification, RBAC, etc.) while providing mechanisms in parallel for how they are consumed.

@visto9259
Copy link

Provided an answer in Laminas-Commons/LmcRbac#11:

When I looked at the different branches, I also though that maybe it would be good idea to separate the Rbac component from the MVC components (guards, strategies, etc.).

I am touching base with @matwright on getting more people on board. I do not think this is an issue as we did not intend to "own" Laminas-Commons.

What would be the suggested repos? I think Laminas-Commons/LmcRbac should continue to be the 2.x flavor and another repo should have the 3.x version. In that case, what should the 3.x version repo be called? Could be the other way around too. Do you have some stats as to how many people are using 2.x vs 3.x?

Since 3.x is still in alpha release, it suggests that whoever is using it, is making an extra effort to select this release in their composer.json.

@visto9259
Copy link

In trying to understand what was done for 3.x, I think that ideally, LmcRbac should only deal with roles, permissions, etc. and there should be a LmcRbacMvc that adds the MVC integration (guard, plugin, developer tools, etc.) on top of LmcRbac.

I personally prefer the ability to add simpler packages that build on top of each other rather than a large package that has many features that I don't use.

I have not looked at the all the differences between 2.x and 3.x so I am not sure what it entails to do what I am suggesting. I have not looked at migrating 3.x to Laminas at all.

@demiankatz
Copy link

@visto9259, I also have not looked too closely at the differences, except to see that it would be significant work for me to upgrade. I like your idea of complementary packages, but in the interest of moving things forward, what if we rename the current LmcRbac repo to LmcRbacMvc, since the existing implementation covers the whole stack? Then we can re-establish LmcRbac with the 3.x code, and if there's a desire to refactor LmcRbacMvc to use LmcRbac as a dependency, we could release that as version 2.0 of the package. That gives us an ability to migrate now with minimal work/disruption, but also a roadmap forward that might lead to a cleaner design in the future.

@visto9259
Copy link

visto9259 commented Jun 26, 2020

@demiankatz, that's my thinking too.

Now's the time as Laminas-Commons has not attracted too much attention yet (only a few installs of LmcRbac so far, members of this thread only?).

The code base of LmcRbac is an import of ZfcRbac as of last week. I think that nothing has been committed since then and I think it has all the 3.x commits.

If everyone agrees, here's the proposed plan:

  • Create a LmcRbacMvc repo that holds the 2.x version (essentially an import of LmcRbac) and release v3.0.0 which is Laminas based as this migration is a serious breaking change from ZfcRbac v2 (namespace has changed, but no added functionality). I started working on a few things like moving to PSR-4 autoload which can then be 3.1.
  • Migrate the 3.x branch of ZfcRbac to Laminas, i.e. work on the 3.x branch code base of the LmcRbac repo.
  • Release the migrated 3.x branch of ZfcRbac as LmcRbac v3.1 (if not namepace change) or v4 if the namespace is changed.
  • In the future, have a LmcRbacMvc v4 that depends on LmcRbac.

I can add people as required to maintain these packages as I, like everybody else, do not have the bandwidth to do all this.

@demiankatz
Copy link

@visto9259, this sounds like a good plan.

Note that the migration of the 3.x branch of ZfcRbac to Laminas is already underway in #393, so that may be a solid starting point for the "new" LmcRbac repo (in other words, I think your second bullet point is already finished, or close to it).

Please feel free to add me as a maintainer if you need some backup. My time is rather limited at the moment, but I plan to keep an eye on this in the long term and will step in to help as needed.

Also, assuming there are no objections in the meantime, please announce the LmcRbacMvc release here when it is ready, and then I can test it with my application and help troubleshoot problems if any arise.

@visto9259
Copy link

Laminas-Commons/LmcRbacMvc 3.0.0 has been released and is available on Packagist.

Beware of the breaking changes which are listed in the README.

I ran the tests scripts that came with the original ZfcRbac repo.

I also did some limited testing using a Laminas MVC Skeleton to check guards, redirect and unauthorized strategies, and to test that the laminas-developer-tools can collect LmcRbacMvc data.

Try it out and log issues in the LmcRbacMvc repo here.

@demiankatz
Copy link

Thanks, @visto9259 -- I was able to migrate my project (vufind-org/vufind#1657) with no problems!

@visto9259
Copy link

@demiankatz, Great! Still needs work to get rid of old stuff.

@demiankatz
Copy link

@visto9259, happy to help test future releases as things develop!

@basz
Copy link
Collaborator Author

basz commented Jul 2, 2020

hi! v3.0 has just been released, effectively deprecating this repository.

@basz basz closed this Jul 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants