Skip to content

AddTimeUnitArgument recipe #403

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

Merged
merged 11 commits into from
Aug 2, 2023
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* Copyright 2023 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.apache.httpclient5;

import lombok.EqualsAndHashCode;
import lombok.Value;
import org.openrewrite.ExecutionContext;
import org.openrewrite.Option;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.StringUtils;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.tree.J;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;


@Value
@EqualsAndHashCode(callSuper = true)
public class AddTimeUnitArgument extends Recipe {

@Option(displayName = "Method pattern",
description = "A method pattern that is used to find matching method invocations.",
example = "org.apache.http.client.config.RequestConfig.Builder setConnectionRequestTimeout(int)")
String methodPattern;

@Option(displayName = "Time Unit",
description = "The TimeUnit enum value we want to add to the method invocation. Defaults to `MILLISECONDS`.",
example = "MILLISECONDS",
required = false)
@Nullable
TimeUnit timeUnit;

@Override
public String getDisplayName() {
return "Adds a TimeUnit argument to the matched method invocations";
}

@Override
public String getDescription() {
return "In Apache Http Client 5.x migration, an extra TimeUnit argument is required in the timeout and duration methods. " +
"Previously in 4.x, all these methods were implicitly having the timeout or duration expressed in milliseconds, " +
"but in 5.x the unit of the timeout or duration is required. So, by default this recipe adds " +
"`TimeUnit.MILLISECONDS`, it is possible to specify this as a parameter. Since all affected methods of " +
"the Apache Http Client 5.x migration only have one integer/long argument, the recipe applies with matched method " +
"invocations of exactly one parameter.";
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new JavaIsoVisitor<ExecutionContext>() {
final MethodMatcher matcher = new MethodMatcher(methodPattern);

@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext executionContext) {
J.MethodInvocation m = super.visitMethodInvocation(method, executionContext);
if (matcher.matches(m)) {
JavaTemplate template = JavaTemplate
.builder(StringUtils.repeat("#{any()}, ", m.getArguments().size()) + "TimeUnit.#{}")
.contextSensitive()
.javaParser(JavaParser.fromJavaVersion().classpath("httpclient5", "httpcore5"))
.imports("java.util.concurrent.TimeUnit")
.build();

List<Object> arguments = new ArrayList<>(m.getArguments());
arguments.add(timeUnit != null ? timeUnit : TimeUnit.MILLISECONDS);

m = template.apply(
updateCursor(m),
m.getCoordinates().replaceArguments(),
Comment on lines +88 to +89
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering what happens to the method type when we don't replace the method, but only add an argument; wouldn't the method type post-recipe still refer to the original (usually single argument) method instead of the new multiple argument method? I'd always thought that replaceArguments was only when you replace with the same number and type or arguments; not when you switch to a different method.

If the method type is updated correctly there's no need for a change, otherwise it might be best to replace the whole method rather than just the arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, the method type is updated correctly

arguments.toArray(new Object[0])
);
maybeAddImport("java.util.concurrent.TimeUnit");
}
return m;
}
};
}
}
Original file line number Diff line number Diff line change
@@ -38,7 +38,6 @@ public String getDescription() {
@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new JavaVisitor<ExecutionContext>() {

final MethodMatcher matcher = new MethodMatcher("org.apache.hc.core5.http.HttpResponse getStatusLine()");
final JavaTemplate template = JavaTemplate.builder("new StatusLine(#{any(org.apache.hc.core5.http.HttpResponse)})")
.javaParser(JavaParser.fromJavaVersion().classpath("httpcore5"))

This file was deleted.

This file was deleted.

20 changes: 17 additions & 3 deletions src/main/resources/META-INF/rewrite/apache-httpclient-5.yml
Original file line number Diff line number Diff line change
@@ -37,10 +37,8 @@ recipeList:
overrideManagedVersion: true
- org.openrewrite.java.apache.httpclient5.UpgradeApacheHttpClient_5_ClassMapping
- org.openrewrite.java.apache.httpclient5.UpgradeApacheHttpClient_5_DeprecatedMethods
- org.openrewrite.java.apache.httpclient5.UseTimeout
- org.openrewrite.java.apache.httpclient5.UseTimeValue
- org.openrewrite.java.apache.httpclient5.UpgradeApacheHttpClient_5_TimeUnit
- org.openrewrite.java.apache.httpclient5.StatusLine

---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.apache.httpclient5.UpgradeApacheHttpClient_5_ClassMapping
@@ -415,6 +413,22 @@ recipeList:
newMethodName: setResponseTimeout
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.apache.httpclient5.UpgradeApacheHttpClient_5_TimeUnit
displayName: Adds `TimeUnit` to timeouts and duration methods
description: Apache HttpClient 5.x Timeout and duration methods need an extra the TimeUnit argument. This recipe uses milliseconds as a default unit.
recipeList:
- org.openrewrite.java.apache.httpclient5.AddTimeUnitArgument:
methodPattern: org.apache.hc.client5.http.config.RequestConfig.Builder setConnectionRequestTimeout(int)
- org.openrewrite.java.apache.httpclient5.AddTimeUnitArgument:
methodPattern: org.apache.hc.client5.http.config.RequestConfig.Builder setConnectTimeout(int)
- org.openrewrite.java.apache.httpclient5.AddTimeUnitArgument:
methodPattern: org.apache.hc.client5.http.config.RequestConfig.Builder setResponseTimeout(int)
- org.openrewrite.java.apache.httpclient5.AddTimeUnitArgument:
methodPattern: org.apache.hc.core5.http.io.SocketConfig.Builder setSoLinger(int)
- org.openrewrite.java.apache.httpclient5.AddTimeUnitArgument:
methodPattern: org.apache.hc.core5.http.io.SocketConfig.Builder setSoTimeout(int)
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.apache.httpclient5.StatusLine
displayName: Migrate to ApacheHttpClient 5.x deprecated methods from 4.x
description: Migrates deprecated methods to their equivalent ones in 5.x
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/*
* Copyright 2023 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.apache.httpclient5;

import org.junit.jupiter.api.Test;
import org.openrewrite.test.RewriteTest;
import org.openrewrite.test.SourceSpecs;

import java.util.concurrent.TimeUnit;

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

public class AddTimeUnitArgumentTest implements RewriteTest {
//language=java
private static final SourceSpecs stubCode = java("""
import java.util.concurrent.TimeUnit;
class A {
private long value;
private float foo;
private TimeUnit timeunit;
A method(int value) {
this.value = value;
this.timeunit = TimeUnit.MILLISECONDS;
return this;
}
A method(long value, TimeUnit timeunit) {
this.value = value;
this.timeunit = timeunit;
return this;
}
A method(int value, float foo) {
this.value = value;
this.foo = foo;
this.timeunit = TimeUnit.MILLISECONDS;
return this;
}
A method(long value, float foo, TimeUnit timeunit) {
this.value = value;
this.foo = foo;
this.timeunit = timeunit;
return this;
}
}
""");

@Test
void addTimeUnitDefaultMilliseconds() {
rewriteRun(
spec -> spec.recipe(new AddTimeUnitArgument("A method(int)", null)),
stubCode,
//language=java
java("""
class B {
void test() {
A a = new A();
a.method(100);
}
}
""", """
import java.util.concurrent.TimeUnit;
class B {
void test() {
A a = new A();
a.method(100, TimeUnit.MILLISECONDS);
}
}
""")
);
}

@Test
void addTimeUnitSpecificTimeUnit() {
rewriteRun(
spec -> spec.recipe(new AddTimeUnitArgument("A method(int)", TimeUnit.SECONDS)),
stubCode,
//language=java
java("""
class B {
void test() {
A a = new A();
a.method(100);
}
}
""", """
import java.util.concurrent.TimeUnit;
class B {
void test() {
A a = new A();
a.method(100, TimeUnit.SECONDS);
}
}
""")
);
}

@Test
void doesModifyMethodsWithMoreThanOneArgument() {
rewriteRun(
spec -> spec.recipe(new AddTimeUnitArgument("A method(int, float)", null)),
//language=java
stubCode,
//language=java
java("""
class B {
void test() {
A a = new A();
a.method(100, 1.0f);
}
}
""", """
import java.util.concurrent.TimeUnit;
class B {
void test() {
A a = new A();
a.method(100, 1.0f, TimeUnit.MILLISECONDS);
}
}
""")
);
}
}
Original file line number Diff line number Diff line change
@@ -115,7 +115,7 @@ void method(HttpEntity entity, String urlStr) throws Exception {
}

@Test
void useTimeoutClass() {
void addTimeUnitsToTimeoutAndDurationMethods() {
rewriteRun(
//language=java
java("""
@@ -124,56 +124,36 @@ void useTimeoutClass() {
class A {
void method() {
int connectTimeout = 500;
RequestConfig.custom()
.setConnectionRequestTimeout(300)
.setConnectTimeout(500)
.setSocketTimeout(1500);
.setConnectTimeout(connectTimeout)
.setSocketTimeout(connectTimeout * 10);
long linger = 420;
SocketConfig.custom()
.setSoTimeout(1000);
.setSoTimeout(1000)
.setSoLinger((int) linger);
}
}
""", """
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.core5.http.io.SocketConfig;
import org.apache.hc.core5.util.Timeout;
import java.util.concurrent.TimeUnit;
class A {
void method() {
int connectTimeout = 500;
RequestConfig.custom()
.setConnectionRequestTimeout(Timeout.ofMilliseconds(300))
.setConnectTimeout(Timeout.ofMilliseconds(500))
.setResponseTimeout(Timeout.ofMilliseconds(1500));
.setConnectionRequestTimeout(300, TimeUnit.MILLISECONDS)
.setConnectTimeout(connectTimeout, TimeUnit.MILLISECONDS)
.setResponseTimeout(connectTimeout * 10, TimeUnit.MILLISECONDS);
long linger = 420;
SocketConfig.custom()
.setSoTimeout(Timeout.ofMilliseconds(1000));
}
}
""")
);
}

@Test
void useTimeValueClass() {
rewriteRun(
//language=java
java("""
import org.apache.http.config.SocketConfig;
class A {
void method() {
SocketConfig.custom()
.setSoLinger(500);
}
}
""", """
import org.apache.hc.core5.http.io.SocketConfig;
import org.apache.hc.core5.util.TimeValue;
class A {
void method() {
SocketConfig.custom()
.setSoLinger(TimeValue.ofMilliseconds(500));
.setSoTimeout(1000, TimeUnit.MILLISECONDS)
.setSoLinger((int) linger, TimeUnit.MILLISECONDS);
}
}
""")