more #514 log-webop status/cancel: add handle-expiration, test coverage

docs/webapi.txt

@@ -219,7 +219,9 @@ POST /operations/$HANDLE?t=cancel
  This terminates the operation, and returns an HTML page explaining what was
  cancelled. If the operation handle has already expired (see below), this
  POST will return a 404, which indicates that the operation is no longer
- running (either it was completed or terminated).
+ running (either it was completed or terminated). The response body will be
+ the same as a t=status on this operation handle, and the handle will be
+ expired immediately afterwards.
 
 The operation handle will eventually expire, to avoid consuming an unbounded
 amount of memory. The handle's time-to-live can be reset at any time, by

src/allmydata/test/test_web.py

@@ -2150,13 +2150,73 @@ class Web(WebMixin, testutil.StallMixin, unittest.TestCase):
                                   client.getPage, url, method="DELETE")
         return d
 
-    def test_bad_ophandle(self):
+    def test_ophandle_bad(self):
         url = self.webish_url + "/operations/bogus?t=status"
-        d = self.shouldHTTPError2("test_bad_ophandle", 400, "400 Bad Request",
+        d = self.shouldHTTPError2("test_ophandle_bad", 404, "404 Not Found",
                                   "unknown/expired handle 'bogus'",
                                   client.getPage, url)
         return d
 
+    def test_ophandle_cancel(self):
+        d = self.POST(self.public_url + "/foo/?t=start-manifest&ophandle=128",
+                      followRedirect=True)
+        d.addCallback(lambda ignored:
+                      self.GET("/operations/128?t=status&output=JSON"))
+        def _check1(res):
+            data = simplejson.loads(res)
+            self.failUnless("finished" in data, res)
+            monitor = self.ws.root.child_operations.handles["128"][0]
+            d = self.POST("/operations/128?t=cancel&output=JSON")
+            def _check2(res):
+                data = simplejson.loads(res)
+                self.failUnless("finished" in data, res)
+                # t=cancel causes the handle to be forgotten
+                self.failUnless(monitor.is_cancelled())
+            d.addCallback(_check2)
+            return d
+        d.addCallback(_check1)
+        d.addCallback(lambda ignored:
+                      self.shouldHTTPError2("test_ophandle_cancel",
+                                            404, "404 Not Found",
+                                            "unknown/expired handle '128'",
+                                            self.GET,
+                                            "/operations/128?t=status&output=JSON"))
+        return d
+
+    def test_ophandle_retainfor(self):
+        d = self.POST(self.public_url + "/foo/?t=start-manifest&ophandle=129&retain-for=60",
+                      followRedirect=True)
+        d.addCallback(lambda ignored:
+                      self.GET("/operations/129?t=status&output=JSON&retain-for=0"))
+        def _check1(res):
+            data = simplejson.loads(res)
+            self.failUnless("finished" in data, res)
+        d.addCallback(_check1)
+        # the retain-for=0 will cause the handle to be expired very soon
+        d.addCallback(self.stall, 2.0)
+        d.addCallback(lambda ignored:
+                      self.shouldHTTPError2("test_ophandle_retainfor",
+                                            404, "404 Not Found",
+                                            "unknown/expired handle '129'",
+                                            self.GET,
+                                            "/operations/129?t=status&output=JSON"))
+        return d
+
+    def test_ophandle_release_after_complete(self):
+        d = self.POST(self.public_url + "/foo/?t=start-manifest&ophandle=130",
+                      followRedirect=True)
+        d.addCallback(self.wait_for_operation, "130")
+        d.addCallback(lambda ignored:
+                      self.GET("/operations/130?t=status&output=JSON&release-after-complete=true"))
+        # the release-after-complete=true will cause the handle to be expired
+        d.addCallback(lambda ignored:
+                      self.shouldHTTPError2("test_ophandle_release_after_complete",
+                                            404, "404 Not Found",
+                                            "unknown/expired handle '130'",
+                                            self.GET,
+                                            "/operations/130?t=status&output=JSON"))
+        return d
+
     def test_incident(self):
         d = self.POST("/report_incident", details="eek")
         def _done(res):

src/allmydata/web/directory.py

@@ -353,10 +353,8 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin):
 
     def _start_operation(self, monitor, renderer, ctx):
         table = IOpHandleTable(ctx)
-        ophandle = get_arg(ctx, "ophandle")
-        assert ophandle
-        table.add_monitor(ophandle, monitor, renderer)
-        return table.redirect_to(ophandle, ctx)
+        table.add_monitor(ctx, monitor, renderer)
+        return table.redirect_to(ctx)
 
     def _POST_start_deep_check(self, ctx):
         # check this directory and everything reachable from it

src/allmydata/web/operations.py

@@ -1,24 +1,65 @@
 
+import time
 from zope.interface import implements
 from nevow import rend, url, tags as T
 from nevow.inevow import IRequest
-from twisted.web import html
+from twisted.internet import reactor
+from twisted.web.http import NOT_FOUND
+from twisted.web.html import escape
+from twisted.application import service
 
-from allmydata.web.common import IOpHandleTable, get_root, get_arg, WebError
+from allmydata.web.common import IOpHandleTable, WebError, \
+     get_root, get_arg, boolean_of_arg
 
