dd4t / DD4T.Model

DD4T content model (.NET)
Apache License 2.0
2 stars 12 forks source link

DD4T.Serialization.JSONSerializerService #35

Open Shiva85 opened 6 years ago

Shiva85 commented 6 years ago

https://tridion.stackexchange.com/questions/17961/dxa-1-2-intermittent-issue-in-json-output https://tridion.stackexchange.com/questions/18235/dxa-1-5-intermittent-issue-in-json-output

I am suspecting that the root cause for the issues reported in the above questions are related to the JSONSerializer fix that was put in for DD4T.

The component presentation builder from DD4T ComponentPresentationBuilder is using the following https://github.com/dd4t/DD4T.TridionTemplates/blob/develop/source/DD4T.Templates.Base/Builder/ComponentPresentationBuilder.cs

try { // we cannot be sure the component template uses the same serializer service as the page template // so we will call a factory which can detect the correct service based on the content ISerializerService serializerService = SerializerServiceFactory.FindSerializerServiceForContent(renderedContent); cp = serializerService.Deserialize(renderedContent); }

In the base SerializerServiceFactory, I noticed that we are using the DD4T.Serialization.JSONSerializerService (https://github.com/dd4t/DD4T.Model/blob/2.0.4-beta1/source/DD4T.Serialization/SerializerServiceFactory.cs), which is using the DD4T.Serialization.JS JSONSerializerService and does not having the concurrency issue that is fixed below

private static string defaultSerializerServerType = "DD4T.Serialization.JSONSerializerService"; // Initialization in DD4T Serialization/SerializerServiceFactory

I see a commit transaction here https://github.com/dd4t/DD4T.Model/commit/ff96b151f06f242989449aa9f84ae7408f358db6 // Fixed and commited to DD4T

I would like to get some feedback before pursuing to upgrade the DD4T.Merged manually with the fix indicated above. The fix for the above issues would be to upgrade DD4T corresponding to the version of DXA.

Also, is it recommended that we switch the NewtonSoft references from 6.0.8 to 10.0.1, for the concurrency issue? I did not see an open issue in DD4T.Model and hence opened a new issue. Let me know your feedback!

cajuncoding commented 6 years ago

Thanks @Shiva85, you open issue documented the details of this issue and related content very well! In case it helps, our environment was extremly unstable due to these exact issues. Here are some additional details on how we finally solved these issues for our client's environment.

Overview of the Problems

This issue affects DD4T as a dependency of DXA (across numerous versions from v1.3-v1.8; from what I can tell by Nuget references). And, it affects both the Content Management side as well as the Content Delivery code base, because they both share the same references to the DD4T.Model project (Nuget reference).

This was a significant production issue causing major crashes in our DXA Content Delivery application.

It's truly frustrating that this has existed for so long and for soo many versions of DXA without any proper documentation other than these open issues and Stack Overflow questions/threads. Even just some release note updates in the Markdown of all older versions, and providing a patched DLL for easy fixing would have been extremely helpful to the community. But, as it is, many environments that are using DXA and DD4T versions prior to v2.0 are extremely likely to be suffering from this concurrency/stability problem.

Content Delivery Solution:

  1. Updated to the latest version of Newtonsoft JSON.Net (v10 for us)
  2. Manually patch the application with a replacement version of the DD4T.Serialization.dll -- manually hotfixed and then replaced in the web application as an updated dpendency. We maintained the Nuget reference to DD4T.Model, but manually updated the project references specifically for DD4T.Serialization.dll to point to a new hotfixed assembly.

Only after replacing it with the code fix (noted above, located here) did the application stabilize when de-serializing JSON. This hotfix wasn't available in an official version of DD4T until v2.2.2, which is only used by DXA v2.0 (based on Nuget references).

Content Management Problem & Solution:

At the same time we were experiencing seemingly random and intermittent issues on the Content Management side whereby the DD4T template building blocks were not serializing the JSON models correctly. We were experiencing the exact issue noted above and located here on StackOverflow), but we are running in DXA v1.6 and not v1.5 as noted in StackOverflow -- this is because the issue still existed in most versions of DXA!

To summarize, the JSON serialization was failing and incorrectly inserting the RenderedContent node into the JSON output (for debugging purposes). BUT, instead of failing publish with helpful error messages, the JSON was frustratingly still rendered & published yielding a false positive which then resulted in failure to properly render on the Content Delivery side.

Since both the Content Management & Content Delivery share the same dependencies the root cause ended up being this same issue on both sides: 1) update Newtonsoft JSON.Net reference to the latest version (v10 for us), and 2) manually replace the DD4T.Serialization assembly with a patched version.

To fix the content management template building blocks, I copied over the same DD4T.Serialization-HOTFIX.dll file that was used to patch the Content Delivery side, into the Template Building Block project, and updated the project references to point to the new hotfixed assembly. The ILMerge build-command, of the template building blocks project (which merges all dependencies into one single DLL to be uploaded into Tridion), would then merge in the patched assembly and finally we were able to stabilize the system from publish through to render!

HOTFIX:

In case it will help anyone else, I've also forked the DD4T.Model v2.0.7 and applied the patch -- this version will work with DXA v1.5, 1.6, (and likely other versions as well), etc.... that are based on DD4T v2.0.7. My fork has the concurrency issue patched, and the DD4T.Serialization-CONCURRENCY_HOTFIX.dll assembly can be compiled and ready to be used.

Forked DD4T.Model with HOTFIX Branch can be found here: https://github.com/cajuncoding/DD4T.Model/tree/Branch_2.0.7-CONCURRENCY_HOTFIX

I hope this helps others that stumble upon this issue, or other cross linked locations about this issue!

-Brandon Bernard

quirijnslings commented 4 years ago

Sorry to be so late to the game, but am I correct in assuming that the issues is solved in the latest DD4T version?