perf: Use and re-use ObjectWriter for data serialization (fixes #1381) #1382
Conversation
| * @since com.adobe.cq.wcm.core.components.models 12.12.0 | ||
| */ | ||
| @Nullable | ||
| @JsonProperty("dataLayer") | ||
| @JsonSerialize(using = ComponentDataModelSerializer.class) |
There was a problem hiding this comment.
The removal of this line breaks the serialization of the data model.
The serialized format should be along the lines of:
{
"<ComponentID>": {
"....serialized ComponentData..."
}
}The results of this change cause
{
"....serialized ComponentData..."
}There was a problem hiding this comment.
Here's a diff of actual test output (expected on left, actual on right).
Please make sure to run the tests.
There was a problem hiding this comment.
This PR should probably be made against
adobe:developmentand notadobe:master.
Thanks, I changed the base of this PR.
The removal of this line breaks the serialization of the data model.
Yes, it did. The change was intentional, though. As I mentioned in the PR's description it's probably not a good idea to reference the serializer (which is located inside "internal") from within the public API.
As far as I can tell the serializer is only necessary to be used inside Unit tests, correct? Are consumers of the API expecting to be able to serialize instances of Component directly which in turn should use the Serializer? Or are consumers only supposed to call ComponentData#getJson?
(This might be a concern especially when using ComponentExporter)
There was a problem hiding this comment.
Component/ContainerExporter serialization is important for SPA developers (which consume .model.json output instead of rendered HTML attributes to feed the DataLayer).
dhardtke
force-pushed
the
perf/1381-use-re-usable-objectwriter
branch
from
65a542f
to
2bd4fcf
Compare
…#1381) Also moves the configuration of ComponentData's serializer inside ComponentDataImpl
|
Kudos, SonarCloud Quality Gate passed!
|
|
Kudos, SonarCloud Quality Gate passed!
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1382 +/- ##
=========================================
Coverage 87.34% 87.34%
Complexity 2648 2648
=========================================
Files 232 232
Lines 7080 7081 +1
Branches 1073 1073
=========================================
+ Hits 6184 6185 +1
Misses 359 359
Partials 537 537 ☔ View full report in Codecov by Sentry. |
Also moves the configuration of ComponentData's serializer inside ComponentDataImpl