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

Add Transient annotation to private accessor methods #425

Merged
merged 23 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
9d0ecdb
Initial implementation with test
evie-lau Feb 22, 2024
7b13dc6
Polishing and add another test
evie-lau Feb 22, 2024
8cb83a6
Check if the method return value is a field from the parent class
evie-lau Feb 22, 2024
9c611b5
Merge branch 'main' into transientGetter
evie-lau Feb 23, 2024
e196629
Prevent applying annotation twice; add unhandled case
timtebeek Feb 24, 2024
932ab06
Add test to show annotated field not changed again
timtebeek Feb 24, 2024
b975a34
Handle FieldAccess and Literal returns, add test for Literal
evie-lau Feb 26, 2024
a1092f7
Cleanup
evie-lau Feb 26, 2024
b0f9e2a
Merge branch 'main' into transientGetter
timtebeek Feb 28, 2024
17c703c
Small improvements and comments
evie-lau Feb 28, 2024
c2a51b5
Merge branch 'main' into transientGetter
evie-lau Apr 10, 2024
58893dd
Refactor, add test
evie-lau Apr 10, 2024
bf9dab1
Handle private accessor in inner class of an entity to match WAMT beh…
evie-lau Apr 10, 2024
ef81876
Fix test formatting
evie-lau Apr 10, 2024
8720c86
Merge branch 'main' into transientGetter
evie-lau Apr 24, 2024
2c72bf8
Merge branch 'main' into transientGetter
evie-lau Apr 25, 2024
32a6cfe
Apply suggestions from code review
timtebeek Apr 26, 2024
f35e327
Merge branch 'main' into transientGetter
evie-lau Apr 27, 2024
f42384d
Add protected getter to test
evie-lau May 1, 2024
d0a9660
Check all returns in method for class field variables
evie-lau May 3, 2024
d6e3b11
Add complex logic test without field access
evie-lau May 3, 2024
3479ecd
Refactor for efficiency
evie-lau May 6, 2024
2613275
Add logic comments
evie-lau May 6, 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* 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.javax;

import lombok.EqualsAndHashCode;
import lombok.Value;
import org.openrewrite.ExecutionContext;
import org.openrewrite.Preconditions;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.search.FindAnnotations;
import org.openrewrite.java.search.UsesType;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;

import java.util.Comparator;

@Value
@EqualsAndHashCode(callSuper = false)
public class AddTransientAnnotationToPrivateAccessor extends Recipe {

@Override
public String getDisplayName() {
return "Private accessor methods must have a `@Transient` annotation";
}

@Override
public String getDescription() {
return "According to the JPA 2.1 specification, when property access is used, the property accessor methods " +
"must be public or protected. OpenJPA ignores any private accessor methods, whereas EclipseLink persists " +
"those attributes. To ignore private accessor methods in EclipseLink, the methods must have a " +
"`@Transient` annotation.";
}


@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(
new UsesType<>("javax.persistence.Entity", true),
new JavaIsoVisitor<ExecutionContext>() {
@Override
public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) {
if (!FindAnnotations.find(classDecl, "javax.persistence.Entity").isEmpty()){
return super.visitClassDeclaration(classDecl, ctx);
}
// Exit if parent class is not tagged for JPA
return classDecl;
}

@Override
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration md, ExecutionContext ctx) {
if (isPrivateAccessorMethodWithoutTransientAnnotation(md)) {// Add @Transient annotation
maybeAddImport("javax.persistence.Transient");
return JavaTemplate.builder("@Transient")
.contextSensitive()
.javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "javax.persistence-api-2.2"))
.imports("javax.persistence.Transient")
.build()
.apply(getCursor(), md.getCoordinates().addAnnotation(Comparator.comparing(J.Annotation::getSimpleName)));
}
return md;
}

private boolean isPrivateAccessorMethodWithoutTransientAnnotation(J.MethodDeclaration method) {
return method.hasModifier(J.Modifier.Type.Private)
&& method.getParameters().get(0) instanceof J.Empty
&& method.getReturnTypeExpression().getType() != JavaType.Primitive.Void
&& FindAnnotations.find(method, "javax.persistence.Transient").isEmpty()
&& method.getBody().getStatements().get(0) instanceof J.Return
&& methodReturnsFieldFromClass((J.Return) method.getBody().getStatements().get(0));
}

