ZF-Commons / zfc-rbac

Role-based access control module to provide additional features on top of Zend\Permissions\Rbac
BSD 3-Clause "New" or "Revised" License
181 stars 111 forks source link

Laminas migration (master) #392

Closed basz closed 4 years ago

coveralls commented 4 years ago

Coverage Status

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

demiankatz commented 4 years ago

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 commented 4 years ago

...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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

demiankatz commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

demiankatz commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

basz commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@demiankatz lets wait on https://github.com/Laminas-Commons/LmcRbac/issues/11 and see what they will say

demiankatz commented 4 years ago

@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 commented 4 years ago

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

I've never been active in this organization. :smile:

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

Thanks, @visto9259 -- I was able to migrate my project (https://github.com/vufind-org/vufind/pull/1657) with no problems!

visto9259 commented 4 years ago

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

demiankatz commented 4 years ago

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

basz commented 4 years ago

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