Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recipes for methods and classes renamed or moved when switching to Java 17 #430

Merged
merged 27 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
916bec7
adding 4 recipes + tests for JavaSE 17
AnuRam123 Mar 11, 2024
d1d7271
changed license to 2024 in test and a small text update to .yml
AnuRam123 Mar 11, 2024
60548d1
Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…
ranuradh Mar 11, 2024
48b640c
Update src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test…
ranuradh Mar 11, 2024
d94c875
remove extra spaces in test
AnuRam123 Mar 11, 2024
2dd45da
Update src/main/resources/META-INF/rewrite/java-version-17.yml
ranuradh Mar 12, 2024
b113d01
Update src/main/resources/META-INF/rewrite/java-version-17.yml
ranuradh Mar 12, 2024
3d73a92
Update src/main/resources/META-INF/rewrite/java-version-17.yml
ranuradh Mar 12, 2024
da81b1a
Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…
ranuradh Mar 12, 2024
d3d0705
Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…
ranuradh Mar 12, 2024
63c632f
Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…
ranuradh Mar 12, 2024
3d4aca8
Update src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test…
ranuradh Mar 12, 2024
d4bdae2
Update src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test…
ranuradh Mar 12, 2024
cf78d9b
Update src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test…
ranuradh Mar 12, 2024
910538a
Update src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test…
ranuradh Mar 12, 2024
0e30ecf
Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…
ranuradh Mar 12, 2024
6f97692
Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…
ranuradh Mar 12, 2024
ff7629a
Merge branch 'main' into recipe_RemovedZipFinalizeMethods
timtebeek Mar 14, 2024
abf75f1
Apply suggestions from code review
timtebeek Mar 15, 2024
c5ccd9d
Apply suggestions from code review
timtebeek Mar 15, 2024
7dcc706
Apply suggestions from code review
timtebeek Mar 15, 2024
9fc5103
updated text in .yml
AnuRam123 Mar 18, 2024
34ae7c5
small fixes to java-version-17.yml
AnuRam123 Mar 18, 2024
170eff2
added small correction to text
AnuRam123 Mar 19, 2024
f03cc5e
Merge remote-tracking branch 'origin/main' into recipe_RemovedZipFina…
AnuRam123 Mar 19, 2024
43e5d42
update license in java-version-17.yml
AnuRam123 Mar 19, 2024
11ec8fa
Merge remote-tracking branch 'origin/main' into recipe_RemovedZipFina…
AnuRam123 Mar 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 73 additions & 1 deletion src/main/resources/META-INF/rewrite/java-version-17.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ recipeList:
groupId: com.sonatype.clm
artifactId: clm-maven-plugin
newVersion: 2.47.6-01

- org.openrewrite.java.migrate.RemovedZipFinalizeMethods
- org.openrewrite.java.migrate.RemovedSSLSessionGetPeerCertificateChainMethodImpl
- org.openrewrite.java.migrate.SunNetSslPackageUnavailable
- org.openrewrite.java.migrate.RemovedRMIConnectorServerCredentialTypesConstant
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.migrate.JavaVersion17
Expand Down Expand Up @@ -122,3 +125,72 @@ recipeList:
- org.openrewrite.java.ChangeMethodAccessLevel:
methodPattern: "*..* premain(java.lang.String, java.lang.instrument.Instrumentation)"
newAccessLevel: public
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.migrate.RemovedZipFinalizeMethods
displayName: Remove `finalize` method in `java.util.zip.Zipfile`, `java.util.zip.Inflater` and `java.util.zip.Deflater`
ranuradh marked this conversation as resolved.
Show resolved Hide resolved
description: >
The `finalize` methods in `java.util.zip.ZipFile` is replaced with `close` method and is replaced by `end` method in
Copy link
Contributor

Choose a reason for hiding this comment

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

methods should be method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in next commit

`java.util.zip.Inflater` and `java.util.zip.Deflater` as it is no longer available in Java SE 12 and later.
tags:
- java17
recipeList:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a test for this recipe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @cjobinabo since it wasnt possible to find a jar with the issue , we decided to not have a test for this.. Refer to our slack conversation

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a conversation you and I had, or you and Tim? If you couldn't find a jar with the dependency, I would have suggested mocking one up as we have before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was about running tests where we need an older version of Java and we cant get that to work -

