Jasig/uPortal — pull request #256 by apetro (2014-03-04 22:19:53+00:00)
Title: DLM logging enhancements for master

Adds vastly more `TRACE` and `DEBUG` logging to DLM without changing behavior.

This is https://github.com/Jasig/uPortal/pull/255 except applied to `master` instead of to the favorites feature branch.

## Features

* No functionality change. Only introduction of logging. Logic is teased apart in parts to facilitate logging branches of the decision tree, but logic and behavior are unchanged, except for `XmlUtilitiesImpl.toString( Node )` becomes null-safe.
* Picks up some `toString()` implementations along the way (new in `FragmentDefinition`, `FragmentNodeInfo`, `IntegrationResult`, `LPAChangeAttribute`, `LPAEditRestriction`, `Paren`, and `PortletSubscribeIdResolver` ; enhanced in `LocaleManager`.)
* Switches to `slf4j` in some classes to pick up the message formatting.
* Uses `XmlUtilitiesImpl.toString( Node )` to log XML elements as Strings. Gates logging invocations at `TRACE` and `DEBUG` level (as in, `if ( logger.isDebugEnabled() ) {` ) to avoid needless expensive XML transformation/serialization to generate Strings that won't actually be logged. Enhances `XmlUtilitiesImpl.toString( Node )` to be null-safe, that is, to return `"null"` on a null argument.
* The weird `DistributedLayoutManager` prototype bean declared in `layoutContext.xml` now has a comment documenting its weirdness.

## Known shortcomings, technical debt, and deferred opportunities

* There are places where XML nodes are logged without converting them to Strings. This results in much less useful log messages than if these spots were enhanced to use `XmlUtilitiesImpl.toString( Node )`. Perhaps these spots will be caught and enhanced as they annoy. It's just laziness that these aren't all caught and fixed for this pull request. Perhaps these should be cleaned up in this pull request review process before merge.
* Does not add documentation on how to configure `logback.xml` to take good advantage of the logging. Indiscriminate `TRACE` logging is quite chatty and overwhelming.
* Rather more care could be added to use various log levels and to use them consistently. What's here is a translating-to-`slf4j` of the existing logging and adding of mostly `TRACE` level statements as opposed to a more holistic refresh of DLM logging.
* No profiling or performance testing has been done to ensure that this changeset doesn't significantly reduce performance. In principle it shouldn't, but that hasn't been empirically verified.
* Besides this routine-logging-of-Java-classes approach to logging, perhaps DLM needs a slightly more formal layout event audit log akin to audit log for impersonation and such. It might be very helpful to be easily capture a log of layout change events.