servlet-spec
  1. servlet-spec
  2. SERVLET_SPEC-66

Need way to track progress of requests; proposal included

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Listeners
    • Labels:
      None
    • Environment:

      n/a

      Description

      Servlet 3.0 added multipart request processing to, in part, make handling file uploads easier (and easier it did make it). Before 3.0, many users used Commons FileUpload to accomplish this task. However, 3.0's multipart processing did not, unfortunately, completely eliminate the need for FileUpload. One of the major features lacking is the ability to track the progress of a large request.

      This feature is sometimes called "file upload progress," but that name is misleading. It's actually "request progress," and it's the ability to measure and periodically report the number of bytes actually received versus the number of bytes indicated in the "Content-Length" header. As I propose below, I believe this should be relatively easy to add to the servlet spec, relatively easy to implement, and quite easy to use.

      As proposed, this is independent of protocol (not strictly tied to HTTP/multipart/Content-Length). That could be changed, but I think this makes sense.

      First, create a new interface:

      ServletRequestProgressListener
      package javax.servlet;
      
      public interface ServletRequestProgressListener
      {
          /**
           * Called whenever the number of bytes read changes, at least every 64 kilobytes.
           *
           * @param bytesRead The number of bytes that have been read so far, at least 0
           * @param bytesExpected The number of bytes expected to be read, -1 if unknown
           * @param itemsRead The number of items (parts in an HTTP multipart) processed so far
           */
          void update(long bytesRead, long bytesExpected, int itemsRead);
      
          /**
           * Called whenever the request has ended, either by being canceled or completed, 
           * normally or abnormally.
           */
          void destroy();
      }
      

      Next, add a method to ServletRequest:

      ServletRequest
      ...
          /**
           * Attaches a progress listener to this request. Progress listeners must be attached in
           * a filter, before the request gets to the Servlet, in order to be effective.
           * 
           * @param progressListener The progress listener to update when the bytes read increases
           * @throws UnsupportedOperationException if the protocol does not support progress listeners
           */
          void setProgressListener(ServletRequestProgressListener progressListener);
      ...
      

      Because the listener can be a source of performance problems, containers would only be required to call update (1) when first attached, and (2) every 64 kilobytes. Containers may call it more often, but do not have to. As proposed, I estimate 30 minutes to create the proposed interfaces and 1.5 hours to update the servlet specification documentation. Should only take 2-3 hours to add to the Tomcat implementation based on my examination of the code. Can't speak for the other implementations.

      Using multipart as the primary example, since multipart processing is completed before the Servlet gets the request, the listener would have to be attached in a filter. A typical use case would be to create a listener and add it to a session so that it can later be queried by some Ajax call:

      Psuedo-Code
      ...
          public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
          {
              if(request-is-large)
              {
                  MyProgressListener listener = new MyProgressListener(request);
                  request.getSession().addAttribute("progressListener", listener);
                  request.setProgressListener(listener);
              }
          }
      ...
      

        Activity

        Hide
        Nick Williams added a comment - - edited

        One unfortunate problem with NOT adopting this suggestion (in some form or another) is that here is no work-around that works within the spec. Either you don't have progress bars, or you don't use the multipart support added in 3.0. There's no "hard way" in the spec that this makes easier. This is making something possible that is currently impossible within the spec.

        Show
        Nick Williams added a comment - - edited One unfortunate problem with NOT adopting this suggestion (in some form or another) is that here is no work-around that works within the spec. Either you don't have progress bars, or you don't use the multipart support added in 3.0. There's no "hard way" in the spec that this makes easier. This is making something possible that is currently impossible within the spec.
        Hide
        Eugene Chung added a comment -

        +1

        But all listeners of servlet are added via configuration(at deploy time). I think your suggestion, which is adding a listener at service time, can break consistency.

        Show
        Eugene Chung added a comment - +1 But all listeners of servlet are added via configuration(at deploy time). I think your suggestion, which is adding a listener at service time, can break consistency.
        Hide
        Nick Williams added a comment -

        I thought about that, and my original draft had this set up as a configuration item, but there's a problem with that. All of the other listeners are configured across the servlet context. A session listener listens for new sessions or changes to sessions; a request listener gets notified of request events; a servlet context listener observes when the context starts and stops.

        But this listener is very different. In fact, it is nothing like the other listeners at all. One of these listeners would be created for exactly one request. When that request was over, the listener would be destroyed and would no longer have any purpose. The update method makes it apparent why this is the case.

        Perhaps, to make it clear that this is not a listener in the Servlet sense, it should have a different name, such as ServletRequestProgressFollower and setProgressFollower, but "listener," in a generic sense, really does describe what it's for.

        (Also: should there be a getProgressListener/getProgressFollower method on ServletRequest in addition to the mutator?)

        Show
        Nick Williams added a comment - I thought about that, and my original draft had this set up as a configuration item, but there's a problem with that. All of the other listeners are configured across the servlet context. A session listener listens for new sessions or changes to sessions; a request listener gets notified of request events; a servlet context listener observes when the context starts and stops. But this listener is very different. In fact, it is nothing like the other listeners at all. One of these listeners would be created for exactly one request. When that request was over, the listener would be destroyed and would no longer have any purpose. The update method makes it apparent why this is the case. Perhaps, to make it clear that this is not a listener in the Servlet sense, it should have a different name, such as ServletRequestProgressFollower and setProgressFollower , but "listener," in a generic sense, really does describe what it's for. (Also: should there be a getProgressListener / getProgressFollower method on ServletRequest in addition to the mutator?)
        Hide
        Nick Williams added a comment -

        Actually, now that I think about it, maybe ServletRequestProgressFollower would be a better name...

        Show
        Nick Williams added a comment - Actually, now that I think about it, maybe ServletRequestProgressFollower would be a better name...
        Hide
        Nick Williams added a comment -

        I know this suggestion is pushing the line for Servlet 3.1, but I think it's a big improvement worth consideration. If there's anything I can do to help, I'm willing to drop whatever I'm working on to make it happen. I'm hoping there's time for just one more thing for Servlet 3.1.

        If it comes to choosing between this and issue #67, 67 can definitely wait until 3.next, because it has a workaround. This issue, unfortunately, does not have a workaround.

        Show
        Nick Williams added a comment - I know this suggestion is pushing the line for Servlet 3.1, but I think it's a big improvement worth consideration. If there's anything I can do to help, I'm willing to drop whatever I'm working on to make it happen. I'm hoping there's time for just one more thing for Servlet 3.1. If it comes to choosing between this and issue #67, 67 can definitely wait until 3.next, because it has a workaround. This issue, unfortunately, does not have a workaround .
        Hide
        Nick Williams added a comment -

        I'd love to get some feedback on this. I haven't seen any discussion about it on the EG mailing list. Does the proposal need changes? Is it too late to add? That would be unfortunate, since there is no workaround.

        Show
        Nick Williams added a comment - I'd love to get some feedback on this. I haven't seen any discussion about it on the EG mailing list. Does the proposal need changes? Is it too late to add? That would be unfortunate, since there is no workaround.

          People

          • Assignee:
            Shing Wai Chan
            Reporter:
            Nick Williams
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:

              Time Tracking

              Estimated:
              Original Estimate - 2 hours
              2h
              Remaining:
              Remaining Estimate - 2 hours
              2h
              Logged:
              Time Spent - Not Specified
              Not Specified