Uploaded image for project: 'Struts 2'
  1. Struts 2
  2. WW-4749

Buffer/Flush behaviour in FreemarkerResult

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.31, 2.5.1
    • Fix Version/s: 2.5.12
    • Component/s: Core Results
    • Labels:
      None

      Description

      Scenario: the application use freemarker with a TemplateExceptionHandler.RETHROW_HANDLER policy, but occasionally needs to produce large XML (20~200Mb) and goes out of memory.

      In FreemarkerResult there are two possible behaviours (line 191):

      • Buffer-behaviour: the whole template is processed and if everything is OK it is flushed to the output, otherwise an exception is thrown and handled at higher level before any output has been sent. This is intended to be used when TemplateExceptionHandler.RETHROW_HANDLER is active
      • Flush-behaviour: template is processed and flushed according to freemarker library policies, used with any other TemplateExceptionHandler

      Since TemplateExceptionHandler cannot be switched for a given request (it is a global configuration embedded in FreemarkerManager) there is no way to force a Flush-behaviour. (you can only force a Buffer-behaviour using isWriteIfCompleted)

      I implemented a more flexible solution that let you force the behaviour in both ways:

      FreemarkerResult.java
          final boolean willUsebufferedWriter;
          if (useBufferedWriter != null){
              willUsebufferedWriter = useBufferedWriter;
          }else{
              willUsebufferedWriter = configuration.getTemplateExceptionHandler() == TemplateExceptionHandler.RETHROW_HANDLER;
          }
                      
          if (willUsebufferedWriter){
          ...
          }else{
          ...
          }       
      

      where useBufferedWriter is a parameter that can be modified per request

      <result type="freemarker">
          <param name="location">big_feed.ftl</param>
          <param name="contentType">text/xml</param>
          <param name="useBufferedWriter">false</param>
      </result>
      

        Issue Links

          Activity

          Show
          fustaki Lorenzo Bernacchioni added a comment - Link to stackoverflow question: http://stackoverflow.com/questions/42504866/generating-big-files-with-struts2-freemarker/42509355
          Hide
          ddekany Daniel Dekany added a comment -

          And while we are here, `configuration.getTemplateExceptionHandler()` should be `template.getTemplateExceptionHandler()`. Otherwise per-template settings (http://freemarker.org/docs/pgui_config_templateconfigurations.html) won't have effect.

          Show
          ddekany Daniel Dekany added a comment - And while we are here, `configuration.getTemplateExceptionHandler()` should be `template.getTemplateExceptionHandler()`. Otherwise per-template settings ( http://freemarker.org/docs/pgui_config_templateconfigurations.html ) won't have effect.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user lukaszlenart opened a pull request:

          https://github.com/apache/struts/pull/134

          WW-4749: Buffer/Flush behaviour in FreemarkerResult

          Implements WW-4749(https://issues.apache.org/jira/browse/WW-4749)

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/lukaszlenart/struts buffer-flush

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/struts/pull/134.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #134


          commit d739be34bccebbde2972c9e2c6225b94bad68d27
          Author: Lukasz Lenart <lukaszlenart@apache.org>
          Date: 2017-04-26T06:42:42Z

          WW-4749 Implements buffered write

          commit 787150d041e32e8e579df5d44c1258881fad6c94
          Author: Lukasz Lenart <lukaszlenart@apache.org>
          Date: 2017-04-26T06:48:01Z

          Moves documentation to wiki

          commit f5125bcd1bb97a75ab54e266399c2bf96987c1dc
          Author: Lukasz Lenart <lukaszlenart@apache.org>
          Date: 2017-04-26T06:51:45Z

          WW-4749 Defines setter to allow specify useBufferedWriter


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user lukaszlenart opened a pull request: https://github.com/apache/struts/pull/134 WW-4749 : Buffer/Flush behaviour in FreemarkerResult Implements WW-4749 ( https://issues.apache.org/jira/browse/WW-4749 ) You can merge this pull request into a Git repository by running: $ git pull https://github.com/lukaszlenart/struts buffer-flush Alternatively you can review and apply these changes as the patch at: https://github.com/apache/struts/pull/134.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #134 commit d739be34bccebbde2972c9e2c6225b94bad68d27 Author: Lukasz Lenart <lukaszlenart@apache.org> Date: 2017-04-26T06:42:42Z WW-4749 Implements buffered write commit 787150d041e32e8e579df5d44c1258881fad6c94 Author: Lukasz Lenart <lukaszlenart@apache.org> Date: 2017-04-26T06:48:01Z Moves documentation to wiki commit f5125bcd1bb97a75ab54e266399c2bf96987c1dc Author: Lukasz Lenart <lukaszlenart@apache.org> Date: 2017-04-26T06:51:45Z WW-4749 Defines setter to allow specify useBufferedWriter
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Lorenzo Bernacchioni and Daniel Dekany can you review the PR?

          Show
          lukaszlenart Lukasz Lenart added a comment - Lorenzo Bernacchioni and Daniel Dekany can you review the PR?
          Hide
          fustaki Lorenzo Bernacchioni added a comment -

          two marginal notes:

          • parameter writeIfCompleted is no longer used and could be removed
          • for sake of readability I would leave useBufferedWriter as a Boolean and eventually if a String input is more compliant parse the String in the setter method
          	public void setUseBufferedWriter(String useBufferedWriter) {
          		this.useBufferedWriter = Boolean.parseBoolean(useBufferedWriter);
          	}
          
          Show
          fustaki Lorenzo Bernacchioni added a comment - two marginal notes: parameter writeIfCompleted is no longer used and could be removed for sake of readability I would leave useBufferedWriter as a Boolean and eventually if a String input is more compliant parse the String in the setter method public void setUseBufferedWriter( String useBufferedWriter) { this .useBufferedWriter = Boolean .parseBoolean(useBufferedWriter); }
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Done, can you double-check?

          Show
          lukaszlenart Lukasz Lenart added a comment - Done, can you double-check?
          Hide
          fustaki Lorenzo Bernacchioni added a comment - - edited

          Uhm.. the change from Boolean to boolean leads to

          final boolean willUseBufferedWriter = isUseBufferedWriter() || template.getTemplateExceptionHandler() == TemplateExceptionHandler.RETHROW_HANDLER;
          

          So we fall again in the issue of my scenario: if I'm adopting a TemplateExceptionHandler.RETHROW_HANDLER policy I can't prevent the use bufferedWriter, because the line above is always true.

          The use of Boolean let you force the behaviour in both ways. Leave it as null to let the behaviour depend on the TemplateExceptionHandler policy.

          private Boolean useBufferedWriter = null;
          [...]
          final boolean willUseBufferedWriter;
          if (useBufferedWriter != null) {
              willUseBufferedWriter = useBufferedWriter;
          } else {
              willUseBufferedWriter = template.getTemplateExceptionHandler() == TemplateExceptionHandler.RETHROW_HANDLER;
          }
          
          Show
          fustaki Lorenzo Bernacchioni added a comment - - edited Uhm.. the change from Boolean to boolean leads to final boolean willUseBufferedWriter = isUseBufferedWriter() || template.getTemplateExceptionHandler() == TemplateExceptionHandler.RETHROW_HANDLER; So we fall again in the issue of my scenario: if I'm adopting a TemplateExceptionHandler.RETHROW_HANDLER policy I can't prevent the use bufferedWriter , because the line above is always true . The use of Boolean let you force the behaviour in both ways. Leave it as null to let the behaviour depend on the TemplateExceptionHandler policy. private Boolean useBufferedWriter = null ; [...] final boolean willUseBufferedWriter; if (useBufferedWriter != null ) { willUseBufferedWriter = useBufferedWriter; } else { willUseBufferedWriter = template.getTemplateExceptionHandler() == TemplateExceptionHandler.RETHROW_HANDLER; }
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Ach... pushed a new version Next time you will be preparing a PR

          Show
          lukaszlenart Lukasz Lenart added a comment - Ach... pushed a new version Next time you will be preparing a PR
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          One more thing: can we stick with writeIfCompleted flag? won't be its meaning the same?

          Show
          lukaszlenart Lukasz Lenart added a comment - One more thing: can we stick with writeIfCompleted flag? won't be its meaning the same?
          Hide
          fustaki Lorenzo Bernacchioni added a comment -

          Yes, agree with th use of writeIfCompleted for retro-compatibility

          Show
          fustaki Lorenzo Bernacchioni added a comment - Yes, agree with th use of writeIfCompleted for retro-compatibility
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Lorenzo Bernacchioni can you double check the PR?

          Show
          lukaszlenart Lukasz Lenart added a comment - Lorenzo Bernacchioni can you double check the PR?
          Hide
          fustaki Lorenzo Bernacchioni added a comment -

          Left this comment in Github

          Now that useBufferedWriter has been changed with writeIfCompleted for backward compatibility, just for the sake of code readability, I would change the name of the local variable willUseBufferedWriter with willWriteIfCompleted

          Show
          fustaki Lorenzo Bernacchioni added a comment - Left this comment in Github Now that useBufferedWriter has been changed with writeIfCompleted for backward compatibility, just for the sake of code readability, I would change the name of the local variable willUseBufferedWriter with willWriteIfCompleted
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

          https://github.com/apache/struts/pull/134

          @fustaki done, are we ok?

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/134 @fustaki done, are we ok?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fustaki commented on the issue:

          https://github.com/apache/struts/pull/134

          Great, OK for me

          Show
          githubbot ASF GitHub Bot added a comment - Github user fustaki commented on the issue: https://github.com/apache/struts/pull/134 Great, OK for me
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit d739be34bccebbde2972c9e2c6225b94bad68d27 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=d739be3 ]

          WW-4749 Implements buffered write

          Show
          jira-bot ASF subversion and git services added a comment - Commit d739be34bccebbde2972c9e2c6225b94bad68d27 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=d739be3 ] WW-4749 Implements buffered write
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit f5125bcd1bb97a75ab54e266399c2bf96987c1dc in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=f5125bc ]

          WW-4749 Defines setter to allow specify useBufferedWriter

          Show
          jira-bot ASF subversion and git services added a comment - Commit f5125bcd1bb97a75ab54e266399c2bf96987c1dc in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=f5125bc ] WW-4749 Defines setter to allow specify useBufferedWriter
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1df89792f93fa71f374eef8a2b1a72b43cc28eab in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=1df8979 ]

          WW-4749 Drops writeIfCompleted and uses Boolean instead String

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1df89792f93fa71f374eef8a2b1a72b43cc28eab in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=1df8979 ] WW-4749 Drops writeIfCompleted and uses Boolean instead String
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit bbcd9dc2514b286065a16657d58418aa9ef4be75 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=bbcd9dc ]

          WW-4749 Uses ordinary boolean instead of Boolean

          Show
          jira-bot ASF subversion and git services added a comment - Commit bbcd9dc2514b286065a16657d58418aa9ef4be75 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=bbcd9dc ] WW-4749 Uses ordinary boolean instead of Boolean
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 6dad53c5e316b3be2551a1bab47ee71b64ac3542 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=6dad53c ]

          WW-4749 Fixes test

          Show
          jira-bot ASF subversion and git services added a comment - Commit 6dad53c5e316b3be2551a1bab47ee71b64ac3542 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=6dad53c ] WW-4749 Fixes test
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          PR got merged

          Show
          lukaszlenart Lukasz Lenart added a comment - PR got merged
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/struts/pull/134

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/struts/pull/134
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 3034b3a97050bcb24e34f39b203a960a0ddca93e in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=3034b3a ]

          WW-4749 Uses Boolean to allow define behaviour per result

          Show
          jira-bot ASF subversion and git services added a comment - Commit 3034b3a97050bcb24e34f39b203a960a0ddca93e in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=3034b3a ] WW-4749 Uses Boolean to allow define behaviour per result
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 38a2ebb72abd90c74bd52d6a81e3a0d967965c7b in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=38a2ebb ]

          WW-4749 Uses writeIfCompleted to keep backward compatibility

          Show
          jira-bot ASF subversion and git services added a comment - Commit 38a2ebb72abd90c74bd52d6a81e3a0d967965c7b in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=38a2ebb ] WW-4749 Uses writeIfCompleted to keep backward compatibility
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit b842a4c779e45e1193e6319970c2dc6f52ec6bf1 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=b842a4c ]

          WW-4749 Renames local variable to better express its meaning

          Show
          jira-bot ASF subversion and git services added a comment - Commit b842a4c779e45e1193e6319970c2dc6f52ec6bf1 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=b842a4c ] WW-4749 Renames local variable to better express its meaning
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 5a0f2e1aaf8d420bd74033175e6e459883160487 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=5a0f2e1 ]

          WW-4749 Implements buffer/flush behaviour in FreemarkerResult

          Show
          jira-bot ASF subversion and git services added a comment - Commit 5a0f2e1aaf8d420bd74033175e6e459883160487 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=5a0f2e1 ] WW-4749 Implements buffer/flush behaviour in FreemarkerResult
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Struts-JDK7-master #627 (See https://builds.apache.org/job/Struts-JDK7-master/627/)
          WW-4749 Implements buffered write (lukaszlenart: rev d739be34bccebbde2972c9e2c6225b94bad68d27)

          • (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java
            WW-4749 Defines setter to allow specify useBufferedWriter (lukaszlenart: rev f5125bcd1bb97a75ab54e266399c2bf96987c1dc)
          • (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java
            WW-4749 Drops writeIfCompleted and uses Boolean instead String (lukaszlenart: rev 1df89792f93fa71f374eef8a2b1a72b43cc28eab)
          • (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java
            WW-4749 Uses ordinary boolean instead of Boolean (lukaszlenart: rev bbcd9dc2514b286065a16657d58418aa9ef4be75)
          • (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java
            WW-4749 Fixes test (lukaszlenart: rev 6dad53c5e316b3be2551a1bab47ee71b64ac3542)
          • (edit) core/src/test/java/org/apache/struts2/views/freemarker/FreeMarkerResultTest.java
            WW-4749 Uses Boolean to allow define behaviour per result (lukaszlenart: rev 3034b3a97050bcb24e34f39b203a960a0ddca93e)
          • (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java
            WW-4749 Uses writeIfCompleted to keep backward compatibility (lukaszlenart: rev 38a2ebb72abd90c74bd52d6a81e3a0d967965c7b)
          • (edit) core/src/test/java/org/apache/struts2/views/freemarker/FreeMarkerResultTest.java
          • (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java
            WW-4749 Renames local variable to better express its meaning (lukaszlenart: rev b842a4c779e45e1193e6319970c2dc6f52ec6bf1)
          • (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Struts-JDK7-master #627 (See https://builds.apache.org/job/Struts-JDK7-master/627/ ) WW-4749 Implements buffered write (lukaszlenart: rev d739be34bccebbde2972c9e2c6225b94bad68d27) (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java WW-4749 Defines setter to allow specify useBufferedWriter (lukaszlenart: rev f5125bcd1bb97a75ab54e266399c2bf96987c1dc) (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java WW-4749 Drops writeIfCompleted and uses Boolean instead String (lukaszlenart: rev 1df89792f93fa71f374eef8a2b1a72b43cc28eab) (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java WW-4749 Uses ordinary boolean instead of Boolean (lukaszlenart: rev bbcd9dc2514b286065a16657d58418aa9ef4be75) (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java WW-4749 Fixes test (lukaszlenart: rev 6dad53c5e316b3be2551a1bab47ee71b64ac3542) (edit) core/src/test/java/org/apache/struts2/views/freemarker/FreeMarkerResultTest.java WW-4749 Uses Boolean to allow define behaviour per result (lukaszlenart: rev 3034b3a97050bcb24e34f39b203a960a0ddca93e) (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java WW-4749 Uses writeIfCompleted to keep backward compatibility (lukaszlenart: rev 38a2ebb72abd90c74bd52d6a81e3a0d967965c7b) (edit) core/src/test/java/org/apache/struts2/views/freemarker/FreeMarkerResultTest.java (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java WW-4749 Renames local variable to better express its meaning (lukaszlenart: rev b842a4c779e45e1193e6319970c2dc6f52ec6bf1) (edit) core/src/main/java/org/apache/struts2/views/freemarker/FreemarkerResult.java

            People

            • Assignee:
              lukaszlenart Lukasz Lenart
              Reporter:
              fustaki Lorenzo Bernacchioni
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development