-class OphandleTable(rend.Page):
+MINUTE = 60
+HOUR = 60*MINUTE
+
+(MONITOR, RENDERER, WHEN_ADDED) = range(3)
+
+class OphandleTable(rend.Page, service.Service):
     implements(IOpHandleTable)
 
+    UNCOLLECTED_HANDLE_LIFETIME = 1*HOUR
+    COLLECTED_HANDLE_LIFETIME = 10*MINUTE
+
     def __init__(self):
-        self.monitors = {}
-        self.handles = {}
+        # both of these are indexed by ophandle
+        self.handles = {} # tuple of (monitor, renderer, when_added)
+        self.timers = {}
 
-    def add_monitor(self, ophandle, monitor, renderer):
-        self.monitors[ophandle] = monitor
-        self.handles[ophandle] = renderer
-        # TODO: expiration
+    def stopService(self):
+        for t in self.timers.values():
+            if t.active():
+                t.cancel()
+        del self.handles # this is not restartable
+        del self.timers
+        return service.Service.stopService(self)
 
-    def redirect_to(self, ophandle, ctx):
+    def add_monitor(self, ctx, monitor, renderer):
+        ophandle = get_arg(ctx, "ophandle")
+        assert ophandle
+        now = time.time()
+        self.handles[ophandle] = (monitor, renderer, now)
+        retain_for = get_arg(ctx, "retain-for", None)
+        if retain_for is not None:
+            self._set_timer(ophandle, int(retain_for))
+        monitor.when_done().addBoth(self._operation_complete, ophandle)
+
+    def _operation_complete(self, res, ophandle):
+        if ophandle in self.handles:
+            if ophandle not in self.timers:
+                # the client has not provided a retain-for= value for this
+                # handle, so we set our own.
+                now = time.time()
+                added = self.handles[ophandle][WHEN_ADDED]
+                when = max(self.UNCOLLECTED_HANDLE_LIFETIME, now - added)
+                self._set_timer(ophandle, when)
+            # if we already have a timer, the client must have provided the
+            # retain-for= value, so don't touch it.
+
+    def redirect_to(self, ctx):
+        ophandle = get_arg(ctx, "ophandle")
+        assert ophandle
         target = get_root(ctx) + "/operations/" + ophandle + "?t=status"
         output = get_arg(ctx, "output")
         if output:
@@ -28,14 +69,41 @@ class OphandleTable(rend.Page):
     def childFactory(self, ctx, name):
         ophandle = name
         if ophandle not in self.handles:
-            raise WebError("unknown/expired handle '%s'" %html.escape(ophandle))
+            raise WebError("unknown/expired handle '%s'" % escape(ophandle),
+                           NOT_FOUND)
+        (monitor, renderer, when_added) = self.handles[ophandle]
+
         t = get_arg(ctx, "t", "status")
         if t == "cancel":
-            monitor = self.monitors[ophandle]
             monitor.cancel()
-            # return the status anyways
+            # return the status anyways, but release the handle
+            self._release_ophandle(ophandle)
 
-        return self.handles[ophandle]
+        else:
+            retain_for = get_arg(ctx, "retain-for", None)
+            if retain_for is not None:
+                self._set_timer(ophandle, int(retain_for))
+
+            if monitor.is_finished():
+                if boolean_of_arg(get_arg(ctx, "release-after-complete", "false")):
+                    self._release_ophandle(ophandle)
+                if retain_for is None:
+                    # this GET is collecting the ophandle, so change its timer
+                    self._set_timer(ophandle, self.COLLECTED_HANDLE_LIFETIME)
+
+        return renderer
+
+    def _set_timer(self, ophandle, when):
+        if ophandle in self.timers and self.timers[ophandle].active():
+            self.timers[ophandle].cancel()
+        t = reactor.callLater(when, self._release_ophandle, ophandle)
+        self.timers[ophandle] = t
+
+    def _release_ophandle(self, ophandle):
+        if ophandle in self.timers and self.timers[ophandle].active():
+            self.timers[ophandle].cancel()
+        self.timers.pop(ophandle, None)
+        self.handles.pop(ophandle, None)
 
 class ReloadMixin:
 

src/allmydata/web/root.py

@@ -118,11 +118,14 @@ class Root(rend.Page):
     addSlash = True
     docFactory = getxmlfile("welcome.xhtml")
 
+    def __init__(self, original=None):
+        rend.Page.__init__(self, original)
+        self.child_operations = operations.OphandleTable()
+
     child_uri = URIHandler()
     child_cap = URIHandler()
     child_file = FileHandler()
     child_named = FileHandler()
-    child_operations = operations.OphandleTable()
 
     child_webform_css = webform.defaultCSS
     child_tahoe_css = nevow_File(resource_filename('allmydata.web', 'tahoe.css'))

src/allmydata/webish.py

@@ -131,6 +131,7 @@ class WebishServer(service.MultiService):
         self.site.requestFactory = MyRequest
         if self.root.child_operations:
             self.site.remember(self.root.child_operations, IOpHandleTable)
+            self.root.child_operations.setServiceParent(self)
         s = strports.service(webport, site)
         s.setServiceParent(self)
         self.listener = s # stash it so the tests can query for the portnum