Tiles
  1. Tiles
  2. TILES-538

Pluggable debugging around rendering

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0
    • Component/s: tiles-api
    • Labels:
      None
    • Flags:
      Patch

      Description

      At finn.no we're using a custom TemplateAttributeRenderer (DispatchRenderer in tiles-3).

      One of the customisation we have is customised debugging around each template rendering. For us this customised debugging does:

      • print "<!-- start insertAttribute /template-name/ -->" before rendering
      • print "<!-- end insertAttribute /template-name/ /render-time-ms/ -->" after rendering
      • maintains a tree of templates rendered which is printed out as a comment at the bottom of the html, eg
        /WEB-INF/tiles/template/main_template.jsp (20ms)
        /WEB-INF/tiles/fragment/header.jsp (2ms)
        /WEB-INF/tiles/fragment/body.jsp (16ms)
        /WEB-INF/tiles/fragment/left_menu.jsp (6ms)
        /WEB-INF/tiles/fragment/content.jsp (10ms)
        /WEB-INF/tiles/fragment/footer.jsp (2ms)

      We find this useful for finding relevant jsps quickly, and for finding performance problems within the presentation layer in development and test environments. And I've seen something similar in other websites.

      Would it make sense to introduce such an api into DispatchRenderer so to make it possible to plug such debugging in rather than having to override the class. Simply overriding the class is not so simple when using the OptionsDispatchRenderer proposed in TILES-539.
      I'm not entirely sure myself so throwing this out there. One problem i can see if that this only works for the DispatchRenderer, although this is intended to be taken advantage of TilesContainerFactory.createTemplateAttributeRenderer(..) where it is specified as the default renderer.

      public class DispatchRenderer implements TypeDetectingRenderer {
      
          public interface DebugWrapper{
              DebugWrapper start(String template, Request request) throws IOException;
              DebugWrapper end();
              void handleIOException(IOException ex, Request request) throws IOException;
          }
      
          private final DebugWrapper debugwrapper;
      
          public DispatchRenderer(){
              this.debugwrapper = new DummyDebugWrapper();
          }
      
          public DispatchRenderer(DebugWrapper debugWrapper){
              this.debugwrapper = debugWrapper;
          }
      
          /** {@inheritDoc} */
          @Override
          public void render(String path, Request request) throws IOException {
              if (path == null) {
                  throw new CannotRenderException("Cannot dispatch a null path");
              }
              try{
                  debugwrapper.start(path, request);
                  request.dispatch(path);
              }catch(IOException ex){
                  debugwrapper.handleIOException(ex, request);
              }finally{
                  debugwrapper.end();
              }
          }
      
          /** {@inheritDoc} */
          public boolean isRenderable(String path, Request request) {
              return path != null && path.startsWith("/");
          }
      
          private static final class DummyDebugWrapper implements DebugWrapper{
              @Override
              public DebugWrapper start(final String template, final Request request) throws IOException {
                  return this;
              }
              @Override
              public DebugWrapper end() {
                  return this;
              }
              @Override
              public void handleIOException(IOException ex, Request request) throws IOException {
                  throw ex;
              }
          }
      }
      

      The handleIOException(..) also allows behaviour to be configurable against environment. For development and test we re-throw the IOException, but in production we ignore it and write a warning as a html comment into the response. This makes the "ignore" attribute to the template more of an assert rather than forced.

        Activity

        mck created issue -
        mck made changes -
        Field Original Value New Value
        Attachment TILES-538.patch [ 12499819 ]
        mck made changes -
        Component/s tiles-api [ 12313340 ]
        Component/s tiles-core [ 12313339 ]
        mck made changes -
        Description At finn.no we're using a custom TemplateAttributeRenderer (DispatchRenderer in tiles-3).

        One of the customisation we have is customised debugging around each template rendering. For us this customised debugging does:
         - print "<!-- start insertAttribute /template-name/ -->" before rendering
         - print "<!-- end insertAttribute /template-name/ /render-time-ms/ -->" after rendering
         - maintains a tree of templates rendered which is printed out as a comment at the bottom of the html, eg
              /WEB-INF/tiles/template/main_template.jsp (20ms)
                  /WEB-INF/tiles/fragment/header.jsp (2ms)
                  /WEB-INF/tiles/fragment/body.jsp (16ms)
                      /WEB-INF/tiles/fragment/left_menu.jsp (6ms)
                      /WEB-INF/tiles/fragment/content.jsp (10ms)
                  /WEB-INF/tiles/fragment/footer.jsp (2ms)

        We find this useful for finding relevant jsps quickly, and for finding performance problems within the presentation layer in development and test environments. And I've seen something similar in other websites.

        Would it make sense to introduce such an api into DispatchRenderer so to make it possible to plug such debugging in rather than having to override the class.
        I'm not entirely sure myself so throwing this out there. One problem i can see if that this only works for the DispatchRenderer, although this is intended to be taken advantage of TilesContainerFactory.createTemplateAttributeRenderer(..) where it is specified as the default renderer.

        {code}
        public class DispatchRenderer implements TypeDetectingRenderer {

            public interface DebugWrapper{
                DebugWrapper start(String template, Request request) throws IOException;
                DebugWrapper end();
                void handleIOException(IOException ex, Request request) throws IOException;
            }

            private final DebugWrapper debugwrapper;

            public DispatchRenderer(){
                this.debugwrapper = new DummyDebugWrapper();
            }

            public DispatchRenderer(DebugWrapper debugWrapper){
                this.debugwrapper = debugWrapper;
            }

            /** {@inheritDoc} */
            @Override
            public void render(String path, Request request) throws IOException {
                if (path == null) {
                    throw new CannotRenderException("Cannot dispatch a null path");
                }
                try{
                    debugwrapper.start(path, request);
                    request.dispatch(path);
                }catch(IOException ex){
                    debugwrapper.handleIOException(ex, request);
                }finally{
                    debugwrapper.end();
                }
            }

            /** {@inheritDoc} */
            public boolean isRenderable(String path, Request request) {
                return path != null && path.startsWith("/");
            }

            private static final class DummyDebugWrapper implements DebugWrapper{
                @Override
                public DebugWrapper start(final String template, final Request request) throws IOException {
                    return this;
                }
                @Override
                public DebugWrapper end() {
                    return this;
                }
                @Override
                public void handleIOException(IOException ex, Request request) throws IOException {
                    throw ex;
                }
            }
        }
        {code}

        The handleIOException(..) also allows behaviour to be configurable against environment. For development and test we re-throw the IOException, but in production we ignore it and write a warning as a html comment into the response. This makes the "ignore" attribute to the template more of an assert rather than forced.
        At finn.no we're using a custom TemplateAttributeRenderer (DispatchRenderer in tiles-3).

        One of the customisation we have is customised debugging around each template rendering. For us this customised debugging does:
         - print "<!-- start insertAttribute /template-name/ -->" before rendering
         - print "<!-- end insertAttribute /template-name/ /render-time-ms/ -->" after rendering
         - maintains a tree of templates rendered which is printed out as a comment at the bottom of the html, eg
              /WEB-INF/tiles/template/main_template.jsp (20ms)
                  /WEB-INF/tiles/fragment/header.jsp (2ms)
                  /WEB-INF/tiles/fragment/body.jsp (16ms)
                      /WEB-INF/tiles/fragment/left_menu.jsp (6ms)
                      /WEB-INF/tiles/fragment/content.jsp (10ms)
                  /WEB-INF/tiles/fragment/footer.jsp (2ms)

        We find this useful for finding relevant jsps quickly, and for finding performance problems within the presentation layer in development and test environments. And I've seen something similar in other websites.

        Would it make sense to introduce such an api into DispatchRenderer so to make it possible to plug such debugging in rather than having to override the class. Simply overriding the class is not so simple when using the OptionsDispatchRenderer proposed in TILES-539.
        I'm not entirely sure myself so throwing this out there. One problem i can see if that this only works for the DispatchRenderer, although this is intended to be taken advantage of TilesContainerFactory.createTemplateAttributeRenderer(..) where it is specified as the default renderer.

        {code}
        public class DispatchRenderer implements TypeDetectingRenderer {

            public interface DebugWrapper{
                DebugWrapper start(String template, Request request) throws IOException;
                DebugWrapper end();
                void handleIOException(IOException ex, Request request) throws IOException;
            }

            private final DebugWrapper debugwrapper;

            public DispatchRenderer(){
                this.debugwrapper = new DummyDebugWrapper();
            }

            public DispatchRenderer(DebugWrapper debugWrapper){
                this.debugwrapper = debugWrapper;
            }

            /** {@inheritDoc} */
            @Override
            public void render(String path, Request request) throws IOException {
                if (path == null) {
                    throw new CannotRenderException("Cannot dispatch a null path");
                }
                try{
                    debugwrapper.start(path, request);
                    request.dispatch(path);
                }catch(IOException ex){
                    debugwrapper.handleIOException(ex, request);
                }finally{
                    debugwrapper.end();
                }
            }

            /** {@inheritDoc} */
            public boolean isRenderable(String path, Request request) {
                return path != null && path.startsWith("/");
            }

            private static final class DummyDebugWrapper implements DebugWrapper{
                @Override
                public DebugWrapper start(final String template, final Request request) throws IOException {
                    return this;
                }
                @Override
                public DebugWrapper end() {
                    return this;
                }
                @Override
                public void handleIOException(IOException ex, Request request) throws IOException {
                    throw ex;
                }
            }
        }
        {code}

        The handleIOException(..) also allows behaviour to be configurable against environment. For development and test we re-throw the IOException, but in production we ignore it and write a warning as a html comment into the response. This makes the "ignore" attribute to the template more of an assert rather than forced.
        Hide
        mck added a comment -

        New patch that:

        • uses a pub-sub implementation,
        • uses delegation to any TypeDetectingRenderer.

        I'm far happier with this solution. The pub-sub pattern, suggested by Antonio, allows multiple subscribers (not just one debugging use-case), and a delegation model is a lot more friendly than simply hardcoding against by extending DispatchRenderer.

        Show
        mck added a comment - New patch that: uses a pub-sub implementation, uses delegation to any TypeDetectingRenderer. I'm far happier with this solution. The pub-sub pattern, suggested by Antonio, allows multiple subscribers (not just one debugging use-case), and a delegation model is a lot more friendly than simply hardcoding against by extending DispatchRenderer.
        mck made changes -
        Attachment TILES-538.patch [ 12501447 ]
        mck made changes -
        Attachment TILES-538.patch [ 12499819 ]
        mck made changes -
        Attachment TILES-538.patch [ 12501506 ]
        mck made changes -
        Attachment TILES-538.patch [ 12501447 ]
        mck made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        mck added a comment -

        committed

        Show
        mck added a comment - committed
        mck made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Nicolas Le Bas made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        10d 5h 24m 1 mck 30/Oct/11 12:42
        In Progress In Progress Resolved Resolved
        8d 23h 5m 1 mck 08/Nov/11 11:47
        Resolved Resolved Closed Closed
        215d 10h 49m 1 Nicolas Le Bas 10/Jun/12 23:36

          People

          • Assignee:
            mck
            Reporter:
            mck
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development