search

adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
737 stars 745 forks source link

[Content Fragment] Non-deterministic behavior of `HashMap` might fail tests using `ContentFragment.getExportedElements()` #2608

Closed SaaiVenkat closed 9 months ago

SaaiVenkat commented 11 months ago

Bug Report

Current Behavior

Steps to Reproduce

Compile the module byte-buddy-dep

mvn clean install -pl bundles/core -am -DskipTests

(Optional) Run the unit test

mvn -pl bundles/core test -Dtest=

Run the unit test using NonDex

mvn -pl bundles/core edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=

- Reproducing the above steps for Tests 1, 2, and 3, will produce the following result:

[INFO] Results: [INFO] [ERROR] Failures: [ERROR] ContentFragmentImplTest.structured:137->assertContentFragment:283->assertContentFragment:311 Element has wrong name ==> expected:

but was: [INFO] [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 [INFO] 

- Reproducing the above steps for Tests 4, 5, and 6 will produce the following result:

[INFO] Results: [INFO] [ERROR] Failures: [ERROR] ElementsDataSourceServletTest.testComponentPathStructured:134->AbstractContentFragmentDataSourceServletTest.assertDataSource:127 expected:

but was: [INFO] [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 [INFO] 


**Expected behavior/code**
- For any amount of runs, the order of `DAMContentFragment.getExportedElementsOrder()` should have been
  1) MAIN
  2) SECOND
- The test should have passed successfully without any errors with `NonDex`.

**Environment**
- Core Components version: `2.23.5-SNAPSHOT`
- Java version: `Java(TM) SE Runtime Environment (build 1.8.0_381-b09)`

**Possible Solution**
<!--- If you have suggestions for the bug fix. Please also consider creating a Pull Request 🤘. -->

- Since the change in ordering of elements loaded by `resource.getChild()` when mocking the `ContentFragment` causes the issue, sorting the elements loaded by the resource will solve the issue. Sorting the elements based on the `resource.getValueMap().keySet()` ensures that the elements be returned in the order `MAIN` followed by `SECOND`.
- Below is the diff needed to solve the above mentioned issue.

-/+ com.adobe.cq.wcm.core.components.internal.models.v1.contentfragment.ContentFragmentMockAdapter.java | LineNo: 111

  • for (String name : master.getValueMap().keySet().stream().sorted().collect(Collectors.toList()))
  • for (String name : master.getValueMap().keySet())
  • This change is made only in the ContentFragmentMockAdapter.java because the main issue results from incorrectly loading the test data.
  • If the proposed solution looks good, I can create a PR.

Additional context / Screenshots

Here's an example scenario where the change in ordering occurs when run through NonDex for the test com.adobe.cq.wcm.core.components.internal.models.v1.contentfragment.ContentFragmentImplTest.structured. Since the issue occurs when loading the json resource for all the tests, the below scenario applies to all the tests.

1) The resource loaded will be specific for each test. However, one of the common files is bundles/core/src/test/resources/contentfragment/test-content-dam-contentfragments.json. This is the JSON from which the mocks are loaded and asserted.

Screenshot 2023-10-31 at 7 19 14 PM

2) The contents of the JSON are parsed recursively at apache sling's JsonContentParser.java:87. The contents are loaded from the resource into JsonObject. This object uses LinkedHashMap to store the tree structure. So, the issue is not in the JsonObject. Looking in the below image, we could observe that object.keySet() gives the elements in the order of the above image. However, each json object is parsed to the resource contents by extracting the values to properties map, which in turn is a HashMap. The order of values in this HashMap might differ based on its implementation in a specific environment or java version.

Screenshot 2023-10-31 at 2 33 40 PM Screenshot 2023-10-31 at 2 35 05 PM

3) While converting the values of the json to their respective resources, the properties map populated in the above imaged is passed to the LoaderContentHandler.java:82. Since this is a HashMap, content.entrySet() might return not return the same order for all the runs. This non-deterministic order map is provider to the JcrResourceProvider, which converts the json values to JcrValueMap used by the ContentFragmentMockAdapter

Screenshot 2023-10-31 at 2 37 40 PM

4) Accessing this map's key set at ContentFragmentMockAdapter gives the following order

Screenshot 2023-10-31 at 2 40 38 PM

5) The mock elements created based on the above order of map is MAIN followed by SECOND

Screenshot 2023-10-31 at 2 55 18 PM

This is the second run for the same method run through NonDex.

1) The order of elements from the JsonObject remains constant because of LinkedHashMap

Screenshot 2023-10-31 at 2 43 57 PM

2) However, when the properties hash map is inserted, the change in order could be observed.

Screenshot 2023-10-31 at 2 44 30 PM

3) Creating the JcrValueMap based on this ordering

Screenshot 2023-10-31 at 2 46 21 PM

4) Accessing this map's key set at ContentFragmentMockAdapter gives a different order

Screenshot 2023-10-31 at 2 47 14 PM

5) The mock elements created based on the above order of map is SECOND followed by MAIN

Screenshot 2023-10-31 at 2 54 40 PM