[#1585] Jakarta namespace and java 17 for 3x

[nsoft]

This updates main to 3.0-snapshot (so a branch for 2x should be made before merging). In this PR I tried to use JakartaEE9 where possible with the exception of spring boot which is forced to JakartaEE 10. Spring never released an EE9 compatible release because by the time they did an update to Jakarta namespaces the Tomcat version for EE9 was already EOL, and they elected to skip EE9 for their test framework. They have EE9 compatibility in the runtime, but since shiro is using their testing framework, this doesn't help. Guice does not yet appear to have EE10 support, hence the EE9 focus.

mvn install/mvn verify pass in this branch, but 13 tests are @disabled. All of these tests are Integration tests and the failure mode in all cases is that the integration infrastructure (meecrowave/openliberty/arquillian) fails to launch. I am not very familiar with these and most of them are JaxRS/Jakarta Restful Services related, also a space I don't play in very often. Hopefully some here will be more familiar and be better able to address these issues.

Following this checklist to help us incorporate your contribution quickly and easily:

• Make sure there is a GitHub issue filed
  for the change (usually before you start working on it). Trivial changes like typos do not
  require a GitHub issue. Your pull request should address just this issue, without pulling in other changes.
• Each commit in the pull request should have a meaningful subject line and body.
• Format the pull request title like [#XXX] - Fixes bug in SessionManager,
  where you replace #XXX with the appropriate GitHub issue. Best practice
  is to use the GitHub issue title in the pull request title and in the first line of the commit message.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• add fixes #XXX if merging the PR should close a related issue.
• Run mvn verify to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
• If you have a group of commits related to the same change, please squash your commits into one and force push your branch using git rebase -i.
• Committers: Make sure a milestone is set on the PR

Trivial changes like typos do not require a GitHub issue (javadoc, comments...).
In this case, just format the pull request title like [DOC] - Add javadoc in SessionManager.

If this is your first contribution, you have to read the Contribution Guidelines

If your pull request is about ~20 lines of code you don't need to sign an Individual Contributor License Agreement
if you are unsure please ask on the developers list.

To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
• In any other case, please file an Apache Individual Contributor License Agreement.

[nsoft]

CI fail is on java 11 which this patch is not meant to support.

[lprimak]

Hi,

Thank you for your contribution, Shiro team appreciates it.

How did you do this change? Did you do OpenRewrite? Some other automated tool?
Can you share your process so I can repeat it / compare with what you have for an easier review?

I will also rebase your PR onto 3.x branch as well.

Thank you!

[nsoft]

I changed the dependencies, ran the build, fixed what seemed broken, rinse and repeat. No automation. I would say that it's worth looking at my adjustments to dependencies, It's possible I've not been as narrow as is ideal, but aside from the disabled tests, it seems to build. Maven is not my preferred too. I use gradle more often since that's what lucene and solr are on, and I just like to use build tools where one slight variation doesn't require a plugin. So if anything I did maven wise looks fishy it probably is. :) Judicious find and replace was used. I tried not to change the one comment in CHANGES.txt that kept matching stuff. :)

[nsoft]

Sorry I didn't notice you had a 3.x in solr we always keep main at the next unreleased major so I didn't think to look I just assumed it must not have been contemplated yet. Every project is different :)

[lprimak]

I think I would favor #2018 because it's done using automated tools.
I can run the same recipe and compare the differences.
Instead of comparing 100s of files.

Just my opinion as of now, it's kinda difficult to proceed with two overlapping huge PRs :)

What do you think?

[nsoft]

I understand the need to choose, but I'm not understanding your criteria. You speak of "repeating" the work in the PR. What is the point of folks going to the effort of creating a PR if not to save you work?. The criteria I would suggest is which PR passes more tests, which PR's changes look more in line with the existing attempts to control dependencies. Also I don't know how explicit the focus on a given EE level is in the other PR, but if it's spring boot based approach, I worry that it's going to be EE10 oriented and that won't mesh with guice and other libraries that are not EE10 supportive yet.

If there is anything you don't understand about what I've done I'll be happy to answer, but I don't understand your desire to throw out BOTH pull requests and create a third (or maybe I'm misunderstanding your intent). I've been contributing to various Apache projects since 2002 (Apache Ant) and I'm a committer since 2019 (Lucene/Solr). I've never heard of such a thing as the starting point, except when a committer already has a set of changes ready to go. I've of course seen PR's abandoned after an attempt to make them work, but I don't understand what you mean by "repeating" them.

If there is something I've done wrong or if you need me to rebase it to your 3.x branch that I missed I can help on that if you like. I spent almost a week developing this to the point where it passes all but a dozen+ tests which AFAIK only fail because of classpath/ or swallowed exceptions during test harness startup. I decided to submit here because I knew I was swimming against the learning curve of tools I don't know well, but I'm sure with some more effort I can get the remaining tests passing. I just figured there was a chance someone who knew the tool would know how to do things like get arquillian to write out logs where I can find it since maven seems to swallow anything relating to the plugin/tool startup. Often I had to debug into things to find out what was actually failing, and that of course continually runs afoul of IDE holding onto references to jars that aren't actually used anymore (many IDE caches, and pom imports died to bring us this information ;) )

I definitely wouldn't advocate the merging of this until all tests pass, and I'm currently using the result to develop against locally and will report if it shows any problems. The vast majority of the patch is just s/javax/jakarta/ replacements so reading it should take a lot less time than the number of files would normally imply. The most mysterious thing is likely to be the deletion of spring remoting related classes and tests, and that's because spring remoting doesn't exist in newer versions. It had been deprecated and was finally dropped. Also, I dropped the guice3 and guice4 testing and focused on the guice 7 as simply "guice" (I didn't think there would be desire to maintain ancient classpaths for running guice 3, 4, 5, 6, and 7 tests).

If the other patch is a better place to start from, because it passes more tests or touches fewer poms, or you like the libraries it selected better that's fine. I won't be at all concerned if you use an better PR than mine. I just don't understand this notion that you need to repeat and compare. Such a thing would leave you with 3 way variations to sort out:

1. Tool vagaries (some of which the other PR probably dealt with),
2. Submitted PR variances (some of which are likely developer choice, not tool related),
3. and things that still need to be done.

It should be simpler to trust your own tests, trust one PR or the other (with review) and fix-up the remaining tests. It's not getting released instantly so there's time to find and fix issues that remain and/or are discovered.

[nsoft]

Another thing I did is I spent some time chasing down stray references to old versions of libs and trying to eliminate them with exclusions. This was sometimes necessary so that the debugger/IDE run tests could agree with the maven run tests.

[lprimak]

I don't understand your desire to throw out BOTH pull requests

Yes, either I misspoke or you misunderstood.

Both PRs are very large (100s of files). This is understandable. My idea was to re-create the automated refactor that was done with OpenRewrite. This would eliminate most files to be reviewed. This way, the "new" starting point would be just the differences that need to be written manually. This would be much easier to review.

To re-iterate, I do not want to "throw away" any PRs. Just have a starting point that will eliminate manual review cycle.

[lprimak]

Also, I just want you to know that the way we look at PRs, is not "who is this"? Never. We always prioritize Community PRs ahead of our "own" work and work tirelessly to give credit and merge Community PRs.

Thank you very much for your contribution and looking to move forward.

[lprimak]

Still have the dilemma, which PR to pursue first... Yours has conflicts and isn't automated.
I am still thinking the other PR first, and then what remains of your changes, we can pursue after.

[nsoft]

Sure, let me try to provide a summary of the library changes to facilitate comparison. While I do that here's the diff I'm working from: https://gist.github.com/nsoft/3901d8c27982f36bd4a1a74d5296f553

[lprimak]

Sorry I didn't notice you had a 3.x in solr we always keep main at the next unreleased major

3.x is nowhere near ready for release / CI, thus "main" remains "main" and 3.x on it's own

[nsoft]

Sorry I didn't notice you had a 3.x in solr we always keep main at the next unreleased major

3.x is nowhere near ready for release / CI, thus "main" remains "main" and 3.x on it's own

So I'm guessing you are running a main=current branching strategy? And then have to merge 3.x into main to release 3.x? And thus there are some 2.x features/fixes that don't exist in 3.x?

(not arguing anything just trying to understand your system)

This is also getting off topic, fee free to hit me up on ASF slack to discuss further.

[nsoft]

Just have a starting point that will eliminate manual review cycle.

I think part of the disconnect is that you maybe trust such tools more than I do :)

[lprimak]

Haven't made the final decision yet. For now, fixes go into main and main gets merged into 3.x

Things are incompatible with 2.x will go into 3.x branch.

My guess is main will be renamed into 2.x at some point and 3.x will be renamed to main.

[nsoft]

This gist should be a bit more useful, it filters out the self references and anything that's a transitive change:
https://gist.github.com/nsoft/cce677cf3d6cd19ae3d0a386a7eeb7fa

[nsoft]

One note from using this patch locally: It's now the case that the faces stuff in shiro-jakarta-ee requires omnifaces and omnifaces is very rude if the container finds the annotated classes and tries to do something with them...

Luckily this library was something I included long ago in my early attempts to get ee stuff working and not actually needed AFAICT, but breaking the jakarta-ee stuff into smaller bits (at leaast one to separate out the omnifaces dependency might be a good idea (but probably a follow on ticket).

For the purposes of this ticket I'm not sure if I might have introduced that dep to get this working or if there might be a more polite alternative implementation, so that should be checked on.

[lprimak]

Hi, Gus,

You may be conflating the jakarta namespace with Jakarta EE features.
Jakarta EE Integration Module shiro-jakarta-ee requires OmniFaces and a compliant Jakarta EE container to work.
Shiro is already split up into shiro-jaxrs and shiro-cdi` modules if you don't have a compliant EE container.
OmniFaces "fail-fast" which is the correct behavior, instead of having you debug issues later on.

See the following section in documentation
"Configuring for Tomcat / Jetty (or without Jakarta Faces)"
Also look for other configuration options in the same page if you need anything else configured specific to your use-case

Let me know if I am at all of base here :)

[nsoft]

Yes, I had conflated those (long before) and that's why it wasn't actually a required module for me. It was just something that had become added and never removed in attempts to get my app working previous to this. Nice that it's got a documented workaround, though I still wonder if more granularity might be valuable. Just wanted to highlight this oops so that you can check my dependency work in the related pom.xml