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

Lombok setter property AccessLevel.NONE is not working. #177

Closed
prasannajoshi2121 opened this issue Sep 23, 2020 · 35 comments
Closed

Lombok setter property AccessLevel.NONE is not working. #177

prasannajoshi2121 opened this issue Sep 23, 2020 · 35 comments

Comments

@prasannajoshi2121
Copy link

HI ,

`@Getter
@Setter
@NoArgsConstructor
@AllArgsConstructor
@CompiledJson
public class TestList {

@JsonProperty("test_name")
@Setter(AccessLevel.NONE)
private List list = new ArrayList<>();
}`

and model class

`public Class Model {

@JsonProperty("test_field1")
@Setter(AccessLevel.NONE)
private String field1;

}`

  1. when I am trying to serialize/deserialze , its not working for list.
    2.Similar issue can be observed if only single field is having @Setter(AccessLevel.NONE) then I am not able to deserialise that property (field1).

Version -
com.dslplatform
dsl-json-java8
1.9.6

Thank you very much , I am trying to implement DSL because I have observed the speed compare to other libraries.

@zapov
Copy link
Member

zapov commented Sep 24, 2020

So the main problem is that DSL-JSON annotation processor picks up annotations from public getters and setters instead of private fields.

While I'm not against introducing maven option to expand this behavior, it would be interesting to see if there is a way for Lombok to copy annotation to getter, which would make it DSL-JSON friendly.

@prasannajoshi2121
Copy link
Author

prasannajoshi2121 commented Sep 24, 2020

some config option is there that we can add in lombok.config is like below.
#lombok.copyableAnnotations += com.fasterxml.jackson.annotation.JsonProperty

<dependency>
  <groupId>org.projectlombok</groupId>
  <artifactId>lombok</artifactId>
  <version>1.18.12</version>
</dependency>

and its working also as its adding the JsonProperty on getter/setter methods but still not able to serialise/deserialise .

but if I am writing getter method manually (without getter annotation from lombok) , its able to serialize

@prasannajoshi2121
Copy link
Author

@zapov any comments on above ??

@zapov
Copy link
Member

zapov commented Sep 24, 2020

I don't use Lombok so my knowledge about it is rather limited.

Did you try copyableAnnotation for dsl-json JsonProperty? Jackson JsonProperty will work only if you enable its usage in Maven (I don't think it's enabled by default)

@prasannajoshi2121
Copy link
Author

"Did you try copyableAnnotation for dsl-json JsonProperty?" for DSL JsonAttribute you mean ??

"Jackson JsonProperty will work only if you enable its usage in Maven (I don't think it's enabled by default)" ??? I did not get this exactly , what kind of maven usage I need to enable ??

@zapov
Copy link
Member

zapov commented Sep 24, 2020

Yes, I meant dsl-json JsonAttribute.

There is maven setting to enable Jackson attributes: https://github.com/ngs-doo/dsl-json/blob/master/examples/Jackson/pom.xml#L37

@prasannajoshi2121
Copy link
Author

Thanks for quick reply but unfortunately

lombok.copyableAnnotations += com.dslplatform.json.JsonAttribute is not working

