Java EE 7

[mdfst13]

I have a Java EE 7 version of Rewrite at my javaee7 branch. This version runs correctly on Tomcat 8.5 with Weld in my IDE. It compiles under Java SE 8, so I believe it would break current Travis configuration. It parameterizes more variables so as to be easier to update the next time (e.g. to Java EE 8). It's at this commit.

It fails three or so tests, but two have IgnoreFor notations (in integration-gwt and transform-minify). The third (from impl-servlet-tests) checks if content that had been lowercase is transformed to uppercase. It's not clear to me why that would happen or why it would be desirable.

I'm not issuing a pull request to submit it, as I don't know if it should go to master. There's an argument that it should go to a separate branch as preparation for a 3.5 or 4.0 version. That would leave room for a future 3.4.3 release and other releases on 3.4 that retain current compatibility. But of course that decision is entirely yours.

I'm finished with this part of my current project, so it is unlikely that I will have any significant time to troubleshoot the remaining issues. I don't know if they are Arquillian artifacts or significant. Nor would I have time to troubleshoot new issues that might arise. That said, you might find it easier to incorporate these changes than to start over and write them.

[chkal]

Hi! Thanks a lot for working on this.

I'm a bit unsure if I like the idea of updating all the spec versions. Especially because this will be a breaking change and users still on Java EE 6 will run into problems.

I'm curious. Why do you think updating the spec versions is necessary? I mean, actually all the specs should be backwards compatible. Did you run into any issues deploying Rewrite to Java EE 7 or 8 container?

[mdfst13]

Given how I did it, you could also release this revision with the old specifications. Part of what I did was to make it easier to change the specifications. Whatever you might set will then propagate to the other POM files from rewrite-parent/pom.xml . To make that change, I had to edit almost all the POM files. To adjust in the future, you shouldn't have to do so. You should be able to edit just the top.

Of course, if you did that, you'd probably need to restore the Xalan override.

The problem that I was seeing was that beans weren't being picked up when I added PrettyFaces. So without PrettyFaces, I could go to old_ugly.xhtml and get beans. With PrettyFaces, I could go to old_ugly.xhtml or pretty.xhtml and get the same content, but the beans weren't working. Or to be more precise, the EL expressions were not showing results from the beans and the beans were not logging accesses (my logging is pretty verbose at FINEST, so I'm confident that no methods with logging were being called including the specific ones that should have been).

That's working now, but I don't know exactly what fixed it. After it started working, I tried backing out some changes to see if they fixed things. But none of the things that I tried backing out, including the last change that I made, have made it stop working in either my main project or my MCVE project. That explicitly includes using 3.4.2.Final rather than my locally built snapshot version. It could have been an IDE or Maven problem for all that I know. Or adding PrettyFaces caught some configuration on my part that was wrong and made it stop working.

The last things that I updated were changing from weld-servlet-shaded to weld-servlet-core and adding jandex. Adding jandex makes some sense, as it changes from bean discovery using reflection to the jandex method. But my MCVE project works without jandex and with weld-servlet-shaded.

When I was debugging, one of the problems that was mentioned was using reflection across Java versions. So since PrettyFaces was the thing that I added last, is something that might reasonably be considered to change bean behavior, and I could fix the behavior by removing PrettyFaces, that's where I was debugging. I was sidetracked for a while since mvn install and mvn test require a maven profile to be set. The way that it defaults on my system, the tests fail. This may not be obvious to you in that you may already have the profile set or your setup better works with the default. Or both.

The container that I'm using is Tomcat 9, which is supposed to be Java EE 8 compatible, but I am targeting Java EE 7 (Servlet 3.1).

Like I said, I'm unlikely to do more debugging of this unless it breaks again. I really need to get back to working on my actual project rather than this one.

[chkal]

Given how I did it, you could also release this revision with the old specifications. Part of what I did was to make it easier to change the specifications.

I agree that this is a good change. Having the spec versions at a single place is definitely a good move.

The problem that I was seeing was that beans weren't being picked up when I added PrettyFaces.

That's really weird. But I don't think that anything from your PR fixed that. Maybe it was really some other issue?

The last things that I updated were changing from weld-servlet-shaded to weld-servlet-core and adding jandex.

I agree that weld-servlet-core should be preferred over the shaded artifact. However, I think the Rewrite core doesn't depend on Weld but only some of the examples, no?

When I was debugging, one of the problems that was mentioned was using reflection across Java versions.

Maybe this is related some some dependency pulled in by Rewrite?

The container that I'm using is Tomcat 9, which is supposed to be Java EE 8 compatible, but I am targeting Java EE 7 (Servlet 3.1).

So you actually didn't update the spec version in Rewrite because it fixed some specific problem but just because it is closer to the current version?

Like I said, I'm unlikely to do more debugging of this unless it breaks again. I really need to get back to working on my actual project rather than this one.

Ok, sure! Feel free to open a pull request for your branch. But I'm not sure if I want to merge it with all the changes currently contained. Maybe I find some time to have a deeper look in the next weeks.

Thanks for your help.