so while there's no good way to unit test such recipes, you'll have to trust us that the particular case works when run on Java 8 or 11

- org.openrewrite.java.ChangeMethodName:
methodPattern: "java.util.zip.Inflater finalize()"
newMethodName: end
ignoreDefinition: true
matchOverrides: true
Copy link
Contributor

Choose a reason for hiding this comment

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

If we ignore definitions, we shouldn't need to do anything with overrides.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in next commit

- org.openrewrite.java.ChangeMethodName:
methodPattern: "java.util.zip.Deflater finalize()"
newMethodName: end
ignoreDefinition: true
- org.openrewrite.java.ChangeMethodName:
methodPattern: "java.util.ZipFile finalize()"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be java.util.zip.ZipFile

newMethodName: close
ignoreDefinition: true
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.migrate.RemovedSSLSessionGetPeerCertificateChainMethodImpl
displayName: Remove `SSLSession.getPeerCertificateChain()` method
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove to Replace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in next commit

description: >
The `javax.net.ssl.SSLSession.getPeerCertificateChain()` method implementation was removed from the SunJSSE provider and HTTP client implementation in Java SE 15.
The default implementation will now throw an `UnsupportedOperationException`.
Applications using this method should be updated to use the `javax.net.ssl.SSLSession.getPeerCertificates()` method instead.
tags:
- java17
recipeList:
- org.openrewrite.java.ChangeMethodName:
methodPattern: "*.net.ssl.SSLSession getPeerCertificateChain()"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we aren't using javax instead of *?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

During one test I had it as *, I reverted it back to javax now

newMethodName: getPeerCertificates
ignoreDefinition: true
matchOverrides: true
Copy link
Contributor

Choose a reason for hiding this comment

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

If we ignore definitions, we shouldn't need to do anything with overrides.

---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.migrate.SunNetSslPackageUnavailable
displayName: Remove `com.sun.net.ssl` package
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove to Replace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in next commit

description: >
The internal API `com.sun.net.ssl` is removed. The package was intended for internal use only and replacement APIs can be found in the `javax.net.ssl` package.
tags:
- java17
recipeList:
- org.openrewrite.java.ChangePackage:
oldPackageName: com.sun.net.ssl
newPackageName: javax.net.ssl
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.migrate.RemovedRMIConnectorServerCredentialTypesConstant
displayName: Remove `RMIConnectorServer.CREDENTIAL_TYPES` constant
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove to Replace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in next commit

description: >
This recipe replaces `RMIConnectorServer.CREDENTIAL_TYPES` constant with the `RMIConnectorServer.CREDENTIALS_FILTER_PATTERN` constant and a filter pattern
ranuradh marked this conversation as resolved.
Show resolved Hide resolved
tags:
- java17
recipeList:
- org.openrewrite.java.ReplaceConstantWithAnotherConstant:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a test for this recipe.

Copy link
Contributor Author

@ranuradh ranuradh Mar 18, 2024

Choose a reason for hiding this comment

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

Hi @cjobinabo since it wasnt possible to find a jar/jdk upgrade we decided to not have a test for this.. Refer to our slack conversation

Copy link
Contributor

Choose a reason for hiding this comment

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

The only recipes we discussed on Slack before I left were about the removed finalize method and the removed ConstantBootstraps constructor. Perhaps this is a conversation you had with someone else. If you can't find a jar with the dependencies, I would suggest mocking one up like we have done in the past. I would rarely suggest omitting a test.

existingFullyQualifiedConstantName: javax.management.remote.rmi.RMIConnectorServer.CREDENTIAL_TYPES
Copy link
Contributor

Choose a reason for hiding this comment

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

CREDENTIAL_TYPES should be CREDENTIAL_TYPE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this rule I see the this rule flags references to the RMIConnectorServer.CREDENTIAL_TYPES constant and jmx.remote.rmi.server.credential.types string literal. The RMIConnectorServer.CREDENTIAL_TYPES constant was removed in Java SE 15. The constant can be replaced with the RMIConnectorServer.CREDENTIALS_FILTER_PATTERN constant and a filter pattern.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right. It seems like there may have been a typo in the link I looked at initially.

fullyQualifiedConstantName: javax.management.remote.rmi.RMIConnectorServer.CREDENTIALS_FILTER_PATTERN






