[
https://issues.apache.org/jira/browse/OFBIZ-10351?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16463904#comment-16463904 ]
Taher Alkhateeb commented on OFBIZ-10351:
-----------------------------------------
Hello James,
Thank you for your work again! Okay I reviewed this feature and I have some comments:
* I felt a little nervous with this implementation, it touches core classes like the ControlServlet and ServiceEcaRule at multiple points.
* This intrusion is then wired into a logic that passes plain text to a class that indents text based on the location in the call graph using a ThreadLocal variable. It does not provide any standard data structure format (XML, JSON, or anything else)
* Whether the option "showServiceCallGraph" is turned on or not, the code you introduced to the core classes would always execute, and you would only inhibit the output to logs.
I would much prefer if the call graph is first converted to a model or some data structure, and then data is rendered from that model not manually by an indentation helper class but a proper data structure rendering class / API. We have plenty of things that can be used in OFBiz to that effect. I think I would also prefer not to weave the logic right into the heart of the framework, this should be a boxed logic that is called into when needed, not called always and inhibited.
I hope you take this feedback positively and consider maybe a safer approach. The classes I mentioned above already require a lot of refactoring and cleanup, and more entanglements would make them heavier and more difficult to refactor.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Locked
