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

Cxf dev2 #1516

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Cxf dev2 #1516

wants to merge 4 commits into from

Conversation

prathyushreddylpr
Copy link
Contributor

Description

Fixed the flaky tests named testDeserialization inside SerializationTest.java and AttributeTest.java classes.

Root Cause

The test testDeserialization has been reported as flaky when run with the NonDex tool. The tests failed because of the serialization of Testbean and AttributeTestBean classes, where the order of elements in the serialized XML changes between runs. The contents of the serialized strings do not remain constant and hence the tests are failing.

Fix

To define a specific order for the XML elements when the classes TestBean1 and AttributeTestBean are serialized, we need to add the propOrder attribute to the @XmlType annotation. This attribute explicitly specifies the order in which fields should be serialized.

How this has been tested?

Java: openjdk version "11.0.20.1"
Maven: Apache Maven 3.6.3

  1. Module build - Successful
    Command used -
mvn install -pl core -am -DskipTests
  1. Regular test - Successful
    Command used -
mvn -pl rt/javascript/javascript-tests test -Dtest=org.apache.cxf.javascript.types.SerializationTest#testDeserialization
mvn -pl rt/javascript/javascript-tests test -Dtest=org.apache.cxf.javascript.types.AttributeTest#testDeserialization
  1. NonDex test - Failed
    Command used -
mvn -pl rt/javascript/javascript-tests edu.illinois:nondex-maven-plugin:2.1.7-SNAPSHOT:nondex -Dtest=org.apache.cxf.javascript.types.SerializationTest#testDeserialization

mvn -pl rt/javascript/javascript-tests edu.illinois:nondex-maven-plugin:2.1.7-SNAPSHOT:nondex -Dtest=org.apache.cxf.javascript.types.AttributeTest#testDeserialization -DnondexRuns=10

NonDex test passed after the fix.

@reta
Copy link
Member

reta commented Nov 11, 2023

@prathyushreddylpr I don't see any comparison logic in org.apache.cxf.javascript.types.SerializationTest.testDeserialization that relies on order of the properties. where this hypothesis is coming from?

@prathyushreddylpr
Copy link
Contributor Author

@prathyushreddylpr I don't see any comparison logic in org.apache.cxf.javascript.types.SerializationTest.testDeserialization that relies on order of the properties. where this hypothesis is coming from?

In the 'testDeserialization' there is a call to 'rhinoCallInContext' which internally calls deserializeTestBean3_1(serialized string) here: '

function deserializeTestBean3_1(xmlString)
' and the actual comparison is happening here.

@reta
Copy link
Member

reta commented Nov 11, 2023

@prathyushreddylpr I don't see any comparison logic in org.apache.cxf.javascript.types.SerializationTest.testDeserialization that relies on order of the properties. where this hypothesis is coming from?

In the 'testDeserialization' there is a call to 'rhinoCallInContext' which internally calls deserializeTestBean3_1(serialized string) here: '

https://github.com/apache/cxf/blob/74069a140737603a0a59435de308401c4351a80b/rt/javascript/javascript-tests/src
' and the actual comparison is happening here.

But it compares deserialized form of each attribute, the order does not matter here

@prathyushreddylpr
Copy link
Contributor Author

@prathyushreddylpr I don't see any comparison logic in org.apache.cxf.javascript.types.SerializationTest.testDeserialization that relies on order of the properties. where this hypothesis is coming from?

In the 'testDeserialization' there is a call to 'rhinoCallInContext' which internally calls deserializeTestBean3_1(serialized string) here: '
https://github.com/apache/cxf/blob/74069a140737603a0a59435de308401c4351a80b/rt/javascript/javascript-tests/src
' and the actual comparison is happening here.

But it compares deserialized form of each attribute, the order does not matter here

Yes but when the class is serialized to a string, the attributes does not follow any particular and when the string is compared with certain value it is causing the issue. For example, below are two serialized strings for two different runs:
First:
image

Second:
image

@reta
Copy link
Member

reta commented Nov 13, 2023

Yes but when the class is serialized to a string, the attributes does not follow any particular and when the string is compared with certain value it is causing the issue. For example, below are two serialized strings for two different runs:

Could you please point out where this serialized string is compared to another serialized strings? I have found none such places, and the order does not matter here as far as all attributes have the same values.

@prathyushreddylpr
Copy link
Contributor Author

prathyushreddylpr commented Nov 14, 2023

Yes but when the class is serialized to a string, the attributes does not follow any particular and when the string is compared with certain value it is causing the issue. For example, below are two serialized strings for two different runs:

Could you please point out where this serialized string is compared to another serialized strings? I have found none such places, and the order does not matter here as far as all attributes have the same values.

The comparison is happening here:

function deserializeTestBean3_1(xmlString)

image

In the above code bean.getStringItem is expecting "bean1>stringItem". But serialized string contains two string items(example is provided in the above comment) and one of them contained the value "required=true" and hence the order is important

@reta
Copy link
Member

reta commented Nov 15, 2023

But serialized string contains two string items(example is provided in the above comment) and one of them contained the value "required=true" and hence the order is important

I don't see two items, I see (from your examples):

image

and

image

The other two string items belong to beanTwoItem which is nested serialized bean

@prathyushreddylpr
Copy link
Contributor Author

But serialized string contains two string items(example is provided in the above comment) and one of them contained the value "required=true" and hence the order is important

I don't see two items, I see (from your examples):

image

and

image

The other two string items belong to beanTwoItem which is nested serialized bean

Sorry for the confusion, I have debugged the issue further and understood that the issue is not with the serializing of the string but with the parsing of the 'xmlString' here:

I observed that sometimes when the string is converted to XML, the attributes' values become null(see screenshot below), and defining propOrder will trigger a more consistent marshaling behavior within JAXB. When propOrder is defined, JAXB takes additional steps to ensure all properties are accounted for and processed systematically, which it might not do when relying on the default (implicit) order.

image

@reta
Copy link
Member

reta commented Nov 24, 2023

When propOrder is defined, JAXB takes additional steps to ensure all properties are accounted for and processed systematically, which it might not do when relying on the default (implicit) order.

Thanks @prathyushreddylpr , I am very surprised by your findings but if that is indeed the case, this definitely not a CXF but JAXB implementation issue (we use Glassfish one), could you please report it there?

@prathyushreddylpr
Copy link
Contributor Author

Glassfish

@reta , can you please let me know how I can report this to JAXB? Is this:https://github.com/eclipse-ee4j/glassfish the glassfish repo that you mentioned?

@reta
Copy link
Member

reta commented Dec 10, 2023

@reta , can you please let me know how I can report this to JAXB? Is this:https://github.com/eclipse-ee4j/glassfish the glassfish repo that you mentioned?

@prathyushreddylpr I suppose you could open an issue there with the test case that demonstrates the problem with JAXB implementation (you may need to craft the reproducer without Apache CXF context since as per your explanation, it is purely dictated by JAXB with propOrder setting).

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

Successfully merging this pull request may close these issues.

2 participants