the maven settings provided by you is also not working :(

@zapov
Copy link
Member

zapov commented Sep 24, 2020

What do you mean the Maven setting is not working?

Anyway... I cant be of more help unless you copy paste what Lombok creates. That should point out the problem.

@prasannajoshi2121
Copy link
Author

model class

`@AllArgsConstructor
@NoArgsConstructor
@tostring
@getter
@Setter
@CompiledJson
public class SimplePojo {

@JsonProperty(value = "new_key")
private String newKey;
}
`

what LOMBOK generates

`@CompiledJson
public class SimplePojo {
@JsonProperty("new_key")
private String newKey;

public SimplePojo(@JsonProperty("new_key") String newKey) {
    this.newKey = newKey;
}

public SimplePojo() {
}

public String toString() {
    return "SimplePojo(newKey=" + this.getNewKey() + ")";
}

@JsonProperty("new_key")
public String getNewKey() {
    return this.newKey;
}

@JsonProperty("new_key")
public void setNewKey(@JsonProperty("new_key") String newKey) {
    this.newKey = newKey;
}

}
`

and what I am trying to run

`public class Test {

public static void main(String[] args) throws Exception {

DslJson<Object> dslJsonList = new DslJson<>(Settings.withRuntime());

byte[] buffer = "{\"new_key\":\"xxx\"}".getBytes("UTF-8");

SimplePojo deser = dslJsonList.deserialize(SimplePojo.class, buffer, buffer.length);


ByteArrayOutputStream ba = new ByteArrayOutputStream();
dslJsonList.serialize(deser, ba);

String str = ba.toString();
System.out.println(str);



DslJson<Object> dslJsonST =
        new DslJson<>(Settings.basicSetup());

ByteArrayOutputStream ba1 = new ByteArrayOutputStream();
dslJsonST.serialize(deser, ba1);

String str1 = ba.toString();
System.out.println(str1);

}
}`

output is :

{"newKey":null}

{"newKey":null}

and Lombok.config is
lombok.copyableAnnotations += com.fasterxml.jackson.annotation.JsonProperty

and compiler plugin for maven

`

org.apache.maven.plugins
maven-compiler-plugin
3.7.0

1.8
1.8
1.8

<Adsljson.unknown>WARNING</Adsljson.unknown>
<Adsljson.jackson>true</Adsljson.jackson>



com.dslplatform
dsl-json-java8
1.9.6


org.projectlombok
lombok
1.18.12

                <annotationProcessors>
                    <annotationProcessor>lombok.launch.AnnotationProcessorHider$AnnotationProcessor</annotationProcessor>
                    <annotationProcessor>com.dslplatform.json.processor.CompiledJsonAnnotationProcessor</annotationProcessor>
                </annotationProcessors>
            </configuration>
        </plugin>`

@prasannajoshi2121
Copy link
Author

prasannajoshi2121 commented Sep 24, 2020

@zapov

seems like I found the issue , when we are running with
DslJson dslJsonList = new DslJson<>(Settings.withRuntime());

@JsonProperty never works even if we manually add on getters/setters or generate using lombok because of following method call ,

https://github.com/ngs-doo/dsl-json/blob/master/java8/src/main/java/com/dslplatform/json/runtime/ObjectAnalyzer.java#L231

final String name = Analysis.beanOrActualName(mget.getName());

which return fieldname but not the annotated value , based on this value we are calculating WeakHash. so please consider kind of below code which can be enhanced .

Another overloaded method to be added in Analysis.Java with method signature as Method instead of String.

https://github.com/ngs-doo/dsl-json/blob/a5e5403e6ab6c41a8ee863b27514bcecc7248fb1/library/src/main/java/com/dslplatform/json/processor/Analysis.java

you can have some flag to execute only if Jackson is enabled and try to get value from annotation if its not present the call
your original method "beanOrActualName". I have tested this locally and its working as expected in both cases (Manual and Lombok generated )

public static String beanOrActualName(Method meth) {
String result = null;
if(System.getProperty("JacksonEnabled")
for (Annotation annotation : meth.getAnnotations()) {
Class<? extends Annotation> type = annotation.annotationType();
if(type.getCanonicalName().equals("com.fasterxml.jackson.annotation.JsonProperty")) {
for (Method method : type.getDeclaredMethods()) {
Object value = null;
try {
value = method.invoke(annotation, (Object[]) null);
if(value instanceof String && !((String)value).isEmpty()){
result = (String)value;
}
} catch (IllegalAccessException e) {
e.printStackTrace();
} catch (InvocationTargetException e) {
e.printStackTrace();
}
}
}
}
if(null!=result) return result;
else return beanOrActualName(meth.getName());
}

please let me know your thoughts :)

@zapov
Copy link
Member

zapov commented Sep 25, 2020

Not really ;(

You were testing runtime behavior of dsl-json which supports only a subset of features.
When in doubt disable runtime settings to test if compile time setup is working.

While I don't have anything major against improving the runtime parity, its much better to rely on compile time setup, so I dont expect much demand for it.

I don't have much time ATM, but I'll try to find some time to make a reproducible on the Lombok example and see whats going on. But again, please try to use JsonAttribute from dsl-json and setup Lombok to copy that attribute... which should minimize various confusions.

@prasannajoshi2121
Copy link
Author

Hi ,

why I don't want to switch from Jackson's JsonProperty to DSL JsonAttribute is

  1. I am having 100+ Model classs which are shared across multiple app , most of them using Jackson for data binding.
  2. The advantages of not changing JsonProperty is I can go back to Jackson at any time if we found similar issue like above

@zapov
Copy link
Member

zapov commented Sep 25, 2020

Mkay. Then I'll look why Jackson property would not be working. If you can create exact reproducible project that would be helpful... so I don't need to copy paste from here and setup stuff

@prasannajoshi2121
Copy link
Author

ok sure. I will give you reproducible example.

@prasannajoshi2121
Copy link
Author

prasannajoshi2121 commented Sep 25, 2020

dslissue.zip

This is having both issues ,

  1. With lombok , neither runtime or compile time works.
  2. Without lombok , if we add manual annotation on getter then Compile time works but runtime does not.

also If you compare Converter classes generated by DSL for both model class (with and without lombok) ,you will observe different quoted_field and named_field values for both converters.

@zapov
Copy link
Member

zapov commented Sep 25, 2020

So there seems to be a problem with dsl-json that it's not picking up annotation info from setter method, only from getter.
This is something which should be fixed.

But on the other hand Lombok does not create annotation on getter, only on setter.
I don't know how to make Lombok create annotation on getter.

@prasannajoshi2121
Copy link
Author

Hi ,

I think Lombok is creating annotation on getter/setters both. if you take look at generated SimplePojoLombok.class in above sample project , you can see following :

import com.dslplatform.json.CompiledJson;
import com.fasterxml.jackson.annotation.JsonProperty;

@CompiledJson
public class SimplePojoLombok {
@JsonProperty("new_key")
private String newKey;

public SimplePojoLombok(@JsonProperty("new_key") String newKey) {
    this.newKey = newKey;
}

public SimplePojoLombok() {
}

public String toString() {
    return "SimplePojoLombok(newKey=" + this.getNewKey()")";
}

@JsonProperty("new_key")
public String getNewKey() {
    return this.newKey;
}

@JsonProperty("new_key")
public void setNewKey(@JsonProperty("new_key") String newKey) {
    this.newKey = newKey;
}

}

which is generating annotations. on both getter/setter

@zapov
Copy link
Member

zapov commented Sep 26, 2020

Thats not getting created for me.
This is from SimplePojoLombok.class

import com.dslplatform.json.CompiledJson;
import com.fasterxml.jackson.annotation.JsonProperty;

@CompiledJson
public class SimplePojoLombok {
    @JsonProperty("new_key")
    private String newKey;

    public SimplePojoLombok(String newKey) {
        this.newKey = newKey;
    }

    public SimplePojoLombok() {
    }

    public String toString() {
        return "SimplePojoLombok(newKey=" + this.getNewKey() + ")";
    }

    @JsonProperty("new_key")
    public void setNewKey(String newKey) {
        this.newKey = newKey;
    }

    public String getNewKey() {
        return this.newKey;
    }
}

@prasannajoshi2121
Copy link
Author

its quite strange , I hope you are having following lombok dependency only.

<dependency>
        <groupId>org.projectlombok</groupId>
        <artifactId>lombok</artifactId>
        <version>1.18.12</version>
    </dependency>

and POM 's build is having following plugin

org.apache.maven.plugins maven-compiler-plugin 3.7.0 1.8 1.8 1.8 WARNING true com.dslplatform dsl-json-java8 1.9.6 org.projectlombok lombok 1.18.12
        <annotationProcessors>
            <annotationProcessor>lombok.launch.AnnotationProcessorHider$AnnotationProcessor</annotationProcessor>
            <annotationProcessor>com.dslplatform.json.processor.CompiledJsonAnnotationProcessor</annotationProcessor>
        </annotationProcessors>
    </configuration>
</plugin>
</plugins>

@zapov
Copy link
Member

zapov commented Sep 26, 2020

I just opened the project in IntelliJ. Did not change anything with the setup.
Once you ran tests from maven option it fails (since some tests are failing)
but in the target/test-classes this is what is created for me.

Anyway... you can see thats that the cause of problems by removing getter annotation from SimplePojo and having the TestSimplePojo.testCompileTime fails. Otherwise if there is annotation on getter it will succeed.

btw. it is expected that dsl-json for runtime only fails, since it does not support that feature

@prasannajoshi2121
Copy link
Author

Ok following are things :

// without Lombok scenario :

  1. TestSimplePojo.testRuntime - is still failing even if SimplePojo is having getter/setter having annotation
    why its not supporting this feature for runtime ??

// with Lombok scenario :
as I tested and was able to generate annotation on both getter/setter using lombok and it was failing still failing for
TestSimplePojolombok.testCompileTime.

I am using - java 8 ( could you please try with same java version ).

also can you confirm if this is enough for initialising dsl-json for compile time scenario or anything else is needed ??
DslJson dslJsonList = new DslJson<>(Settings.basicSetup());

@zapov
Copy link
Member

zapov commented Sep 26, 2020

There is some problem with your environment causing this issues.
How are you running the app?

I just now ran \apache-maven-3.3.9\bin\mvn clean test on your zip
and it resulted in 3 out of 4 failures. The pass was on the TestSimplePojo.testCompileTime

java -version
openjdk version "1.8.0_252"
OpenJDK Runtime Environment (AdoptOpenJDK)(build 1.8.0_252-b09)
OpenJDK 64-Bit Server VM (AdoptOpenJDK)(build 25.252-b09, mixed mode)

The runtime analysis does not look into annotations and extract info from there, such as custom attribute name. This is only implemented for compile time analysis.

Yes, this should be sufficient (as this is the setup from tests).
There are some advanced usages which could be causing problems, such as custom classloader - where you need to specify classloader during setup, but this is not the case with this tests.

If I comment out those 3 problematic tests this is output I get

[INFO] Scanning for projects...
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building dsl-issue 1.0-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ dsl-issue ---
[INFO] Deleting D:\Downloads\dslissue2\dslissue\target
[INFO]
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ dsl-issue
---
[WARNING] Using platform encoding (Cp1250 actually) to copy filtered resources,
i.e. build is platform dependent!
[INFO] Copying 0 resource
[INFO]
[INFO] --- maven-compiler-plugin:3.7.0:compile (default-compile) @ dsl-issue ---

[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- maven-resources-plugin:2.6:testResources (default-testResources) @ ds
l-issue ---
[WARNING] Using platform encoding (Cp1250 actually) to copy filtered resources,
i.e. build is platform dependent!
[INFO] skip non existing resourceDirectory D:\Downloads\dslissue2\dslissue\src\t
est\resources
[INFO]
[INFO] --- maven-compiler-plugin:3.7.0:testCompile (default-testCompile) @ dsl-i
ssue ---
[INFO] Changes detected - recompiling the module!
[WARNING] File encoding has not been set, using platform encoding Cp1250, i.e. b
uild is platform dependent!
[INFO] Compiling 4 source files to D:\Downloads\dslissue2\dslissue\target\test-c
lasses
[INFO] /D:/Downloads/dslissue2/dslissue/target/generated-test-sources/test-annot
ations/_SimplePojoLombok_DslJsonConverter.java: Some input files use unchecked o
r unsafe operations.
[INFO] /D:/Downloads/dslissue2/dslissue/target/generated-test-sources/test-annot
ations/_SimplePojoLombok_DslJsonConverter.java: Recompile with -Xlint:unchecked
for details.
[INFO]
[INFO] --- maven-surefire-plugin:2.12.4:test (default-test) @ dsl-issue ---
[INFO] Surefire report directory: D:\Downloads\dslissue2\dslissue\target\surefir
e-reports

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running TestSimplePojo
{"new_key":"test"}
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.09 sec

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 2.676 s
[INFO] Finished at: 2020-09-26T09:33:54+02:00
[INFO] Final Memory: 21M/485M
[INFO] ------------------------------------------------------------------------

If you are getting anything else... thats something to look into

@prasannajoshi2121
Copy link
Author

Hi , those test cases are for reproducing the issue.

Test class : TestSimplePojo.java - this will test simple Pojo's serialisation/deserialisation which is having @JsonProperty annotation on getter/setters manually added by me.

TC-1 - testRuntime - this is to test simple Pojo's serialisation/deserialisation using runtime settings.
TC-2 - testCompileTime - this is to test simple Pojo's serialisation/deserialisation using basic setup settings.

Test class : TestSimplePojolombok.java - this will test SimplePojoLombok Pojo's serialisation/deserialisation which is having @JsonProperty annotation on getter/setters generated by Lombok.

TC-3 - testRuntime - this is to test SimplePojoLombok Pojo's serialisation/deserialisation using runtime settings.
TC-4- testCompileTime - this is to test SimplePojoLombok Pojo's serialisation/deserialisation using basic setup settings.

from all 4 test cases , 3 were supposed to fail ( TC-1,TC-3,TC-4 which is reproducible example ).

Now as you confirmed , "The runtime analysis does not look into annotations and extract info from there, such as custom attribute name. This is only implemented for compile time analysis."

we can ignore/drop TC-1 and TC-3 (runtime settings scenario) for now.

but still I am not sure why TC-4 is failing where annotation are getting generated using lombok but its failing . I am attaching project one more time where I have removed runtime test cases. Lets concentrate on compile time test case.

The project which I am attaching now its having 2 test case :

  1. TestSimplePojo.java - testCompileTime() which is success and working
  2. TestSimplePojolombok - testCompileTime() which is failing.

Please open project in IntelliJ , do "clean install -DskipTests". once its build please check how the class is getting generated under

target/test-classes - SimplePojoLombok.class

if its having annotation above getter method then we are good else we will close this issue :(.

Project - dslissue copy.zip

@zapov
Copy link
Member

zapov commented Sep 26, 2020

Yes, I'm now seeing the same thing as you since you included lombok.config

I can only guess why it's not working... which would be that Lombok uses some strange method of code gen which is not getting picked up by annotation processor, or that this annotations are being created after the dsl-json annotation processor runs.

Thing which you could try is to separate this pojo classes in model.jar and have Lombok set them up as expected.
Then have dsl-json in a different jar with a reference to pojo classes from model.jar since dsl-json will then explore their relationships.

Thus if you create some dummy pojo in your project and annotate it with @CompiledJson plus have public property to top level model classes it should work (although its hacky and not really friendly)

Again, I don't use Lombok much and don't know what magic it does, all I'm aware that they don't use the standard annotation processor infrastructure.

Anyway, ideally there should be an option of dsl-json picking up annotations from private members of relevant getters which should resolve this issue.

I'll look now how feasible is that...

@prasannajoshi2121
Copy link
Author

prasannajoshi2121 commented Sep 26, 2020

perfect , I was also testing one more scenario where similar to String as property inside model , I added StringBuffer property inside model ,while deserialising Json I did not find any issue but during serialisation Stringbuffer was not getting populated properly .

Note : Again this is NOT Lombok generated properties but added manually and I am testing with Compile time settings .

Attaching project again with test case.

Test case : TestSimplePojo .java -> testCompileTimeWithStringBuffer

dslissue copy.zip

please let me know if this is also issue

@zapov
Copy link
Member

zapov commented Sep 26, 2020

Pushed a change which will now cause analysis of private members too.

As for StringBuffer, there is only reader registered in the system, thus it's not getting picked up: https://github.com/ngs-doo/dsl-json/blob/master/library/src/main/java/com/dslplatform/json/DslJson.java#L587
You could register your own converter which should resolve the issue: https://github.com/ngs-doo/dsl-json/blob/master/examples/MavenJava6/src/main/java/com/dslplatform/maven/Example.java#L115

@prasannajoshi2121
Copy link
Author

Thanks , its working now 👍 , StringBuffer still I am trying to figure out as like other inbuilt classes , it should also work if we are not having any custom requirement.

@prasannajoshi2121
Copy link
Author

Hi @zapov , reopening issue because the original thing is still not solved.

"Lombok setter property AccessLevel.NONE is not working."

mostly the use case will be for following :

@Setter(AccessLevel.NONE) // Lombok won't generate setter for this.
@JsonProperty("model_name")
private List model =new ArrayList<>();

// if we don't want Lombok then

@JsonProperty("model_name")
private List model =new ArrayList<>();

public getModel(){
return model;
}

NO SETTER METHOD FOR MODEL ( as its list , we want to add value inside list instead. of setting)

in this case , if there are no setter methods available then we need to check for field if its of type (List or Set) and then add value in that collection accordingly instead of setting value.

Please let me know your thoughts on this :)

@zapov
Copy link
Member

zapov commented Sep 28, 2020

I think I only extended the case to support private fields for bean standard. It could be extended to support ctor arguments (should be a nice learning experience)

@zapov
Copy link
Member

zapov commented Sep 28, 2020

Ah, you want to have lists without setters. Thats a different problem. dsl-json does not know how to handle this (although it doesn't seem like it would be difficult to support). It does require some additional hints/annotations for this ;(

@prasannajoshi2121
Copy link
Author

prasannajoshi2121 commented Sep 28, 2020

I actually compared both generated _DSL_converters files , I observed that compiler is not at all generating fields if setter method is not present.

private static final byte[] quoted_buffer;
private static final byte[] name_buffer;

I guess @Setter(AccessLevel.NONE) itself is enough hint for you understand that this field may be of type of collection and you can generate its setter or special method like

serialzelist( <? extends Collections > list, T value)
{
list.add(value)
}

something like that , you can enable this functionality only if Lombok's @Setter(AccessLevel.NONE) annotation is present.

if you want to extend this functionality to NON-Lombok methods then you can introduce one more flag inside your standard
@JsonAttribute annotation.

@zapov
Copy link
Member

zapov commented Oct 1, 2020

I guess having JsonProperty should indicate the desire for resolution and could cope with missing setter for collections.
I'll try to find some time to look into this...

@prasannajoshi2121
Copy link
Author

Thanks , I am keeping this issue open so that you can update me here once solution is available.

@zapov
Copy link
Member

zapov commented Oct 8, 2020

This should work with latest commit. Although you will first need to add NonNull annotation on it

@zapov
Copy link
Member

zapov commented Oct 11, 2020

v1.9.6 released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants