Tidy up X509CRLImpl

[ckelleyRH]

• Reorder modifiers to match the JLS
• Rename static constant to match JLS
• Remove commented out code
• Use pattern matching with instanceof
• Remove unnecessary else clauses
• Use ternary operator where appropriate
• Put array designator on the type not the variable
• Remove unnecessary negation in logic of parse() method

[ckelleyRH]

This exploded nicely because the build is against Java 11 and the pattern matching instanceof was introduced in Java 14.

[fmarco76]

LGTM.

BTW, CMake seems to build with java 17 sources but maven build with java 11. They should use the same version

[ckelleyRH]

BTW, CMake seems to build with java 17 sources but maven build with java 11. They should use the same version

Yeah I thought I had updated this when we moved on to OpenJDK 17 but clearly not, I must be thinking of another repo or mis-remembering.

[ckelleyRH]

Simply updating the <release> tag to 17 is insufficient, as Maven looks for the JDK at JAVA_HOME, which is not defined on my machine or in the CI, so it falls back somehow on Java 11. You can force Maven to use a specific compiler like this:

<configuration>
    <fork>true</fork>
    <compilerVersion>17</compilerVersion>
    <executable>/usr/lib/jvm/java-17-openjdk-17.0.3.0.7-1.fc36.x86_64/bin/javac</executable>
    <release>17</release>
</configuration>

This still causes a failure with the tests:

[INFO] --- maven-surefire-plugin:2.12.4:test (default-test) @ jss ---
[INFO] Surefire report directory: /home/ckelley/git/jss/target/surefire-reports

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
org.apache.maven.surefire.util.SurefireReflectionException: java.lang.reflect.InvocationTargetException; nested exception is java.lang.reflect.InvocationTargetException: null
java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
	at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
	at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)
Caused by: java.lang.UnsupportedClassVersionError: org/mozilla/jss/tests/JCASigTest has been compiled by a more recent version of the Java Runtime (class file version 61.0), this version of the Java Runtime only recognizes class file versions up to 55.0

Here it looks like a test file has already been compiled with Java 17 but the runner is still trying to use Java 11 somehow.

[ckelleyRH]

I don't expect the current revision to work, but wanted to push in case either of you have spotted that I have missed a Java 11 declaration somewhere!

[ckelleyRH]

Now it works locally but doesn't work in the CI for some reason. Will look again tomorrow.

[edewata]

I tried installing openjdk-17-jdk on a Debian container, the files got installed under /usr/lib/jvm/java-17-openjdk-amd64 so maybe try that instead.

[edewata]

Can we actually set an environment variable this way in GH workflow? Or do we need to use env:?

[fmarco76]

The SonarCloud now is working but the env with the correct jvm should go in all tests with ubuntu/debian.

[ckelleyRH]

Ah yes, I will use env, forgot about that.

[ckelleyRH]

@edewata @fmarco76 - now it fails because of #861. So I think we're "good".

Edit, no, some of the Azure tests are still failing with cmake problems.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

[ckelleyRH]

OK, now everything fails the "proper" way. Interestingly, The Debian/Ubuntu Azure pipelines were previously successful. They were not affected by #861. Perhaps it is a consequence of Java 17, the issue we are seeing?

[edewata]

It's reported on Debian too:
https://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg1855971.html
Regardless, the PR looks good. Thanks for the update!

[ckelleyRH]

Thanks @edewata @fmarco76 !