ranuradh marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Copyright 2024 the original author or authors.
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.java.migrate;

import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.InMemoryExecutionContext;
import org.openrewrite.config.Environment;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.migrate.jakarta.RemoveBeanIsNullable;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

import static org.openrewrite.java.Assertions.*;


class SunNetSslPackageUnavailableTest implements RewriteTest {
timtebeek marked this conversation as resolved.
Show resolved Hide resolved

@Override
public void defaults(RecipeSpec spec) {
spec.parser(JavaParser.fromJavaVersion()
.classpathFromResources(new InMemoryExecutionContext(),
"com.sun.net.ssl"))
.recipeFromResource("/META-INF/rewrite/java-version-17.yml", "org.openrewrite.java.migrate.SunNetSslPackageUnavailable");
}
@Test
ranuradh marked this conversation as resolved.
Show resolved Hide resolved
void sunNetSslPackageUnavailableTest(){
rewriteRun(
//language=java
java(
"""
package testing.stuff;

ranuradh marked this conversation as resolved.
Show resolved Hide resolved
import com.sun.net.ssl.HttpsURLConnection;

public class TestSunNetSsl {
public void useThePackages() {
ranuradh marked this conversation as resolved.
Show resolved Hide resolved
HttpsURLConnection con;
}
}
""",
"""
package testing.stuff;

ranuradh marked this conversation as resolved.
Show resolved Hide resolved
import javax.net.ssl.HttpsURLConnection;

public class TestSunNetSsl {
public void useThePackages() {
ranuradh marked this conversation as resolved.
Show resolved Hide resolved
HttpsURLConnection con;
}
}
"""
)
);
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 the original author or authors.
* Copyright 2024 the original author or authors.
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,6 +17,7 @@

import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.InMemoryExecutionContext;
import org.openrewrite.config.Environment;
import org.openrewrite.java.marker.JavaVersion;
import org.openrewrite.test.RecipeSpec;
Expand Down Expand Up @@ -291,7 +292,6 @@ void bar(LogRecord record) {
), 17)
);
}

@Test
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
void needToUpgradeMavenCompilerPluginToSupportReleaseTag() {
rewriteRun(
Expand Down Expand Up @@ -527,4 +527,46 @@ void lombokBumpedGoingTo17() {
)
);
}



@Test
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
void removedSSLSessionGetPeerCertificateChainMethodImplTest(){
Copy link
Contributor

@cjobinabo cjobinabo Mar 18, 2024

Choose a reason for hiding this comment

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

If possible, please split each recipe into its own test method.

Copy link
Contributor Author

@ranuradh ranuradh Mar 18, 2024

Choose a reason for hiding this comment

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

Actually I thought unless we had multiple cases it was better to put them in the common Test.java, I could add it to another text if u feel strongly but I went with what I saw in the file.

Copy link
Contributor

Choose a reason for hiding this comment

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

I left the earlier comment here because I thought this test method was attempting to test more than one recipe. But that actually does not appear to be the case, so nothing needs to be changed here.

rewriteRun(
//language=java
java(
"""
package com.test;
ranuradh marked this conversation as resolved.
Show resolved Hide resolved
import java.security.cert.Certificate;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLSession;
public class RemovedSSLSessionGetPeerCertificateChainMethodImplApp {
public void test() throws Exception {
ranuradh marked this conversation as resolved.
Show resolved Hide resolved
SSLEngine sslEngine = SSLContext.getDefault().createSSLEngine();
SSLSession session = sslEngine.getHandshakeSession();
session.getPeerCertificateChain(); //This should trigger
Certificate[] certs = session.getPeerCertificates(); //This should not trigger
}
}
""",
"""
package com.test;
ranuradh marked this conversation as resolved.
Show resolved Hide resolved
import java.security.cert.Certificate;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLSession;
public class RemovedSSLSessionGetPeerCertificateChainMethodImplApp {
public void test() throws Exception {
ranuradh marked this conversation as resolved.
Show resolved Hide resolved
SSLEngine sslEngine = SSLContext.getDefault().createSSLEngine();
SSLSession session = sslEngine.getHandshakeSession();
session.getPeerCertificates(); //This should trigger
Certificate[] certs = session.getPeerCertificates(); //This should not trigger
}
}
"""
)
);
}

}
Binary file not shown.