/**
* Check if the given method returns a field defined in the given class
*/
private boolean methodReturnsFieldFromClass(J.Return returnStatement) {
J.ClassDeclaration classDecl = getCursor().dropParentUntil(parent -> parent instanceof J.ClassDeclaration).getValue();
JavaType.Variable returnedVar = ((J.Identifier)returnStatement.getExpression()).getFieldType();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about matching on the field type here ; I'd expect we'd want to match that the identifier or accessed field name are the same as what's returned.

Copy link
Contributor Author

@evie-lau evie-lau Feb 26, 2024

Choose a reason for hiding this comment

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

It seemed like the Identifier sometimes had different object numbers when I was debugging, but getting it's FieldType object always matched up (which also contains the variable name).

Copy link
Contributor

Choose a reason for hiding this comment

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

the return expression can be a lot of things, but I imagine we'd only want to add an annotation when it's exactly an identifier or field access that references a field in the surrounding class; I think that were we were headed already right?

Copy link
Contributor Author

@evie-lau evie-lau Feb 26, 2024

Choose a reason for hiding this comment

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

For visual reference:
image
The J.Identifier object (name) didn't match up between the class's field Identifier and the method's returned Identifier. simpleName and fieldType were the highest level matches I found, and comparing the objects seemed like a better choice than comparing strings.

return classDecl.getBody().getStatements().stream()
.filter(statement -> statement instanceof J.VariableDeclarations)
.map(J.VariableDeclarations.class::cast)
.map(J.VariableDeclarations::getVariables)
.flatMap(vars -> vars.stream())
.map(var -> var.getName().getFieldType())
.anyMatch(var -> var.equals(returnedVar));
}
}
);
}
}
evie-lau marked this conversation as resolved.
Show resolved Hide resolved
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ tags:
- javaee7
- deprecated
recipeList:
- org.openrewrite.java.migrate.javax.AddTableGenerator
- org.openrewrite.java.migrate.javax.AddTableGenerator
- org.openrewrite.java.migrate.javax.AddTransientAnnotationToPrivateAccessor
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
/*
* 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.javax;

import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.InMemoryExecutionContext;
import org.openrewrite.java.JavaParser;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

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

class AddTransientAnnotationToPrivateAccessorTest implements RewriteTest {
@Override
public void defaults(RecipeSpec spec) {
evie-lau marked this conversation as resolved.
Show resolved Hide resolved
spec.parser(JavaParser.fromJavaVersion().classpathFromResources(new InMemoryExecutionContext(), "javax.persistence-api-2.2"))
.recipe(new AddTransientAnnotationToPrivateAccessor());
}

@DocumentExample
@Test
void addTransientToMethodReturningIdentifier() {
//language=java
rewriteRun(
java(
"""
package entities;

import javax.persistence.Entity;
import javax.persistence.Id;

@Entity
public class PrivateAccessor {
private int id;
private int nonPersistentField;

@Id
public int getId() {
return id;
}

private int getNonPersistentField() {
return nonPersistentField;
}
}
""",
"""
package entities;

import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.Transient;

@Entity
public class PrivateAccessor {
private int id;
private int nonPersistentField;

@Id
public int getId() {
return id;
}

@Transient
private int getNonPersistentField() {
return nonPersistentField;
}
}
"""
)
);
}

@Test
void addTransientToMethodReturningFieldAccess() {
//language=java
rewriteRun(
java(
"""
package entities;

import javax.persistence.Entity;
import javax.persistence.Id;

@Entity
public class PrivateAccessor {
private int id;
private int nonPersistentField;

@Id
public int getId() {
return id;
}

private int getNonPersistentField() {
return this.nonPersistentField;
}
}
""",
"""
package entities;

import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.Transient;

@Entity
public class PrivateAccessor {
private int id;
private int nonPersistentField;

@Id
public int getId() {
return id;
}

@Transient
private int getNonPersistentField() {
return this.nonPersistentField;
}
}
"""
)
);
}

@Test
void doNotChangePublicGetter() {
//language=java
rewriteRun(
java(
"""
package entities;

import javax.persistence.Entity;
import javax.persistence.Id;

@Entity
public class PrivateAccessor {
private int id;
private int field;

@Id
public int getId() {
return id;
}

public int getField() {
return field; // Public method
}
}
"""
)
);
}


@Test
void doNotChangeVoidReturnType() {
//language=java
rewriteRun(
java(
"""
package entities;

import javax.persistence.Entity;
import javax.persistence.Id;

@Entity
public class PrivateAccessor {
private int id;
private int field;

@Id
public int getId() {
return id;
}

private void getField() {
// void return type
}
}
"""
)
);
}
}
evie-lau marked this conversation as resolved.
Show resolved Hide resolved
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
Loading