From 9520ad71eb3c6ffa5d42b80d525f26936e79163b Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 21:24:21 -0400 Subject: [PATCH 001/148] Rearrange imports --- src/allmydata/web/status.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index b20aa679e..327d610a6 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -1,5 +1,7 @@ -import pprint, itertools, hashlib +import pprint +import itertools +import hashlib import json from twisted.internet import defer from twisted.python.filepath import FilePath @@ -24,8 +26,14 @@ from allmydata.web.common import ( MultiFormatPage, MultiFormatResource, ) -from allmydata.interfaces import IUploadStatus, IDownloadStatus, \ - IPublishStatus, IRetrieveStatus, IServermapUpdaterStatus + +from allmydata.interfaces import ( + IUploadStatus, + IDownloadStatus, + IPublishStatus, + IRetrieveStatus, + IServermapUpdaterStatus, +) class RateAndTimeMixin(object): From 81dc63b511704269b2038e7113b3ce753671512a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 21:32:44 -0400 Subject: [PATCH 002/148] Use twisted template tags in status page template --- src/allmydata/web/status.xhtml | 44 +++++++++++++++++----------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/allmydata/web/status.xhtml b/src/allmydata/web/status.xhtml index d827002f4..045d5aa85 100644 --- a/src/allmydata/web/status.xhtml +++ b/src/allmydata/web/status.xhtml @@ -1,4 +1,4 @@ - + Tahoe-LAFS - Recent and Active Operations @@ -11,8 +11,8 @@

Active Operations:

- - +
+ @@ -20,21 +20,21 @@ - - - - - - - + + + + + + + - +
Type Storage Index Helper?Progress Status
No active operations!
No active operations!

Recent Operations:

- - +
+ @@ -43,16 +43,16 @@ - - - - - - - - + + + + + + + + - +
Started Type Storage IndexProgress Status
No recent operations!
No recent operations!

From 3357f4c5ceea5a0dcde94d0ff069914821dbde61 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 21:43:41 -0400 Subject: [PATCH 003/148] Make Status a MultiFormatResource --- src/allmydata/web/status.py | 89 ++++++++++++++++++++++--------------- 1 file changed, 53 insertions(+), 36 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 327d610a6..e5165a570 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -1125,14 +1125,19 @@ def marshal_json(s): return item -class Status(MultiFormatPage): - docFactory = getxmlfile("status.xhtml") +class Status(MultiFormatResource): + addSlash = True def __init__(self, history): - rend.Page.__init__(self, history) + super(Status, self).__init__() self.history = history + def render_HTML(self, req): + elem = StatusElement(self._get_active_operations(), + self._get_recent_operations()) + return renderElement(req, elem) + def render_JSON(self, req): # modern browsers now render this instead of forcing downloads req.setHeader("content-type", "application/json") @@ -1180,7 +1185,51 @@ class Status(MultiFormatPage): recent.reverse() return recent - def render_row(self, ctx, data): + def childFactory(self, ctx, name): + h = self.history + try: + stype, count_s = name.split("-") + except ValueError: + raise RuntimeError( + "no - in '{}'".format(name) + ) + count = int(count_s) + if stype == "up": + for s in itertools.chain(h.list_all_upload_statuses(), + h.list_all_helper_statuses()): + # immutable-upload helpers use the same status object as a + # regular immutable-upload + if s.get_counter() == count: + return UploadStatusPage(s) + if stype == "down": + for s in h.list_all_download_statuses(): + if s.get_counter() == count: + return DownloadStatusPage(s) + if stype == "mapupdate": + for s in h.list_all_mapupdate_statuses(): + if s.get_counter() == count: + return MapupdateStatusPage(s) + if stype == "publish": + for s in h.list_all_publish_statuses(): + if s.get_counter() == count: + return PublishStatusPage(s) + if stype == "retrieve": + for s in h.list_all_retrieve_statuses(): + if s.get_counter() == count: + return RetrieveStatusPage(s) + + +class StatusElement(Element): + + loader = XMLFile(FilePath(__file__).sibling("status.xhtml")) + + def __init__(self, active, recent): + super(StatusElement, self).__init__() + self._active = active + self._recent = recent + + @renderer + def row(self, req, tag): s = data started_s = render_time(s.get_started()) @@ -1231,38 +1280,6 @@ class Status(MultiFormatPage): ctx.fillSlots("status", T.a(href=link)[s.get_status()]) return ctx.tag - def childFactory(self, ctx, name): - h = self.history - try: - stype, count_s = name.split("-") - except ValueError: - raise RuntimeError( - "no - in '{}'".format(name) - ) - count = int(count_s) - if stype == "up": - for s in itertools.chain(h.list_all_upload_statuses(), - h.list_all_helper_statuses()): - # immutable-upload helpers use the same status object as a - # regular immutable-upload - if s.get_counter() == count: - return UploadStatusPage(s) - if stype == "down": - for s in h.list_all_download_statuses(): - if s.get_counter() == count: - return DownloadStatusPage(s) - if stype == "mapupdate": - for s in h.list_all_mapupdate_statuses(): - if s.get_counter() == count: - return MapupdateStatusPage(s) - if stype == "publish": - for s in h.list_all_publish_statuses(): - if s.get_counter() == count: - return PublishStatusPage(s) - if stype == "retrieve": - for s in h.list_all_retrieve_statuses(): - if s.get_counter() == count: - return RetrieveStatusPage(s) # Render "/helper_status" page. class HelperStatus(MultiFormatResource): From a6e32135e866c8bb3621ae725137cb27953128df Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 22:10:40 -0400 Subject: [PATCH 004/148] Update status page child route handler and table renderers Drop nevow-isms and use twisted.web's way of doing things. --- src/allmydata/web/status.py | 162 ++++++++++++++++++++---------------- 1 file changed, 90 insertions(+), 72 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index e5165a570..1e0c81b98 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -25,6 +25,7 @@ from allmydata.web.common import ( render_time, MultiFormatPage, MultiFormatResource, + SlotsSequenceElement, ) from allmydata.interfaces import ( @@ -1127,8 +1128,6 @@ def marshal_json(s): class Status(MultiFormatResource): - addSlash = True - def __init__(self, history): super(Status, self).__init__() self.history = history @@ -1153,45 +1152,24 @@ class Status(MultiFormatResource): return json.dumps(data, indent=1) + "\n" - def _get_all_statuses(self): - h = self.history - return itertools.chain(h.list_all_upload_statuses(), - h.list_all_download_statuses(), - h.list_all_mapupdate_statuses(), - h.list_all_publish_statuses(), - h.list_all_retrieve_statuses(), - h.list_all_helper_statuses(), - ) + def getChild(self, path, request): + # The "if (path is empty) return self" line should handle + # trailing slash in request path. + # + # Twisted Web's documentation says this: "If the URL ends in a + # slash, for example ``http://example.com/foo/bar/`` , the + # final URL segment will be an empty string. Resources can + # thus know if they were requested with or without a final + # slash." + if not path: + return self - def data_active_operations(self, ctx, data): - return self._get_active_operations() - - def _get_active_operations(self): - active = [s - for s in self._get_all_statuses() - if s.get_active()] - active.sort(lambda a, b: cmp(a.get_started(), b.get_started())) - active.reverse() - return active - - def data_recent_operations(self, ctx, data): - return self._get_recent_operations() - - def _get_recent_operations(self): - recent = [s - for s in self._get_all_statuses() - if not s.get_active()] - recent.sort(lambda a, b: cmp(a.get_started(), b.get_started())) - recent.reverse() - return recent - - def childFactory(self, ctx, name): h = self.history try: - stype, count_s = name.split("-") + stype, count_s = path.split("-") except ValueError: raise RuntimeError( - "no - in '{}'".format(name) + "no - in '{}'".format(path) ) count = int(count_s) if stype == "up": @@ -1218,6 +1196,32 @@ class Status(MultiFormatResource): if s.get_counter() == count: return RetrieveStatusPage(s) + def _get_all_statuses(self): + h = self.history + return itertools.chain(h.list_all_upload_statuses(), + h.list_all_download_statuses(), + h.list_all_mapupdate_statuses(), + h.list_all_publish_statuses(), + h.list_all_retrieve_statuses(), + h.list_all_helper_statuses(), + ) + + def _get_active_operations(self): + active = [s + for s in self._get_all_statuses() + if s.get_active()] + active.sort(lambda a, b: cmp(a.get_started(), b.get_started())) + active.reverse() + return active + + def _get_recent_operations(self): + recent = [s + for s in self._get_all_statuses() + if not s.get_active()] + recent.sort(lambda a, b: cmp(a.get_started(), b.get_started())) + recent.reverse() + return recent + class StatusElement(Element): @@ -1229,56 +1233,70 @@ class StatusElement(Element): self._recent = recent @renderer - def row(self, req, tag): - s = data + def active_operations(self, req, tag): + active = [self.get_op_state(op) for op in self._active] + return SlotsSequenceElement(tag, active) - started_s = render_time(s.get_started()) - ctx.fillSlots("started", started_s) + @renderer + def recent_operations(self, req, tag): + active = [self.get_op_state(op) for op in self._recent] + return SlotsSequenceElement(tag, active) - si_s = base32.b2a_or_none(s.get_storage_index()) + @staticmethod + def get_op_state(op): + result = dict() + + started_s = render_time(op.get_started()) + result.update({"started": started_s}) + + si_s = base32.b2a_or_none(op.get_storage_index()) if si_s is None: si_s = "(None)" - ctx.fillSlots("si", si_s) - ctx.fillSlots("helper", {True: "Yes", - False: "No"}[s.using_helper()]) - size = s.get_size() + result.update({"si": si_s}) + result.update({"helper": + {True: "Yes", False: "No"}[op.using_helper()]}) + + size = op.get_size() if size is None: size = "(unknown)" elif isinstance(size, (int, long, float)): size = abbreviate_size(size) - ctx.fillSlots("total_size", size) - progress = data.get_progress() - if IUploadStatus.providedBy(data): - link = "up-%d" % data.get_counter() - ctx.fillSlots("type", "upload") + result.update({"total_size": size}) + + progress = op.get_progress() + if IUploadStatus.providedBy(op): + link = "up-%d" % op.get_counter() + result.update({"type": "upload"}) # TODO: make an ascii-art bar (chk, ciphertext, encandpush) = progress progress_s = ("hash: %.1f%%, ciphertext: %.1f%%, encode: %.1f%%" % - ( (100.0 * chk), - (100.0 * ciphertext), - (100.0 * encandpush) )) - ctx.fillSlots("progress", progress_s) - elif IDownloadStatus.providedBy(data): - link = "down-%d" % data.get_counter() - ctx.fillSlots("type", "download") - ctx.fillSlots("progress", "%.1f%%" % (100.0 * progress)) - elif IPublishStatus.providedBy(data): - link = "publish-%d" % data.get_counter() - ctx.fillSlots("type", "publish") - ctx.fillSlots("progress", "%.1f%%" % (100.0 * progress)) - elif IRetrieveStatus.providedBy(data): - ctx.fillSlots("type", "retrieve") - link = "retrieve-%d" % data.get_counter() - ctx.fillSlots("progress", "%.1f%%" % (100.0 * progress)) + ((100.0 * chk), + (100.0 * ciphertext), + (100.0 * encandpush))) + result.update({"progress": progress_s}) + elif IDownloadStatus.providedBy(op): + link = "down-%d" % op.get_counter() + result.update({"type": "download"}) + result.update({"progress": "%.1f%%" % (100.0 * progress)}) + elif IPublishStatus.providedBy(op): + link = "publish-%d" % op.get_counter() + result.update({"type": "publish"}) + result.update({"progress": "%.1f%%" % (100.0 * progress)}) + elif IRetrieveStatus.providedBy(op): + result.update({"type": "retrieve"}) + link = "retrieve-%d" % op.get_counter() + result.update({"progress": "%.1f%%" % (100.0 * progress)}) else: - assert IServermapUpdaterStatus.providedBy(data) - ctx.fillSlots("type", "mapupdate %s" % data.get_mode()) - link = "mapupdate-%d" % data.get_counter() - ctx.fillSlots("progress", "%.1f%%" % (100.0 * progress)) - ctx.fillSlots("status", T.a(href=link)[s.get_status()]) - return ctx.tag + assert IServermapUpdaterStatus.providedBy(op) + result.update({"type": "mapupdate %s" % op.get_mode()}) + link = "mapupdate-%d" % op.get_counter() + result.update({"progress": "%.1f%%" % (100.0 * progress)}) + + result.update({"status": tags.a(op.get_status(), href=link)}) + + return result # Render "/helper_status" page. From bd799d9d726953c08b82ff011c5f84b5d98d987a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 22:18:58 -0400 Subject: [PATCH 005/148] Add newsfragment --- newsfragments/3254.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3254.minor diff --git a/newsfragments/3254.minor b/newsfragments/3254.minor new file mode 100644 index 000000000..e69de29bb From 93d015e468dc427743de9dbf2cf35c67ca7ad189 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 22:24:09 -0400 Subject: [PATCH 006/148] Document status.Status --- src/allmydata/web/status.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 1e0c81b98..db127db59 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -1127,8 +1127,12 @@ def marshal_json(s): class Status(MultiFormatResource): + """Renders /status page.""" def __init__(self, history): + """ + :param allmydata.history.History history: provides operation statuses. + """ super(Status, self).__init__() self.history = history From d3f43d31d998fb97f8ad3e6fac007ed3fe57ab15 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Sat, 23 May 2020 06:17:06 -0400 Subject: [PATCH 007/148] Remove unused import --- src/allmydata/web/status.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index db127db59..ec18426dc 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -23,7 +23,6 @@ from allmydata.web.common import ( plural, compute_rate, render_time, - MultiFormatPage, MultiFormatResource, SlotsSequenceElement, ) From f9e864c51cf079a61cff449074f35ec3d7bc2a53 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 24 Jun 2020 08:15:18 -0400 Subject: [PATCH 008/148] Use simpler syntax to update map --- src/allmydata/web/status.py | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index ec18426dc..8214913de 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -1250,15 +1250,14 @@ class StatusElement(Element): result = dict() started_s = render_time(op.get_started()) - result.update({"started": started_s}) + result["started"] = started_s si_s = base32.b2a_or_none(op.get_storage_index()) if si_s is None: si_s = "(None)" - result.update({"si": si_s}) - result.update({"helper": - {True: "Yes", False: "No"}[op.using_helper()]}) + result["si"] = si_s + result["helper"] = {True: "Yes", False: "No"}[op.using_helper()] size = op.get_size() if size is None: @@ -1266,38 +1265,38 @@ class StatusElement(Element): elif isinstance(size, (int, long, float)): size = abbreviate_size(size) - result.update({"total_size": size}) + result["total_size"] = size progress = op.get_progress() if IUploadStatus.providedBy(op): link = "up-%d" % op.get_counter() - result.update({"type": "upload"}) + result["type"] = "upload" # TODO: make an ascii-art bar (chk, ciphertext, encandpush) = progress progress_s = ("hash: %.1f%%, ciphertext: %.1f%%, encode: %.1f%%" % ((100.0 * chk), (100.0 * ciphertext), (100.0 * encandpush))) - result.update({"progress": progress_s}) + result["progress"] = progress_s elif IDownloadStatus.providedBy(op): link = "down-%d" % op.get_counter() - result.update({"type": "download"}) - result.update({"progress": "%.1f%%" % (100.0 * progress)}) + result["type"] = "download" + result["progress"] = "%.1f%%" % (100.0 * progress) elif IPublishStatus.providedBy(op): link = "publish-%d" % op.get_counter() - result.update({"type": "publish"}) - result.update({"progress": "%.1f%%" % (100.0 * progress)}) + result["type"] = "publish" + result["progress"] = "%.1f%%" % (100.0 * progress) elif IRetrieveStatus.providedBy(op): - result.update({"type": "retrieve"}) + result["type"] = "retrieve" link = "retrieve-%d" % op.get_counter() - result.update({"progress": "%.1f%%" % (100.0 * progress)}) + result["progress"] = "%.1f%%" % (100.0 * progress) else: assert IServermapUpdaterStatus.providedBy(op) - result.update({"type": "mapupdate %s" % op.get_mode()}) + result["type"] = "mapupdate %s" % op.get_mode() link = "mapupdate-%d" % op.get_counter() - result.update({"progress": "%.1f%%" % (100.0 * progress)}) + result["progress"] = "%.1f%%" % (100.0 * progress) - result.update({"status": tags.a(op.get_status(), href=link)}) + result["status"] = tags.a(op.get_status(), href=link) return result From 54248f334ed8a7eaab0df7ffa6e5ce981e4187d5 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 9 Jul 2020 10:01:13 -0400 Subject: [PATCH 009/148] Use WebError, not RuntimeError, to indicate error Several problems with using RuntimeError to signal error here: - It dumps a rather unhelpful webpage at the user. - The exception backtrace on Tahoe console is not quite necessary here. - It really is not a runtime error: it is just an expected failure. - But mainly, testing for RuntimeError is harder. --- src/allmydata/web/status.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 8214913de..0c9e1ebb1 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -25,6 +25,7 @@ from allmydata.web.common import ( render_time, MultiFormatResource, SlotsSequenceElement, + WebError, ) from allmydata.interfaces import ( @@ -1171,9 +1172,7 @@ class Status(MultiFormatResource): try: stype, count_s = path.split("-") except ValueError: - raise RuntimeError( - "no - in '{}'".format(path) - ) + raise WebError("no '-' in '{}'".format(path)) count = int(count_s) if stype == "up": for s in itertools.chain(h.list_all_upload_statuses(), From 2e160c15266284d84e1e94b9025814b0206fe0a1 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 9 Jul 2020 14:23:26 -0400 Subject: [PATCH 010/148] Test for failure caused by invalid path A hyphen is expected when rendering /status page child nodes: "/status/up" is wrong; "/status/up-0" is right. --- src/allmydata/test/web/test_web.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 5be0b2f7b..c3779c0be 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1034,6 +1034,11 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return d + def test_status_path_error(self): + # Expect an error, because path is expected to be of the form + # "/status/{up,down,..}-%number", with a hyphen. + return self.assertFailure(self.GET("/storage/nodash"), error.Error) + def test_status_numbers(self): drrm = status.DownloadResultsRendererMixin() self.failUnlessReallyEqual(drrm.render_time(None, None), "") From 4dd6c8695076ce579f96f3af01116bdd6c0694e0 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 2 Jun 2020 11:56:59 -0400 Subject: [PATCH 011/148] Use twisted web directives in web.operations.ReloadMixin --- src/allmydata/web/operations.py | 36 ++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/allmydata/web/operations.py b/src/allmydata/web/operations.py index 93de2169e..eb9321344 100644 --- a/src/allmydata/web/operations.py +++ b/src/allmydata/web/operations.py @@ -1,7 +1,11 @@ import time -from nevow import rend, url, tags as T +from nevow import rend, url from nevow.inevow import IRequest +from twisted.web.template import ( + renderer, + tags as T, +) from twisted.python.failure import Failure from twisted.internet import reactor, defer from twisted.web.http import NOT_FOUND @@ -122,31 +126,31 @@ class OphandleTable(rend.Page, service.Service): class ReloadMixin(object): REFRESH_TIME = 1*MINUTE - def render_refresh(self, ctx, data): + @renderer + def refresh(self, req, tag): if self.monitor.is_finished(): return "" # dreid suggests ctx.tag(**dict([("http-equiv", "refresh")])) # but I can't tell if he's joking or not - ctx.tag.attributes["http-equiv"] = "refresh" - ctx.tag.attributes["content"] = str(self.REFRESH_TIME) - return ctx.tag + tag.attributes["http-equiv"] = "refresh" + tag.attributes["content"] = str(self.REFRESH_TIME) + return tag - def render_reload(self, ctx, data): + @renderer + def reload(self, req, tag): if self.monitor.is_finished(): return "" - req = IRequest(ctx) # url.gethere would break a proxy, so the correct thing to do is # req.path[-1] + queryargs ophandle = req.prepath[-1] reload_target = ophandle + "?output=html" cancel_target = ophandle + "?t=cancel" - cancel_button = T.form(action=cancel_target, method="POST", - enctype="multipart/form-data")[ - T.input(type="submit", value="Cancel"), - ] + cancel_button = T.form(T.input(type="submit", value="Cancel"), + action=cancel_target, + method="POST", + enctype="multipart/form-data",) - return [T.h2["Operation still running: ", - T.a(href=reload_target)["Reload"], - ], - cancel_button, - ] + return (T.h2("Operation still running: ", + T.a("Reload", href=reload_target), + ), + cancel_button,) From c043d1beadde39dc79385729c8603ba2117b2926 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 2 Jun 2020 12:06:48 -0400 Subject: [PATCH 012/148] Add newsfragment --- newsfragments/3315.minor | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/3315.minor diff --git a/newsfragments/3315.minor b/newsfragments/3315.minor new file mode 100644 index 000000000..ed00922a6 --- /dev/null +++ b/newsfragments/3315.minor @@ -0,0 +1 @@ +Add your info here \ No newline at end of file From 88accc23c3c0614a2f1d09ee10467a9cc7d56b0e Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 5 Jun 2020 09:28:36 -0400 Subject: [PATCH 013/148] Update newsfragment --- newsfragments/3315.minor | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/3315.minor b/newsfragments/3315.minor index ed00922a6..0536c297a 100644 --- a/newsfragments/3315.minor +++ b/newsfragments/3315.minor @@ -1 +1 @@ -Add your info here \ No newline at end of file +Replace nevow with twisted.web in web.operations.ReloadMixin From 7444d6b7a9a1c6ca71e6b499e304c1bb0f807f14 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 28 May 2020 13:42:55 -0400 Subject: [PATCH 014/148] Correct arguments to super() This was incidentally found when looking at some test failures: super(type, object) is the right invocation, and not the other way around. --- src/allmydata/test/common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/common.py b/src/allmydata/test/common.py index e7a94ff40..a2af857b9 100644 --- a/src/allmydata/test/common.py +++ b/src/allmydata/test/common.py @@ -1269,5 +1269,5 @@ class TrialTestCase(_TrialTestCase): if six.PY2: if isinstance(msg, six.text_type): - return super(self, TrialTestCase).fail(msg.encode("utf8")) - return super(self, TrialTestCase).fail(msg) + return super(TrialTestCase, self).fail(msg.encode("utf8")) + return super(TrialTestCase, self).fail(msg) From 5892eae580a71d2b99760ec72685532df9a8cb45 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 28 May 2020 13:49:13 -0400 Subject: [PATCH 015/148] Render self in DirectoryNodeHandler.getChild() We need self.render_POST() etc. to be invoked when we have a request such as "POST /uri/URI:DIR:..."; throwing an error here is probably not the right thing to do. --- src/allmydata/web/directory.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 57c222788..2129cec25 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -103,12 +103,10 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): # for these nodes, which is that a URI like # "/uri/URI%3ADIR2%3Aj...vq/" (that is, with a trailing slash # or no further children) renders "this" page - name = name.decode('utf8') if not name: - raise EmptyPathnameComponentError( - u"The webapi does not allow empty pathname components", - ) + return self + name = name.decode('utf8') d = self.node.get(name) d.addBoth(self._got_child, req, name) return d From 83b5807ca119a4641ee1a5e928e6a10d3ef5d147 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 29 May 2020 09:24:47 -0400 Subject: [PATCH 016/148] Add newsfragment --- newsfragments/3312.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/3312.bugfix diff --git a/newsfragments/3312.bugfix b/newsfragments/3312.bugfix new file mode 100644 index 000000000..ed00922a6 --- /dev/null +++ b/newsfragments/3312.bugfix @@ -0,0 +1 @@ +Add your info here \ No newline at end of file From d3d0d0285596ac75c055be5662fb7b9099630821 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 29 May 2020 19:47:12 -0400 Subject: [PATCH 017/148] Reject URIs that contain empty segments --- src/allmydata/web/directory.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 2129cec25..fced8c68e 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -106,6 +106,16 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): if not name: return self + # Rejecting URIs that contain empty path pieces (for example: + # "/uri/URI:DIR2:../foo//new.txt" or "/uri/URI:DIR2:..//") was + # the old nevow behavior and it is encoded in the test suite; + # we will follow suit. + for segment in req.prepath: + if not segment: + raise EmptyPathnameComponentError( + u"The webapi does not allow empty pathname components", + ) + name = name.decode('utf8') d = self.node.get(name) d.addBoth(self._got_child, req, name) From 378b20c51a489a22deb80567f4a18807c7ed55d4 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 29 May 2020 21:38:17 -0400 Subject: [PATCH 018/148] Reject trailing empty path piece in URIs --- src/allmydata/web/directory.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index fced8c68e..b1618a3f5 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -102,8 +102,9 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): # trying to replicate what I have observed as Nevow behavior # for these nodes, which is that a URI like # "/uri/URI%3ADIR2%3Aj...vq/" (that is, with a trailing slash - # or no further children) renders "this" page - if not name: + # or no further children) renders "this" page. We also need + # to reject "/uri/URI:DIR2:..//", so we look at postpath. + if not name and req.postpath != ['']: return self # Rejecting URIs that contain empty path pieces (for example: From fccc3316711523b021f46f0b2b932da9080f5ff4 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 29 May 2020 21:38:35 -0400 Subject: [PATCH 019/148] Make "Refresh" link work This seems to be a subtle difference from nevow: with `href="."`, rendered link target will be `/uri/`, so clicking "Refresh" will result in an error message like so: "GET /uri requires uri=". With `href=""`, the rendered link target will be `/uri/URI:...`, which is what we need. --- src/allmydata/web/directory.xhtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/directory.xhtml b/src/allmydata/web/directory.xhtml index 88058efa4..eeb04692d 100644 --- a/src/allmydata/web/directory.xhtml +++ b/src/allmydata/web/directory.xhtml @@ -31,7 +31,7 @@
From b956d9a58c2f4e92b5871071a861318a669c8428 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 29 May 2020 21:46:23 -0400 Subject: [PATCH 020/148] Decode name before null check Make the diff against master a tad bit smaller. --- src/allmydata/web/directory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index b1618a3f5..a5ba3bed5 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -104,6 +104,7 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): # "/uri/URI%3ADIR2%3Aj...vq/" (that is, with a trailing slash # or no further children) renders "this" page. We also need # to reject "/uri/URI:DIR2:..//", so we look at postpath. + name = name.decode('utf8') if not name and req.postpath != ['']: return self @@ -117,7 +118,6 @@ class DirectoryNodeHandler(ReplaceMeMixin, Resource, object): u"The webapi does not allow empty pathname components", ) - name = name.decode('utf8') d = self.node.get(name) d.addBoth(self._got_child, req, name) return d From 1fd220dba0796d0b7abf1aeb45c4e02b9e331f7a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 16 Jun 2020 12:10:44 -0400 Subject: [PATCH 021/148] Update newsfragment text --- newsfragments/3312.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/3312.bugfix b/newsfragments/3312.bugfix index ed00922a6..9939fe1f0 100644 --- a/newsfragments/3312.bugfix +++ b/newsfragments/3312.bugfix @@ -1 +1 @@ -Add your info here \ No newline at end of file +Make directory page links work. From 6e2285ed2c024f271a8b656bbeb5a2563c64bbfa Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 20 May 2020 22:43:21 -0600 Subject: [PATCH 022/148] proof-of-concept verified-fake for testing --- integration/test_testing.py | 26 +++ src/allmydata/testing/__init__.py | 0 src/allmydata/testing/web.py | 263 ++++++++++++++++++++++++++++++ 3 files changed, 289 insertions(+) create mode 100644 integration/test_testing.py create mode 100644 src/allmydata/testing/__init__.py create mode 100644 src/allmydata/testing/web.py diff --git a/integration/test_testing.py b/integration/test_testing.py new file mode 100644 index 000000000..47498b1d7 --- /dev/null +++ b/integration/test_testing.py @@ -0,0 +1,26 @@ + + +from allmydata.testing.web import ( + create_fake_tahoe_root, + deterministic_key_generator, +) + +import pytest +import pytest_twisted + + +@pytest_twisted.inlineCallbacks +def test_retrieve_cap(): + """ + WebUI Fake can serve a read-capability back + """ + + keys = deterministic_key_generator() + root = yield create_fake_tahoe_root() + dummy_readcap = yield root.add_data( + next(keys), + "some dummy content\n"*20 + ) + print("readcap: {}".format(dummy_readcap)) + + assert dummy_readcap.to_string() == "URI:CHK:ifaucqkbifaucqkbifaucqkbie:qg3n5r4q36wvwt33fobghsyqacc5ymnadk7wf6hafamsh6amybza:1:1:380" diff --git a/src/allmydata/testing/__init__.py b/src/allmydata/testing/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py new file mode 100644 index 000000000..5cf517a54 --- /dev/null +++ b/src/allmydata/testing/web.py @@ -0,0 +1,263 @@ +import io +import os +import string + +import attr + +from twisted.internet import defer +from twisted.python.filepath import ( + FilePath, +) +from twisted.web.resource import ( + Resource, +) +from twisted.web.client import ( + Agent, + FileBodyProducer, +) + +from treq.client import ( + HTTPClient, +) +from treq.testing import ( + RequestTraversalAgent, + RequestSequence, + StubTreq, +) + + +class _FakeTahoeRoot(Resource): + """ + This is a sketch of how an in-memory 'fake' of a Tahoe + WebUI. Ultimately, this will live in Tahoe + """ + + def __init__(self, uri=None): + Resource.__init__(self) # this is an old-style class :( + self._uri = uri + self.putChild(b"uri", self._uri) + + def add_data(self, key, data): + return self._uri.add_data(key, data) + +@attr.s +class _FakeCapability(object): + """ + """ + data=attr.ib() + + +# XXX want to make all kinds of caps, like +# URI:CHK:... URI:DIR2:... etc + +import allmydata.uri +KNOWN_CAPABILITIES = [ + getattr(allmydata.uri, t).BASE_STRING + for t in dir(allmydata.uri) + if hasattr(getattr(allmydata.uri, t), 'BASE_STRING') +] + + +from allmydata.immutable.upload import BaseUploadable +from allmydata.interfaces import IUploadable +from zope.interface import implementer +from twisted.internet.defer import ( + inlineCallbacks, + succeed, + returnValue, +) + + +def deterministic_key_generator(): + character = 0 + while character < (26 * 2): + key = string.letters[character] * 16 + character += 1 + yield key + raise RuntimeError("Ran out of keys") + + +@implementer(IUploadable) +class DataUploadable(BaseUploadable): + # Base gives us: + # set_upload_status + # set_default_encoding_parameters + # get_all_encoding_parameters + + def __init__(self, data, key=None): + self._data = data + self._where = 0 + self._key = key if key is not None else urandom(16) + + def get_encryption_key(self): + return succeed(self._key) + + def get_size(self): + return succeed(len(self._data)) + + @inlineCallbacks + def read(self, amount): + data = [self._data[self._where : self._where + amount]] + self._where += amount + yield + returnValue(data) + + def close(self): + pass + +@inlineCallbacks +def create_fake_capability(kind, key, data): + if kind not in KNOWN_CAPABILITIES: + raise ValueError( + "'{}' not a known kind: {}".format( + kind, + ", ".join(list(KNOWN_CAPABILITIES.keys())), + ) + ) + + # XXX to use a allmydata.immutable.upload.CHKUploader directly, + # we'd need to instantiate: + + from allmydata.immutable.upload import ( + CHKUploader, + EncryptAnUploadable, + ) + from allmydata.immutable.encode import ( + Encoder, + ) + + class _FakeSecretHolder(object): + def get_renewal_secret(self): + return "renewal_secret" + + def get_cancel_secret(self): + return "cancel_secret" + + + @attr.s + class _FakeBucket(object): + data = attr.ib(init=False, default="") + + def callRemoteOnly(self, *args): + pass # print("callRemoteOnly({})".format(args)) + + def callRemote(self, verb, *args): + if verb == 'write': + offset, data = args + assert offset >= len(self.data) + while offset > len(self.data): + self.data += 'X' # marker data; we're padding + self.data += data + elif verb == 'close': + pass + else: + print("callRemote({})".format(args)) + + + @attr.s + class _FakeStorageServer(object): + buckets = attr.ib(default=attr.Factory(lambda: [_FakeBucket()])) + + def get_buckets(self, storage_index): + return succeed(self.buckets) + + def allocate_buckets(self, storage_index, renew_secret, cancel_secret, sharenums, allocated_size, canary=None): + # returns a 2-tuple .. second one maps share-num to BucketWriter + return succeed(( + {}, + { + i: bucket + for i, bucket in enumerate(self.buckets) + } + )) + + class _FakeServer(object): + def get_serverid(self): + return "fake_server" + + def get_name(self): + return "steven" + + def get_version(self): + return { + "http://allmydata.org/tahoe/protocols/storage/v1": { + "maximum-immutable-share-size": 10*1024*1024*1024, + } + } + + def get_lease_seed(self): + return "decafbaddecafbaddeca" + + def get_storage_server(self): + return _FakeStorageServer() + + + class _FakeStorageBroker(object): + def get_servers_for_psi(self, storage_index): + return [_FakeServer()] + + storage_broker = _FakeStorageBroker() + secret_holder = _FakeSecretHolder() + uploader = CHKUploader(storage_broker, secret_holder, progress=None, reactor=None) + uploadable = DataUploadable(data, key=key) + uploadable.set_default_encoding_parameters({ + "n": 1, + "k": 1, + "happy": 1, + }) + encrypted_uploadable = EncryptAnUploadable(uploadable) + + encoder = Encoder() + yield encoder.set_encrypted_uploadable(encrypted_uploadable) + + uploadresults = yield uploader.start(encrypted_uploadable) + + enc_key = yield uploadable.get_encryption_key() + + from allmydata.uri import from_string as uri_from_string + from allmydata.uri import CHKFileURI + + verify_cap = uri_from_string(uploadresults.get_verifycapstr()) + read_cap = CHKFileURI( + enc_key, + verify_cap.uri_extension_hash, + verify_cap.needed_shares, + verify_cap.total_shares, + verify_cap.size, + ) + uploadresults.set_uri(read_cap.to_string()) + + returnValue(read_cap) + + +class _FakeTahoeUriHandler(Resource): + """ + """ + + isLeaf = True + + @inlineCallbacks + def add_data(self, key, data): + """ + adds some data to our grid, returning a capability + """ + cap = yield create_fake_capability("URI:CHK:", key, data) + returnValue(cap) + + def render_GET(self, request): + print(request) + print(request.uri) + return b"URI:DIR2-CHK:some capability" + + +@inlineCallbacks +def create_fake_tahoe_root(): + """ + Probably should take some params to control what this fake does: + return errors, pre-populate capabilities, ... + """ + root = _FakeTahoeRoot( + uri=_FakeTahoeUriHandler(), + ) + yield + returnValue(root) From ab7936db56a2326c43980a69b5ec38ba5dac4c77 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 1 Jun 2020 09:06:46 -0600 Subject: [PATCH 023/148] actual usable agent --- src/allmydata/testing/web.py | 49 ++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 5cf517a54..1767cb114 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -261,3 +261,52 @@ def create_fake_tahoe_root(): ) yield returnValue(root) + + +@implementer(IBodyProducer) +class _SynchronousProducer(object): + """ + A partial implementation of an :obj:`IBodyProducer` which produces its + entire payload immediately. There is no way to access to an instance of + this object from :obj:`RequestTraversalAgent` or :obj:`StubTreq`, or even a + :obj:`Resource: passed to :obj:`StubTreq`. + + This does not implement the :func:`IBodyProducer.stopProducing` method, + because that is very difficult to trigger. (The request from + `RequestTraversalAgent` would have to be canceled while it is still in the + transmitting state), and the intent is to use `RequestTraversalAgent` to + make synchronous requests. + """ + + def __init__(self, body): + """ + Create a synchronous producer with some bytes. + """ + if not isinstance(body, bytes): + raise ValueError( + "'body' must be bytes" + ) + self.body = body + self.length = len(body) + + def startProducing(self, consumer): + """ + Immediately produce all data. + """ + consumer.write(self.body) + return succeed(None) + + +@inlineCallbacks +def create_tahoe_treq_client(root=None): + """ + """ + + if root is None: + root = yield create_fake_tahoe_root() + + client = HTTPClient( + agent=RequestTraversalAgent(root), + data_to_body_producer=_SynchronousProducer, + ) + returnValue(client) From 95531fa7295c24837d5b53054dc61d36777e8155 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 3 Jun 2020 11:32:22 -0600 Subject: [PATCH 024/148] synchronous-only up/download tahoe code to keep treq.testing happy --- src/allmydata/testing/web.py | 73 ++++++++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 7 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 1767cb114..32fe0c084 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -1,10 +1,12 @@ import io import os +import json import string import attr -from twisted.internet import defer +from hyperlink import DecodedURL + from twisted.python.filepath import ( FilePath, ) @@ -15,6 +17,9 @@ from twisted.web.client import ( Agent, FileBodyProducer, ) +from twisted.web.iweb import ( + IBodyProducer, +) from treq.client import ( HTTPClient, @@ -40,6 +45,7 @@ class _FakeTahoeRoot(Resource): def add_data(self, key, data): return self._uri.add_data(key, data) + @attr.s class _FakeCapability(object): """ @@ -70,8 +76,8 @@ from twisted.internet.defer import ( def deterministic_key_generator(): character = 0 - while character < (26 * 2): - key = string.letters[character] * 16 + while True:#character < (26 * 2): + key = string.letters[character % 52] * 16 character += 1 yield key raise RuntimeError("Ran out of keys") @@ -159,7 +165,7 @@ def create_fake_capability(kind, key, data): buckets = attr.ib(default=attr.Factory(lambda: [_FakeBucket()])) def get_buckets(self, storage_index): - return succeed(self.buckets) + return succeed({0: self.buckets}) def allocate_buckets(self, storage_index, renew_secret, cancel_secret, sharenums, allocated_size, canary=None): # returns a 2-tuple .. second one maps share-num to BucketWriter @@ -230,11 +236,34 @@ def create_fake_capability(kind, key, data): returnValue(read_cap) +def capability_generator(): + number = 0 + while True: + cap = u"URI:CHK:fake capability {}".format(number) + number += 1 + yield cap.encode("ascii") + + class _FakeTahoeUriHandler(Resource): """ """ isLeaf = True + _data = None + _key_generator = None + _capability_generator = None + + def _get_key(self): + if self._key_generator is None: + self._key_generator = deterministic_key_generator() + key = next(self._key_generator) + return key + + def _generate_cap(self): + if self._capability_generator is None: + self._capability_generator = capability_generator() + capability = next(self._capability_generator) + return capability @inlineCallbacks def add_data(self, key, data): @@ -242,12 +271,42 @@ class _FakeTahoeUriHandler(Resource): adds some data to our grid, returning a capability """ cap = yield create_fake_capability("URI:CHK:", key, data) + if self._data is None: + self._data = dict() + self._data[cap] = data returnValue(cap) + def render_PUT(self, request): + data = request.content.read() +# if len(data) == 0: +# raise RuntimeError("No empty data") + + cap = self._generate_cap() + if self._data is None: + self._data = dict() + self._data[cap] = data + return cap + + def render_POST(self, request): + kind = request.args[u"t"][0] + data = request.content.read() + + cap = self._generate_cap() + if self._data is None: + self._data = dict() + self._data[cap] = data + return cap +# if kind == 'mkdir-immutable': + def render_GET(self, request): - print(request) - print(request.uri) - return b"URI:DIR2-CHK:some capability" + uri = DecodedURL.from_text(request.uri.decode('utf8')) + # XXX FIXME + capability = uri.query[0][1] + + if self._data is None or capability not in self._data: + return u"No data for '{}'".format(capability).decode("ascii") + + return self._data[capability] @inlineCallbacks From 66116020fb78da074f1d7752fe2540dd686744c3 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 11 Jun 2020 13:26:09 -0600 Subject: [PATCH 025/148] simplify tahoe web fakes --- src/allmydata/testing/web.py | 287 ++++++++++------------------------- 1 file changed, 76 insertions(+), 211 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 32fe0c084..78d2b7931 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -2,6 +2,7 @@ import io import os import json import string +import hashlib import attr @@ -20,6 +21,11 @@ from twisted.web.client import ( from twisted.web.iweb import ( IBodyProducer, ) +from twisted.internet.defer import ( + inlineCallbacks, + succeed, + returnValue, +) from treq.client import ( HTTPClient, @@ -29,6 +35,12 @@ from treq.testing import ( RequestSequence, StubTreq, ) +from zope.interface import implementer + +import allmydata.uri +from allmydata.util import ( + base32, +) class _FakeTahoeRoot(Resource): @@ -53,10 +65,6 @@ class _FakeCapability(object): data=attr.ib() -# XXX want to make all kinds of caps, like -# URI:CHK:... URI:DIR2:... etc - -import allmydata.uri KNOWN_CAPABILITIES = [ getattr(allmydata.uri, t).BASE_STRING for t in dir(allmydata.uri) @@ -64,183 +72,46 @@ KNOWN_CAPABILITIES = [ ] -from allmydata.immutable.upload import BaseUploadable -from allmydata.interfaces import IUploadable -from zope.interface import implementer -from twisted.internet.defer import ( - inlineCallbacks, - succeed, - returnValue, -) +def capability_generator(kind): + """ + :param str kind: the kind of capability, like `URI:CHK` - -def deterministic_key_generator(): - character = 0 - while True:#character < (26 * 2): - key = string.letters[character % 52] * 16 - character += 1 - yield key - raise RuntimeError("Ran out of keys") - - -@implementer(IUploadable) -class DataUploadable(BaseUploadable): - # Base gives us: - # set_upload_status - # set_default_encoding_parameters - # get_all_encoding_parameters - - def __init__(self, data, key=None): - self._data = data - self._where = 0 - self._key = key if key is not None else urandom(16) - - def get_encryption_key(self): - return succeed(self._key) - - def get_size(self): - return succeed(len(self._data)) - - @inlineCallbacks - def read(self, amount): - data = [self._data[self._where : self._where + amount]] - self._where += amount - yield - returnValue(data) - - def close(self): - pass - -@inlineCallbacks -def create_fake_capability(kind, key, data): + :returns: a generator that yields new capablities of a particular + kind. + """ if kind not in KNOWN_CAPABILITIES: raise ValueError( - "'{}' not a known kind: {}".format( + "Unknown capability kind '{} (valid are {})'".format( kind, - ", ".join(list(KNOWN_CAPABILITIES.keys())), + ", ".join(KNOWN_CAPABILITIES), ) ) - - # XXX to use a allmydata.immutable.upload.CHKUploader directly, - # we'd need to instantiate: - - from allmydata.immutable.upload import ( - CHKUploader, - EncryptAnUploadable, - ) - from allmydata.immutable.encode import ( - Encoder, - ) - - class _FakeSecretHolder(object): - def get_renewal_secret(self): - return "renewal_secret" - - def get_cancel_secret(self): - return "cancel_secret" - - - @attr.s - class _FakeBucket(object): - data = attr.ib(init=False, default="") - - def callRemoteOnly(self, *args): - pass # print("callRemoteOnly({})".format(args)) - - def callRemote(self, verb, *args): - if verb == 'write': - offset, data = args - assert offset >= len(self.data) - while offset > len(self.data): - self.data += 'X' # marker data; we're padding - self.data += data - elif verb == 'close': - pass - else: - print("callRemote({})".format(args)) - - - @attr.s - class _FakeStorageServer(object): - buckets = attr.ib(default=attr.Factory(lambda: [_FakeBucket()])) - - def get_buckets(self, storage_index): - return succeed({0: self.buckets}) - - def allocate_buckets(self, storage_index, renew_secret, cancel_secret, sharenums, allocated_size, canary=None): - # returns a 2-tuple .. second one maps share-num to BucketWriter - return succeed(( - {}, - { - i: bucket - for i, bucket in enumerate(self.buckets) - } - )) - - class _FakeServer(object): - def get_serverid(self): - return "fake_server" - - def get_name(self): - return "steven" - - def get_version(self): - return { - "http://allmydata.org/tahoe/protocols/storage/v1": { - "maximum-immutable-share-size": 10*1024*1024*1024, - } - } - - def get_lease_seed(self): - return "decafbaddecafbaddeca" - - def get_storage_server(self): - return _FakeStorageServer() - - - class _FakeStorageBroker(object): - def get_servers_for_psi(self, storage_index): - return [_FakeServer()] - - storage_broker = _FakeStorageBroker() - secret_holder = _FakeSecretHolder() - uploader = CHKUploader(storage_broker, secret_holder, progress=None, reactor=None) - uploadable = DataUploadable(data, key=key) - uploadable.set_default_encoding_parameters({ - "n": 1, - "k": 1, - "happy": 1, - }) - encrypted_uploadable = EncryptAnUploadable(uploadable) - - encoder = Encoder() - yield encoder.set_encrypted_uploadable(encrypted_uploadable) - - uploadresults = yield uploader.start(encrypted_uploadable) - - enc_key = yield uploadable.get_encryption_key() - - from allmydata.uri import from_string as uri_from_string - from allmydata.uri import CHKFileURI - - verify_cap = uri_from_string(uploadresults.get_verifycapstr()) - read_cap = CHKFileURI( - enc_key, - verify_cap.uri_extension_hash, - verify_cap.needed_shares, - verify_cap.total_shares, - verify_cap.size, - ) - uploadresults.set_uri(read_cap.to_string()) - - returnValue(read_cap) - - -def capability_generator(): + # what we do here is to start with empty hashers for the key and + # ueb_hash and repeatedly feed() them a zero byte on each + # iteration .. so the same sequence of capabilities will always be + # produced. We could add a seed= argument if we wanted to produce + # different sequences. number = 0 + key_hasher = hashlib.new("sha256") + ueb_hasher = hashlib.new("sha256") + + # capabilities are "prefix:<128-bits-base32>:<256-bits-base32>:N:K:size" while True: - cap = u"URI:CHK:fake capability {}".format(number) number += 1 + key_hasher.update("\x00") + ueb_hasher.update("\x00") + + key = base32.b2a(key_hasher.digest()[:16]) # key is 16 bytes + ueb_hash = base32.b2a(ueb_hasher.digest()) # ueb hash is 32 bytes + + cap = u"{kind}:{key}:{ueb_hash}:{n}:{k}:{size}".format( + kind=kind, + key=key, + ueb_hash=ueb_hash, + n=1, + k=1, + size=number * 1000, + ) yield cap.encode("ascii") @@ -250,53 +121,50 @@ class _FakeTahoeUriHandler(Resource): isLeaf = True _data = None - _key_generator = None - _capability_generator = None + _capability_generators = None - def _get_key(self): - if self._key_generator is None: - self._key_generator = deterministic_key_generator() - key = next(self._key_generator) - return key + def _generate_capability(self, kind): + """ + :param str kind: any valid capability-string type - def _generate_cap(self): - if self._capability_generator is None: - self._capability_generator = capability_generator() - capability = next(self._capability_generator) + :returns: the next capability-string for the given kind + """ + if self._capability_generators is None: + self._capability_generators = dict() + + if kind not in self._capability_generators: + self._capability_generators[kind] = capability_generator(kind) + capability = next(self._capability_generators[kind]) return capability - @inlineCallbacks - def add_data(self, key, data): + def add_data(self, kind, data): """ - adds some data to our grid, returning a capability + adds some data to our grid + + :returns: a capability-string """ - cap = yield create_fake_capability("URI:CHK:", key, data) + assert isinstance(data, bytes) + + cap = self._generate_capability(kind) if self._data is None: self._data = dict() + assert cap not in self._data, "already have '{}'".format(cap) self._data[cap] = data - returnValue(cap) + return cap def render_PUT(self, request): data = request.content.read() -# if len(data) == 0: -# raise RuntimeError("No empty data") - - cap = self._generate_cap() - if self._data is None: - self._data = dict() - self._data[cap] = data - return cap + return self.add_data("URI:CHK:", data) def render_POST(self, request): - kind = request.args[u"t"][0] + t = request.args[u"t"][0] data = request.content.read() - cap = self._generate_cap() - if self._data is None: - self._data = dict() - self._data[cap] = data - return cap -# if kind == 'mkdir-immutable': + type_to_kind = { + "mkdir-immutable": "URI:DIR2-CHK:" + } + kind = type_to_kind[t] + return self.add_data(kind, data) def render_GET(self, request): uri = DecodedURL.from_text(request.uri.decode('utf8')) @@ -309,17 +177,15 @@ class _FakeTahoeUriHandler(Resource): return self._data[capability] -@inlineCallbacks def create_fake_tahoe_root(): """ - Probably should take some params to control what this fake does: - return errors, pre-populate capabilities, ... + :returns: an IResource instance that will handle certain Tahoe URI + endpoints similar to a real Tahoe server. """ root = _FakeTahoeRoot( uri=_FakeTahoeUriHandler(), ) - yield - returnValue(root) + return root @implementer(IBodyProducer) @@ -356,16 +222,15 @@ class _SynchronousProducer(object): return succeed(None) -@inlineCallbacks def create_tahoe_treq_client(root=None): """ """ if root is None: - root = yield create_fake_tahoe_root() + root = create_fake_tahoe_root() client = HTTPClient( agent=RequestTraversalAgent(root), data_to_body_producer=_SynchronousProducer, ) - returnValue(client) + return client From f216f977f0a9efc62b20639d2f5bebeef55d404c Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 11 Jun 2020 13:36:28 -0600 Subject: [PATCH 026/148] news --- newsfragments/3317.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/3317.feature diff --git a/newsfragments/3317.feature b/newsfragments/3317.feature new file mode 100644 index 000000000..5ed00acf5 --- /dev/null +++ b/newsfragments/3317.feature @@ -0,0 +1 @@ +Verified fakes for (some of) Tahoe's WebUI via treq \ No newline at end of file From 05140a58813573160d76a244850192a2a89ec15e Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 11 Jun 2020 14:14:42 -0600 Subject: [PATCH 027/148] simple test of testing/web --- src/allmydata/test/test_testing.py | 40 ++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 src/allmydata/test/test_testing.py diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py new file mode 100644 index 000000000..37e68a0de --- /dev/null +++ b/src/allmydata/test/test_testing.py @@ -0,0 +1,40 @@ +import io + +from twisted.trial.unittest import ( + TestCase, +) +from twisted.internet.defer import ( + inlineCallbacks, +) + +from allmydata.testing.web import ( + create_tahoe_treq_client, +) + + +class FakeWebTest(TestCase): + """ + Test the WebUI verified-fakes infrastucture + """ + + def setUp(self): + super(FakeWebTest, self).setUp() + self.http_client = create_tahoe_treq_client() + + @inlineCallbacks + def test_create_and_download(self): + """ + Upload some content and then download it + """ + content = "fake data\n" * 100 + resp = yield self.http_client.put("http://example.com/uri", content) + cap = yield resp.content() + + self.assertTrue(cap.startswith("URI:CHK:")) + + resp = yield self.http_client.get( + "http://example.com/uri?uri={}".format(cap) + ) + round_trip_content = yield resp.content() + + self.assertEqual(content, round_trip_content) From 18b62bf7465c1bc00bf5a8391b6e019e35604165 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 11 Jun 2020 15:34:40 -0600 Subject: [PATCH 028/148] unused imports --- src/allmydata/test/test_testing.py | 2 -- src/allmydata/testing/web.py | 15 --------------- 2 files changed, 17 deletions(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index 37e68a0de..da37b1de3 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -1,5 +1,3 @@ -import io - from twisted.trial.unittest import ( TestCase, ) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 78d2b7931..bc0138326 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -1,30 +1,17 @@ -import io -import os -import json -import string import hashlib import attr from hyperlink import DecodedURL -from twisted.python.filepath import ( - FilePath, -) from twisted.web.resource import ( Resource, ) -from twisted.web.client import ( - Agent, - FileBodyProducer, -) from twisted.web.iweb import ( IBodyProducer, ) from twisted.internet.defer import ( - inlineCallbacks, succeed, - returnValue, ) from treq.client import ( @@ -32,8 +19,6 @@ from treq.client import ( ) from treq.testing import ( RequestTraversalAgent, - RequestSequence, - StubTreq, ) from zope.interface import implementer From 02a663c244189c916d0c27ba473dc5b3d4ea7419 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 11 Jun 2020 15:34:47 -0600 Subject: [PATCH 029/148] new-style classes only --- src/allmydata/testing/web.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index bc0138326..de515ebc9 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -28,7 +28,7 @@ from allmydata.util import ( ) -class _FakeTahoeRoot(Resource): +class _FakeTahoeRoot(Resource, object): """ This is a sketch of how an in-memory 'fake' of a Tahoe WebUI. Ultimately, this will live in Tahoe @@ -100,7 +100,7 @@ def capability_generator(kind): yield cap.encode("ascii") -class _FakeTahoeUriHandler(Resource): +class _FakeTahoeUriHandler(Resource, object): """ """ From 1f2ad6e634ded72a47fd1494ffc597dba88620dd Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 11 Jun 2020 15:55:19 -0600 Subject: [PATCH 030/148] the tests are unit-tests --- integration/test_testing.py | 26 -------------------------- 1 file changed, 26 deletions(-) delete mode 100644 integration/test_testing.py diff --git a/integration/test_testing.py b/integration/test_testing.py deleted file mode 100644 index 47498b1d7..000000000 --- a/integration/test_testing.py +++ /dev/null @@ -1,26 +0,0 @@ - - -from allmydata.testing.web import ( - create_fake_tahoe_root, - deterministic_key_generator, -) - -import pytest -import pytest_twisted - - -@pytest_twisted.inlineCallbacks -def test_retrieve_cap(): - """ - WebUI Fake can serve a read-capability back - """ - - keys = deterministic_key_generator() - root = yield create_fake_tahoe_root() - dummy_readcap = yield root.add_data( - next(keys), - "some dummy content\n"*20 - ) - print("readcap: {}".format(dummy_readcap)) - - assert dummy_readcap.to_string() == "URI:CHK:ifaucqkbifaucqkbifaucqkbie:qg3n5r4q36wvwt33fobghsyqacc5ymnadk7wf6hafamsh6amybza:1:1:380" From c0ce7164309b3aea9f7d983eb9e2d315cca0eb2f Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 11 Jun 2020 19:57:11 -0600 Subject: [PATCH 031/148] handle more inputs in test body-producer --- src/allmydata/testing/web.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index de515ebc9..aa3b2a4a7 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -16,6 +16,7 @@ from twisted.internet.defer import ( from treq.client import ( HTTPClient, + FileBodyProducer, ) from treq.testing import ( RequestTraversalAgent, @@ -192,9 +193,12 @@ class _SynchronousProducer(object): """ Create a synchronous producer with some bytes. """ + if isinstance(body, FileBodyProducer): + body = body._inputFile.read() + if not isinstance(body, bytes): raise ValueError( - "'body' must be bytes" + "'body' must be bytes not '{}'".format(type(body)) ) self.body = body self.length = len(body) From 1710530b6bc7c884a705b0bf4f578ee01f8e3ffd Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 11 Jun 2020 19:57:21 -0600 Subject: [PATCH 032/148] key -> kind --- src/allmydata/testing/web.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index aa3b2a4a7..fa6a2555a 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -40,8 +40,8 @@ class _FakeTahoeRoot(Resource, object): self._uri = uri self.putChild(b"uri", self._uri) - def add_data(self, key, data): - return self._uri.add_data(key, data) + def add_data(self, kind, data): + return self._uri.add_data(kind, data) @attr.s From 0cc020b5f224f967cb01e05bc53f45b85b78dff7 Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 12 Jun 2020 22:04:03 -0600 Subject: [PATCH 033/148] copyright, header --- src/allmydata/test/test_testing.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index da37b1de3..4dc9c6bb6 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -1,6 +1,16 @@ -from twisted.trial.unittest import ( - TestCase, -) +# -*- coding: utf-8 -*- +# Tahoe-LAFS -- secure, distributed storage grid +# +# Copyright © 2020 The Tahoe-LAFS Software Foundation +# +# This file is part of Tahoe-LAFS. +# +# See the docs/about.rst file for licensing information. + +""" +Tests for the allmydata.testing helpers +""" + from twisted.internet.defer import ( inlineCallbacks, ) From b01995f158870a45ab8d848f8fae9e74191c6d6c Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 12 Jun 2020 22:04:12 -0600 Subject: [PATCH 034/148] use hypothesis -- which doesn't know about Deferred, so need testtools too --- src/allmydata/test/test_testing.py | 52 +++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index 4dc9c6bb6..e350546ca 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -19,6 +19,23 @@ from allmydata.testing.web import ( create_tahoe_treq_client, ) +from hypothesis import ( + given, +) +from hypothesis.strategies import ( + binary, +) + +from testtools import ( + TestCase, +) +from testtools.matchers import ( + Always, +) +from testtools.twistedsupport import ( + succeeded, +) + class FakeWebTest(TestCase): """ @@ -29,20 +46,31 @@ class FakeWebTest(TestCase): super(FakeWebTest, self).setUp() self.http_client = create_tahoe_treq_client() - @inlineCallbacks - def test_create_and_download(self): + @given( + content=binary(), + ) + def test_create_and_download(self, content): """ - Upload some content and then download it + Upload some content (via 'PUT /uri') and then download it (via + 'GET /uri?uri=...') """ - content = "fake data\n" * 100 - resp = yield self.http_client.put("http://example.com/uri", content) - cap = yield resp.content() - self.assertTrue(cap.startswith("URI:CHK:")) + @inlineCallbacks + def do_test(): + resp = yield self.http_client.put("http://example.com/uri", content) + self.assertEqual(resp.code, 200) - resp = yield self.http_client.get( - "http://example.com/uri?uri={}".format(cap) + cap = yield resp.content() + self.assertTrue(cap.startswith("URI:CHK:")) + + resp = yield self.http_client.get( + "http://example.com/uri?uri={}".format(cap) + ) + self.assertEqual(resp.code, 200) + + round_trip_content = yield resp.content() + self.assertEqual(content, round_trip_content) + self.assertThat( + do_test(), + succeeded(Always()), ) - round_trip_content = yield resp.content() - - self.assertEqual(content, round_trip_content) From 683c8d3e4f0ddac997cdc16ede90778bd597db75 Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 12 Jun 2020 22:08:36 -0600 Subject: [PATCH 035/148] actually returns CREATED for new data --- src/allmydata/test/test_testing.py | 2 +- src/allmydata/testing/web.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index e350546ca..a5c712d77 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -58,7 +58,7 @@ class FakeWebTest(TestCase): @inlineCallbacks def do_test(): resp = yield self.http_client.put("http://example.com/uri", content) - self.assertEqual(resp.code, 200) + self.assertEqual(resp.code, 201) cap = yield resp.content() self.assertTrue(cap.startswith("URI:CHK:")) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index fa6a2555a..99dabeddf 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -140,6 +140,7 @@ class _FakeTahoeUriHandler(Resource, object): def render_PUT(self, request): data = request.content.read() + request.setResponseCode(201) # real code does this for brand-new files return self.add_data("URI:CHK:", data) def render_POST(self, request): From 9d5e438c2d36d336da24d081d1bd2acc7f7501c5 Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 12 Jun 2020 22:57:38 -0600 Subject: [PATCH 036/148] use real parser --- src/allmydata/test/test_testing.py | 11 ++++++++--- src/allmydata/testing/web.py | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index a5c712d77..b5bbacad4 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -15,6 +15,10 @@ from twisted.internet.defer import ( inlineCallbacks, ) +from allmydata.uri import ( + from_string, + CHKFileURI, +) from allmydata.testing.web import ( create_tahoe_treq_client, ) @@ -60,11 +64,12 @@ class FakeWebTest(TestCase): resp = yield self.http_client.put("http://example.com/uri", content) self.assertEqual(resp.code, 201) - cap = yield resp.content() - self.assertTrue(cap.startswith("URI:CHK:")) + cap_raw = yield resp.content() + cap = from_string(cap_raw) + self.assertIsInstance(cap, CHKFileURI) resp = yield self.http_client.get( - "http://example.com/uri?uri={}".format(cap) + "http://example.com/uri?uri={}".format(cap.to_string()) ) self.assertEqual(resp.code, 200) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 99dabeddf..f6411792d 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -90,7 +90,7 @@ def capability_generator(kind): key = base32.b2a(key_hasher.digest()[:16]) # key is 16 bytes ueb_hash = base32.b2a(ueb_hasher.digest()) # ueb hash is 32 bytes - cap = u"{kind}:{key}:{ueb_hash}:{n}:{k}:{size}".format( + cap = u"{kind}{key}:{ueb_hash}:{n}:{k}:{size}".format( kind=kind, key=key, ueb_hash=ueb_hash, From e481e37beba39ab5a1de9b1630bd35a578b193ea Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 12 Jun 2020 23:01:02 -0600 Subject: [PATCH 037/148] housekeeping --- src/allmydata/testing/web.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index f6411792d..2b0f830f2 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -1,3 +1,16 @@ +# -*- coding: utf-8 -*- +# Tahoe-LAFS -- secure, distributed storage grid +# +# Copyright © 2020 The Tahoe-LAFS Software Foundation +# +# This file is part of Tahoe-LAFS. +# +# See the docs/about.rst file for licensing information. + +""" +Test-helpers for clients that use the WebUI. +""" + import hashlib import attr @@ -28,6 +41,11 @@ from allmydata.util import ( base32, ) +__all__ = ( + "create_fake_tahoe_root", + "create_tahoe_treq_client", +) + class _FakeTahoeRoot(Resource, object): """ From 94207778109cbdf60fc2df6cfa455e53474ca08a Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 12 Jun 2020 23:05:32 -0600 Subject: [PATCH 038/148] cleanup, docstrings --- src/allmydata/testing/web.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 2b0f830f2..7d9b5730b 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -49,8 +49,8 @@ __all__ = ( class _FakeTahoeRoot(Resource, object): """ - This is a sketch of how an in-memory 'fake' of a Tahoe - WebUI. Ultimately, this will live in Tahoe + An in-memory 'fake' of a Tahoe WebUI root. Currently it only + implements (some of) the `/uri` resource. """ def __init__(self, uri=None): @@ -62,13 +62,6 @@ class _FakeTahoeRoot(Resource, object): return self._uri.add_data(kind, data) -@attr.s -class _FakeCapability(object): - """ - """ - data=attr.ib() - - KNOWN_CAPABILITIES = [ getattr(allmydata.uri, t).BASE_STRING for t in dir(allmydata.uri) @@ -121,6 +114,8 @@ def capability_generator(kind): class _FakeTahoeUriHandler(Resource, object): """ + An in-memory fake of (some of) the `/uri` endpoint of a Tahoe + WebUI """ isLeaf = True @@ -232,6 +227,12 @@ class _SynchronousProducer(object): def create_tahoe_treq_client(root=None): """ + :param root: an instance created via `create_fake_tahoe_root`. The + caller might want a copy of this to call `.add_data` for example. + + :returns: an instance of treq.client.HTTPClient wired up to + in-memory fakes of the Tahoe WebUI. Only a subset of the real + WebUI is available. """ if root is None: From 10715fa26b88e3999d081a3c1b9623b6aef781d5 Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 12 Jun 2020 23:06:33 -0600 Subject: [PATCH 039/148] docstring --- src/allmydata/testing/web.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 7d9b5730b..64e615cd6 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -54,6 +54,9 @@ class _FakeTahoeRoot(Resource, object): """ def __init__(self, uri=None): + """ + :param uri: a Resource to handle the `/uri` tree. + """ Resource.__init__(self) # this is an old-style class :( self._uri = uri self.putChild(b"uri", self._uri) From 6fd0d61d5e2083b2141658c542897824b7682295 Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 12 Jun 2020 23:08:04 -0600 Subject: [PATCH 040/148] comment --- src/allmydata/testing/web.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 64e615cd6..c2bced12e 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -93,7 +93,7 @@ def capability_generator(kind): # different sequences. number = 0 key_hasher = hashlib.new("sha256") - ueb_hasher = hashlib.new("sha256") + ueb_hasher = hashlib.new("sha256") # ueb means "URI Extension Block" # capabilities are "prefix:<128-bits-base32>:<256-bits-base32>:N:K:size" while True: From 835341f2c24ac7f47b9ac315b695efa292d71124 Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 13 Jun 2020 00:39:12 -0600 Subject: [PATCH 041/148] more docstring --- src/allmydata/testing/web.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index c2bced12e..63f6b9877 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -74,6 +74,10 @@ KNOWN_CAPABILITIES = [ def capability_generator(kind): """ + Deterministically generates a streap of valid capabilities of the + given kind. The N, K and size values aren't related to anything + real. + :param str kind: the kind of capability, like `URI:CHK` :returns: a generator that yields new capablities of a particular From bd858f8fef6848abe0bc9d34f11073fb5c44762f Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 13 Jun 2020 00:39:57 -0600 Subject: [PATCH 042/148] assert -> raise --- src/allmydata/testing/web.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 63f6b9877..90b50d2e0 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -149,7 +149,8 @@ class _FakeTahoeUriHandler(Resource, object): :returns: a capability-string """ - assert isinstance(data, bytes) + if not isinstance(data, bytes): + raise TypeError("'data' must be bytes") cap = self._generate_capability(kind) if self._data is None: From 66f61c514b987c42b152ff2204aa86ed834b8f7d Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 13 Jun 2020 00:41:22 -0600 Subject: [PATCH 043/148] assert -> ValueError --- src/allmydata/testing/web.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 90b50d2e0..5e6d564fa 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -155,7 +155,8 @@ class _FakeTahoeUriHandler(Resource, object): cap = self._generate_capability(kind) if self._data is None: self._data = dict() - assert cap not in self._data, "already have '{}'".format(cap) + if cap in self._data: + raise ValueError("already have '{}'".format(cap)) self._data[cap] = data return cap From 0f6b4c33ac206d384e2d6adba0ad562e40ad3c59 Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 13 Jun 2020 00:46:55 -0600 Subject: [PATCH 044/148] match query args --- src/allmydata/testing/web.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 5e6d564fa..3231c5820 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -177,8 +177,16 @@ class _FakeTahoeUriHandler(Resource, object): def render_GET(self, request): uri = DecodedURL.from_text(request.uri.decode('utf8')) - # XXX FIXME - capability = uri.query[0][1] + capability = None + for arg, value in uri.query: + if arg == u"uri": + capability = value + if capability is None: + raise Exception( + "No ?uri= arguent in GET '{}'".format( + uri.to_string() + ) + ) if self._data is None or capability not in self._data: return u"No data for '{}'".format(capability).decode("ascii") From a3f4cc698327ee7e791cef853fe14528ff88e558 Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 13 Jun 2020 01:14:45 -0600 Subject: [PATCH 045/148] symbols, not magic-numbers --- src/allmydata/testing/web.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 3231c5820..5ab870f4f 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -23,6 +23,10 @@ from twisted.web.resource import ( from twisted.web.iweb import ( IBodyProducer, ) +from twisted.web import ( + http, +) + from twisted.internet.defer import ( succeed, ) @@ -162,7 +166,7 @@ class _FakeTahoeUriHandler(Resource, object): def render_PUT(self, request): data = request.content.read() - request.setResponseCode(201) # real code does this for brand-new files + request.setResponseCode(http.CREATED) # real code does this for brand-new files return self.add_data("URI:CHK:", data) def render_POST(self, request): From eaf96b22fef6a128c944e9b649aa32f55b9d8dce Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 13 Jun 2020 01:55:14 -0600 Subject: [PATCH 046/148] replciate Tahoe's behavior for replace=true/false --- src/allmydata/test/test_testing.py | 21 +++++++++++++++++++- src/allmydata/testing/web.py | 31 +++++++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index b5bbacad4..a995e756d 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -61,7 +61,7 @@ class FakeWebTest(TestCase): @inlineCallbacks def do_test(): - resp = yield self.http_client.put("http://example.com/uri", content) + resp = yield self.http_client.put("http://example.com/uri?replace=true", content) self.assertEqual(resp.code, 201) cap_raw = yield resp.content() @@ -79,3 +79,22 @@ class FakeWebTest(TestCase): do_test(), succeeded(Always()), ) + + @inlineCallbacks + def test_duplicate_upload(self): + """ + Upload the same content (via 'PUT /uri') twice with no overwrite + """ + + content = "fake content\n" * 200 + + resp = yield self.http_client.put("http://example.com/uri", content) + self.assertEqual(resp.code, 201) + + cap_raw = yield resp.content() + cap = from_string(cap_raw) + self.assertIsInstance(cap, CHKFileURI) + + # this one fails: same content but no ?replace=true + resp = yield self.http_client.put("http://example.com/uri", content) + self.assertEqual(resp.code, 409) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 5ab870f4f..ef7cd5bed 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -30,6 +30,9 @@ from twisted.web import ( from twisted.internet.defer import ( succeed, ) +from twisted.python.failure import ( + Failure, +) from treq.client import ( HTTPClient, @@ -44,6 +47,13 @@ import allmydata.uri from allmydata.util import ( base32, ) +from allmydata.interfaces import ( + ExistingChildError, +) +from allmydata.web.common import ( + humanize_failure, +) + __all__ = ( "create_fake_tahoe_root", @@ -147,7 +157,7 @@ class _FakeTahoeUriHandler(Resource, object): capability = next(self._capability_generators[kind]) return capability - def add_data(self, kind, data): + def add_data(self, kind, data, allow_duplicate=False): """ adds some data to our grid @@ -156,9 +166,18 @@ class _FakeTahoeUriHandler(Resource, object): if not isinstance(data, bytes): raise TypeError("'data' must be bytes") - cap = self._generate_capability(kind) if self._data is None: self._data = dict() + + for k in self._data: + if self._data[k] == data: + if allow_duplicate: + return k + raise ValueError( + "Duplicate data" + ) + + cap = self._generate_capability(kind) if cap in self._data: raise ValueError("already have '{}'".format(cap)) self._data[cap] = data @@ -167,7 +186,13 @@ class _FakeTahoeUriHandler(Resource, object): def render_PUT(self, request): data = request.content.read() request.setResponseCode(http.CREATED) # real code does this for brand-new files - return self.add_data("URI:CHK:", data) + replace = request.args.get("replace", None) + try: + return self.add_data("URI:CHK:", data, allow_duplicate=replace) + except ValueError: + msg, code = humanize_failure(Failure(ExistingChildError())) + request.setResponseCode(code) + return msg def render_POST(self, request): t = request.args[u"t"][0] From d91996aa827b15e4090b042c562377b52920bf6f Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 13 Jun 2020 02:03:19 -0600 Subject: [PATCH 047/148] better docstring --- src/allmydata/testing/web.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index ef7cd5bed..5f011f8b1 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -225,6 +225,14 @@ class _FakeTahoeUriHandler(Resource, object): def create_fake_tahoe_root(): """ + If you wish to pre-populate data into the fake Tahoe grid, retain + a reference to this root and pass it to + `create_tahoe_treq_client`. For example:: + + root = create_fake_tahoe_root() + root.add_data(...) + client = create_tahoe_treq_client(root) + :returns: an IResource instance that will handle certain Tahoe URI endpoints similar to a real Tahoe server. """ From ced9bdeb2590ec0121036ff35d4d81bff2462202 Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 13 Jun 2020 02:05:27 -0600 Subject: [PATCH 048/148] different words for news --- newsfragments/3317.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/3317.feature b/newsfragments/3317.feature index 5ed00acf5..2a7048397 100644 --- a/newsfragments/3317.feature +++ b/newsfragments/3317.feature @@ -1 +1 @@ -Verified fakes for (some of) Tahoe's WebUI via treq \ No newline at end of file +allmydata.testing.web, a new module, now offers a supported Python API for testing Tahoe-LAFS web API clients. \ No newline at end of file From af70911e2f38c1443a6e5e1e9144f39514c249f6 Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 13 Jun 2020 17:22:17 -0600 Subject: [PATCH 049/148] embetter docstring --- src/allmydata/testing/web.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 5f011f8b1..ca010afa4 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -226,11 +226,11 @@ class _FakeTahoeUriHandler(Resource, object): def create_fake_tahoe_root(): """ If you wish to pre-populate data into the fake Tahoe grid, retain - a reference to this root and pass it to + a reference to this root by creating it yourself and passing it to `create_tahoe_treq_client`. For example:: root = create_fake_tahoe_root() - root.add_data(...) + cap_string = root.add_data(...) client = create_tahoe_treq_client(root) :returns: an IResource instance that will handle certain Tahoe URI From e0287a7bfe654c40ce34b98d3af02d9cd8194d75 Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 13 Jun 2020 17:28:28 -0600 Subject: [PATCH 050/148] use attr.s --- src/allmydata/testing/web.py | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index ca010afa4..7c6fc1996 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -133,6 +133,7 @@ def capability_generator(kind): yield cap.encode("ascii") +@attr.s class _FakeTahoeUriHandler(Resource, object): """ An in-memory fake of (some of) the `/uri` endpoint of a Tahoe @@ -140,8 +141,9 @@ class _FakeTahoeUriHandler(Resource, object): """ isLeaf = True - _data = None - _capability_generators = None + + data = attr.ib(default=attr.Factory(dict)) + capability_generators = attr.ib(default=attr.Factory(dict)) def _generate_capability(self, kind): """ @@ -149,12 +151,9 @@ class _FakeTahoeUriHandler(Resource, object): :returns: the next capability-string for the given kind """ - if self._capability_generators is None: - self._capability_generators = dict() - - if kind not in self._capability_generators: - self._capability_generators[kind] = capability_generator(kind) - capability = next(self._capability_generators[kind]) + if kind not in self.capability_generators: + self.capability_generators[kind] = capability_generator(kind) + capability = next(self.capability_generators[kind]) return capability def add_data(self, kind, data, allow_duplicate=False): @@ -166,11 +165,8 @@ class _FakeTahoeUriHandler(Resource, object): if not isinstance(data, bytes): raise TypeError("'data' must be bytes") - if self._data is None: - self._data = dict() - - for k in self._data: - if self._data[k] == data: + for k in self.data: + if self.data[k] == data: if allow_duplicate: return k raise ValueError( @@ -178,9 +174,9 @@ class _FakeTahoeUriHandler(Resource, object): ) cap = self._generate_capability(kind) - if cap in self._data: + if cap in self.data: raise ValueError("already have '{}'".format(cap)) - self._data[cap] = data + self.data[cap] = data return cap def render_PUT(self, request): @@ -217,10 +213,10 @@ class _FakeTahoeUriHandler(Resource, object): ) ) - if self._data is None or capability not in self._data: + if capability not in self.data: return u"No data for '{}'".format(capability).decode("ascii") - return self._data[capability] + return self.data[capability] def create_fake_tahoe_root(): From 26d7a3a957faf35520ce349d235da840fa8b2c15 Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 19 Jun 2020 17:15:01 -0600 Subject: [PATCH 051/148] Get rid of ?replace= handling entirely and more-correctly support /uri variants --- src/allmydata/test/test_testing.py | 98 ++++++++++++++++++++++++------ src/allmydata/testing/web.py | 52 ++++++++-------- 2 files changed, 107 insertions(+), 43 deletions(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index a995e756d..67c38ed57 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -21,6 +21,11 @@ from allmydata.uri import ( ) from allmydata.testing.web import ( create_tahoe_treq_client, + capability_generator, +) + +from hyperlink import ( + DecodedURL, ) from hypothesis import ( @@ -35,9 +40,12 @@ from testtools import ( ) from testtools.matchers import ( Always, + Equals, + MatchesStructure, ) from testtools.twistedsupport import ( succeeded, + failed, ) @@ -46,9 +54,9 @@ class FakeWebTest(TestCase): Test the WebUI verified-fakes infrastucture """ - def setUp(self): - super(FakeWebTest, self).setUp() - self.http_client = create_tahoe_treq_client() + # Note: do NOT use setUp() because Hypothesis doesn't work + # properly with it. You must instead do all fixture-type work + # yourself in each test. @given( content=binary(), @@ -58,21 +66,31 @@ class FakeWebTest(TestCase): Upload some content (via 'PUT /uri') and then download it (via 'GET /uri?uri=...') """ + http_client = create_tahoe_treq_client() @inlineCallbacks def do_test(): - resp = yield self.http_client.put("http://example.com/uri?replace=true", content) + resp = yield http_client.put("http://example.com/uri", content) self.assertEqual(resp.code, 201) cap_raw = yield resp.content() cap = from_string(cap_raw) self.assertIsInstance(cap, CHKFileURI) - resp = yield self.http_client.get( + resp = yield http_client.get( "http://example.com/uri?uri={}".format(cap.to_string()) ) self.assertEqual(resp.code, 200) + round_trip_content = yield resp.content() + + # using the form "/uri/" is also valid + + resp = yield http_client.get( + "http://example.com/uri/{}".format(cap.to_string()) + ) + self.assertEqual(resp.code, 200) + round_trip_content = yield resp.content() self.assertEqual(content, round_trip_content) self.assertThat( @@ -80,21 +98,67 @@ class FakeWebTest(TestCase): succeeded(Always()), ) - @inlineCallbacks - def test_duplicate_upload(self): + @given( + content=binary(), + ) + def test_duplicate_upload(self, content): """ - Upload the same content (via 'PUT /uri') twice with no overwrite + Upload the same content (via 'PUT /uri') twice """ - content = "fake content\n" * 200 + http_client = create_tahoe_treq_client() - resp = yield self.http_client.put("http://example.com/uri", content) - self.assertEqual(resp.code, 201) + @inlineCallbacks + def do_test(): + resp = yield http_client.put("http://example.com/uri", content) + self.assertEqual(resp.code, 201) - cap_raw = yield resp.content() - cap = from_string(cap_raw) - self.assertIsInstance(cap, CHKFileURI) + cap_raw = yield resp.content() + cap = from_string(cap_raw) + self.assertIsInstance(cap, CHKFileURI) - # this one fails: same content but no ?replace=true - resp = yield self.http_client.put("http://example.com/uri", content) - self.assertEqual(resp.code, 409) + resp = yield http_client.put("http://example.com/uri", content) + self.assertThat(resp.code, Equals(200)) + self.assertThat( + do_test(), + succeeded(Always()), + ) + + def test_download_missing(self): + """ + Error if we download a capability that doesn't exist + """ + + http_client = create_tahoe_treq_client() + cap_gen = capability_generator("URI:CHK:") + + uri = DecodedURL.from_text(u"http://example.com/uri?uri={}".format(next(cap_gen))) + resp = http_client.get(uri.to_uri().to_text()) + + self.assertThat( + resp, + succeeded( + MatchesStructure( + code=Equals(500) + ) + ) + ) + + def test_download_no_arg(self): + """ + Error if we GET from "/uri" with no ?uri= query-arg + """ + + http_client = create_tahoe_treq_client() + + uri = DecodedURL.from_text(u"http://example.com/uri/") + resp = http_client.get(uri.to_uri().to_text()) + + self.assertThat( + resp, + succeeded( + MatchesStructure( + code=Equals(400) + ) + ) + ) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 7c6fc1996..aaa795d06 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -76,7 +76,8 @@ class _FakeTahoeRoot(Resource, object): self.putChild(b"uri", self._uri) def add_data(self, kind, data): - return self._uri.add_data(kind, data) + fresh, cap = self._uri.add_data(kind, data) + return cap KNOWN_CAPABILITIES = [ @@ -156,39 +157,32 @@ class _FakeTahoeUriHandler(Resource, object): capability = next(self.capability_generators[kind]) return capability - def add_data(self, kind, data, allow_duplicate=False): + def add_data(self, kind, data): """ adds some data to our grid - :returns: a capability-string + :returns: a two-tuple: a bool (True if the data is freshly added) and a capability-string """ if not isinstance(data, bytes): raise TypeError("'data' must be bytes") for k in self.data: if self.data[k] == data: - if allow_duplicate: - return k - raise ValueError( - "Duplicate data" - ) + return (False, k) cap = self._generate_capability(kind) - if cap in self.data: - raise ValueError("already have '{}'".format(cap)) + assert cap not in self.data, "logic error" # shouldn't happen self.data[cap] = data - return cap + return (True, cap) def render_PUT(self, request): data = request.content.read() - request.setResponseCode(http.CREATED) # real code does this for brand-new files - replace = request.args.get("replace", None) - try: - return self.add_data("URI:CHK:", data, allow_duplicate=replace) - except ValueError: - msg, code = humanize_failure(Failure(ExistingChildError())) - request.setResponseCode(code) - return msg + fresh, cap = self.add_data("URI:CHK:", data) + if fresh: + request.setResponseCode(http.CREATED) # real code does this for brand-new files + else: + request.setResponseCode(http.OK) # replaced/modified files + return cap def render_POST(self, request): t = request.args[u"t"][0] @@ -198,7 +192,8 @@ class _FakeTahoeUriHandler(Resource, object): "mkdir-immutable": "URI:DIR2-CHK:" } kind = type_to_kind[t] - return self.add_data(kind, data) + fresh, cap = self.add_data(kind, data) + return cap def render_GET(self, request): uri = DecodedURL.from_text(request.uri.decode('utf8')) @@ -206,14 +201,19 @@ class _FakeTahoeUriHandler(Resource, object): for arg, value in uri.query: if arg == u"uri": capability = value - if capability is None: - raise Exception( - "No ?uri= arguent in GET '{}'".format( - uri.to_string() - ) - ) + # it's legal to use the form "/uri/" + if capability is None and request.postpath and request.postpath[0]: + capability = request.postpath[0] + # if we don't yet have a capability, that's an error + if capability is None: + request.setResponseCode(http.BAD_REQUEST) + return b"GET /uri requires uri=" + + # the user gave us a capability; if our Grid doesn't have any + # data for it, that's an error. if capability not in self.data: + request.setResponseCode(http.BAD_REQUEST) return u"No data for '{}'".format(capability).decode("ascii") return self.data[capability] From 1ce760b2370ea156d680b0291f6fdf3d9b89a081 Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 19 Jun 2020 17:16:39 -0600 Subject: [PATCH 052/148] testtool's asserts --- src/allmydata/test/test_testing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index 67c38ed57..6ba622f3c 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -71,7 +71,7 @@ class FakeWebTest(TestCase): @inlineCallbacks def do_test(): resp = yield http_client.put("http://example.com/uri", content) - self.assertEqual(resp.code, 201) + self.assertThat(resp.code, Equals(201)) cap_raw = yield resp.content() cap = from_string(cap_raw) @@ -80,7 +80,7 @@ class FakeWebTest(TestCase): resp = yield http_client.get( "http://example.com/uri?uri={}".format(cap.to_string()) ) - self.assertEqual(resp.code, 200) + self.assertThat(resp.code, Equals(200)) round_trip_content = yield resp.content() From 77a2f11694c68cc0b07ee10f97550f4aadb01951 Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 19 Jun 2020 17:19:30 -0600 Subject: [PATCH 053/148] more-testtools-y asserts --- src/allmydata/test/test_testing.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index 6ba622f3c..37a5f3c7b 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -41,7 +41,9 @@ from testtools import ( from testtools.matchers import ( Always, Equals, + IsInstance, MatchesStructure, + AfterPreprocessing, ) from testtools.twistedsupport import ( succeeded, @@ -75,7 +77,7 @@ class FakeWebTest(TestCase): cap_raw = yield resp.content() cap = from_string(cap_raw) - self.assertIsInstance(cap, CHKFileURI) + self.assertThat(cap, IsInstance(CHKFileURI)) resp = yield http_client.get( "http://example.com/uri?uri={}".format(cap.to_string()) @@ -114,8 +116,13 @@ class FakeWebTest(TestCase): self.assertEqual(resp.code, 201) cap_raw = yield resp.content() - cap = from_string(cap_raw) - self.assertIsInstance(cap, CHKFileURI) + self.assertThat( + cap_raw, + AfterPreprocessing( + from_string, + IsInstance(CHKFileURI) + ) + ) resp = yield http_client.put("http://example.com/uri", content) self.assertThat(resp.code, Equals(200)) From 1f4b619fda37cab02762c1f50dff74cba0f33c18 Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 19 Jun 2020 17:22:42 -0600 Subject: [PATCH 054/148] unused imports --- src/allmydata/test/test_testing.py | 1 - src/allmydata/testing/web.py | 9 --------- 2 files changed, 10 deletions(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index 37a5f3c7b..ec6935914 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -47,7 +47,6 @@ from testtools.matchers import ( ) from testtools.twistedsupport import ( succeeded, - failed, ) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index aaa795d06..6656236ca 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -30,9 +30,6 @@ from twisted.web import ( from twisted.internet.defer import ( succeed, ) -from twisted.python.failure import ( - Failure, -) from treq.client import ( HTTPClient, @@ -47,12 +44,6 @@ import allmydata.uri from allmydata.util import ( base32, ) -from allmydata.interfaces import ( - ExistingChildError, -) -from allmydata.web.common import ( - humanize_failure, -) __all__ = ( From 5cfb35077185d2d87ab3b5b13d8ef61bf89d8933 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 30 Jun 2020 08:15:58 -0600 Subject: [PATCH 055/148] streap -> stream --- src/allmydata/testing/web.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 6656236ca..0d6e7cb6e 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -80,7 +80,7 @@ KNOWN_CAPABILITIES = [ def capability_generator(kind): """ - Deterministically generates a streap of valid capabilities of the + Deterministically generates a stream of valid capabilities of the given kind. The N, K and size values aren't related to anything real. From 7a37eeee678d5765d85cbd52e6c6515c90b6a538 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 30 Jun 2020 08:17:34 -0600 Subject: [PATCH 056/148] assert -> Exception --- src/allmydata/testing/web.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 0d6e7cb6e..12f5ef40c 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -162,7 +162,10 @@ class _FakeTahoeUriHandler(Resource, object): return (False, k) cap = self._generate_capability(kind) - assert cap not in self.data, "logic error" # shouldn't happen + # it should be impossible for this to already be in our data, + # but check anyway to be sure + if cap in self.data: + raise Exception("Internal error; key already exists somehow") self.data[cap] = data return (True, cap) From aad3557d2b6452dc1c0245c080c3df2062c9c06b Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 4 Feb 2020 06:46:08 -0500 Subject: [PATCH 057/148] Use twisted.web.template in web/storage.py Related to ticket:3247. Nevow usage has been removed, and generated page looks the same as its former self, but tests are failing because test_storage.py assumes that we're using nevow. --- src/allmydata/web/storage.py | 258 ++++++++++++++----------- src/allmydata/web/storage_status.xhtml | 60 +++--- 2 files changed, 182 insertions(+), 136 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 79c0a8c38..3000fb70d 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -1,10 +1,10 @@ import time, json -from nevow import rend, tags as T +from twisted.python.filepath import FilePath +from twisted.web.template import tags as T, renderer, Element, renderElement, XMLFile from allmydata.web.common import ( - getxmlfile, abbreviate_time, - MultiFormatPage, + MultiFormatResource ) from allmydata.util.abbreviate import abbreviate_space from allmydata.util import time_format, idlib @@ -16,91 +16,100 @@ def remove_prefix(s, prefix): return s[len(prefix):] -class StorageStatus(MultiFormatPage): - docFactory = getxmlfile("storage_status.xhtml") - # the default 'data' argument is the StorageServer instance +class StorageStatusElement(Element): + loader = XMLFile(FilePath(__file__).sibling("storage_status.xhtml")) - def __init__(self, storage, nickname=""): - rend.Page.__init__(self, storage) + def __init__(self, storage, nickname): + super(StorageStatusElement, self).__init__() self.storage = storage - self.nickname = nickname + self.nick = nickname - def render_JSON(self, req): - req.setHeader("content-type", "text/plain") - d = {"stats": self.storage.get_stats(), - "bucket-counter": self.storage.bucket_counter.get_state(), - "lease-checker": self.storage.lease_checker.get_state(), - "lease-checker-progress": self.storage.lease_checker.get_progress(), - } - return json.dumps(d, indent=1) + "\n" + @renderer + def nickname(self, req, tag): + return self.nick - def data_nickname(self, ctx, storage): - return self.nickname - def data_nodeid(self, ctx, storage): + @renderer + def nodeid(self, req, tag): return idlib.nodeid_b2a(self.storage.my_nodeid) - def render_storage_running(self, ctx, storage): - if storage: - return ctx.tag - else: - return T.h1["No Storage Server Running"] + def get_stat(self, key): + return self.storage.get_stats().get(key) - def render_bool(self, ctx, data): - return {True: "Yes", False: "No"}[bool(data)] + def str(self, tag, val): + if val is None: + return tag("?") + return tag(str(val)) - def render_abbrev_space(self, ctx, size): - if size is None: - return "?" - return abbreviate_space(size) + def abbr(self, tag, val): + if val is None: + return tag("?") + return tag(abbreviate_space(val)) - def render_space(self, ctx, size): - if size is None: - return "?" - return "%d" % size + @renderer + def disk_total(self, req, tag): + return self.str(tag, self.get_stat("storage_server.disk_total")) - def data_stats(self, ctx, data): - # FYI: 'data' appears to be self, rather than the StorageServer - # object in self.original that gets passed to render_* methods. I - # still don't understand Nevow. + @renderer + def disk_total_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.disk_total")) - # Nevow has nevow.accessors.DictionaryContainer: Any data= directive - # that appears in a context in which the current data is a dictionary - # will be looked up as keys in that dictionary. So if data_stats() - # returns a dictionary, then we can use something like this: - # - #
    - #
  • disk_total:
    • - #
+ @renderer + def disk_used(self, req, tag): + return self.str(tag, self.get_stat("storage_server.disk_used")) - # to use get_stats()["storage_server.disk_total"] . However, - # DictionaryContainer does a raw d[] instead of d.get(), so any - # missing keys will cause an error, even if the renderer can tolerate - # None values. To overcome this, we either need a dict-like object - # that always returns None for unknown keys, or we must pre-populate - # our dict with those missing keys, or we should get rid of data_ - # methods that return dicts (or find some way to override Nevow's - # handling of dictionaries). + @renderer + def disk_used_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.disk_used")) - d = dict([ (remove_prefix(k, "storage_server."), v) - for k,v in self.storage.get_stats().items() ]) - d.setdefault("disk_total", None) - d.setdefault("disk_used", None) - d.setdefault("disk_free_for_root", None) - d.setdefault("disk_free_for_nonroot", None) - d.setdefault("reserved_space", None) - d.setdefault("disk_avail", None) - return d + @renderer + def disk_free_for_root(self, req, tag): + return self.str(tag, self.get_stat("storage_server.disk_free_for_root")) - def data_last_complete_bucket_count(self, ctx, data): + @renderer + def disk_free_for_root_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.disk_free_for_root")) + + @renderer + def disk_free_for_nonroot(self, req, tag): + return self.str(tag, self.get_stat("storage_server.disk_free_for_nonroot")) + + @renderer + def disk_free_for_nonroot_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.disk_free_for_nonroot")) + + @renderer + def reserved_space(self, req, tag): + return self.str(tag, self.get_stat("storage_server.reserved_space")) + + @renderer + def reserved_space_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.reserved_space")) + + @renderer + def disk_avail(self, req, tag): + return self.str(tag, self.get_stat("storage_server.disk_avail")) + + @renderer + def disk_avail_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.disk_avail")) + + @renderer + def accepting_immutable_shares(self, req, tag): + accepting = self.get_stat("storage_server.accepting_immutable_shares") + return {True: "Yes", False: "No"}[bool(accepting)] + + @renderer + def last_complete_bucket_count(self, req, tag): s = self.storage.bucket_counter.get_state() count = s.get("last-complete-bucket-count") if count is None: return "Not computed yet" - return count + return str(count) - def render_count_crawler_status(self, ctx, storage): + @renderer + def count_crawler_status(self, req, tag): p = self.storage.bucket_counter.get_progress() - return ctx.tag[self.format_crawler_progress(p)] + return self.format_crawler_progress(p) def format_crawler_progress(self, p): cycletime = p["estimated-time-per-cycle"] @@ -127,55 +136,51 @@ class StorageStatus(MultiFormatPage): return ["Next crawl in %s" % abbreviate_time(soon), cycletime_s] - def render_lease_expiration_enabled(self, ctx, data): + @renderer + def storage_running(self, req, tag): + if self.storage: + return tag + return tag("No Storage Server Running") + + @renderer + def lease_expiration_enabled(self, req, tag): lc = self.storage.lease_checker if lc.expiration_enabled: - return ctx.tag["Enabled: expired leases will be removed"] + return tag("Enabled: expired leases will be removed") else: - return ctx.tag["Disabled: scan-only mode, no leases will be removed"] + return tag("Disabled: scan-only mode, no leases will be removed") - def render_lease_expiration_mode(self, ctx, data): + @renderer + def lease_expiration_mode(self, req, tag): lc = self.storage.lease_checker if lc.mode == "age": if lc.override_lease_duration is None: - ctx.tag["Leases will expire naturally, probably 31 days after " - "creation or renewal."] + tag("Leases will expire naturally, probably 31 days after " + "creation or renewal.") else: - ctx.tag["Leases created or last renewed more than %s ago " - "will be considered expired." - % abbreviate_time(lc.override_lease_duration)] + tag("Leases created or last renewed more than %s ago " + "will be considered expired." + % abbreviate_time(lc.override_lease_duration)) else: assert lc.mode == "cutoff-date" localizedutcdate = time.strftime("%d-%b-%Y", time.gmtime(lc.cutoff_date)) isoutcdate = time_format.iso_utc_date(lc.cutoff_date) - ctx.tag["Leases created or last renewed before %s (%s) UTC " - "will be considered expired." % (isoutcdate, localizedutcdate, )] + tag("Leases created or last renewed before %s (%s) UTC " + "will be considered expired." + % (isoutcdate, localizedutcdate, )) if len(lc.mode) > 2: - ctx.tag[" The following sharetypes will be expired: ", - " ".join(sorted(lc.sharetypes_to_expire)), "."] - return ctx.tag + tag(" The following sharetypes will be expired: ", + " ".join(sorted(lc.sharetypes_to_expire)), ".") + return tag - def format_recovered(self, sr, a): - def maybe(d): - if d is None: - return "?" - return "%d" % d - return "%s shares, %s buckets (%s mutable / %s immutable), %s (%s / %s)" % \ - (maybe(sr["%s-shares" % a]), - maybe(sr["%s-buckets" % a]), - maybe(sr["%s-buckets-mutable" % a]), - maybe(sr["%s-buckets-immutable" % a]), - abbreviate_space(sr["%s-diskbytes" % a]), - abbreviate_space(sr["%s-diskbytes-mutable" % a]), - abbreviate_space(sr["%s-diskbytes-immutable" % a]), - ) - - def render_lease_current_cycle_progress(self, ctx, data): + @renderer + def lease_current_cycle_progress(self, req, tag): lc = self.storage.lease_checker p = lc.get_progress() - return ctx.tag[self.format_crawler_progress(p)] + return tag(self.format_crawler_progress(p)) - def render_lease_current_cycle_results(self, ctx, data): + @renderer + def lease_current_cycle_results(self, req, tag): lc = self.storage.lease_checker p = lc.get_progress() if not p["cycle-in-progress"]: @@ -229,10 +234,10 @@ class StorageStatus(MultiFormatPage): T.ul[ [T.li[ ["SI %s shnum %d" % corrupt_share for corrupt_share in so_far["corrupt-shares"] ] ]]]) + return tag("Current cycle:", p) - return ctx.tag["Current cycle:", p] - - def render_lease_last_cycle_results(self, ctx, data): + @renderer + def lease_last_cycle_results(self, req, tag): lc = self.storage.lease_checker h = lc.get_state()["history"] if not h: @@ -240,15 +245,15 @@ class StorageStatus(MultiFormatPage): last = h[max(h.keys())] start, end = last["cycle-start-finish-times"] - ctx.tag["Last complete cycle (which took %s and finished %s ago)" - " recovered: " % (abbreviate_time(end-start), - abbreviate_time(time.time() - end)), - self.format_recovered(last["space-recovered"], "actual") - ] + tag("Last complete cycle (which took %s and finished %s ago)" + " recovered: " % (abbreviate_time(end-start), + abbreviate_time(time.time() - end)), + self.format_recovered(last["space-recovered"], "actual")) p = T.ul() + def add(*pieces): - p[T.li[pieces]] + p(T.li(pieces)) saw = self.format_recovered(last["space-recovered"], "examined") add("and saw a total of ", saw) @@ -264,4 +269,37 @@ class StorageStatus(MultiFormatPage): for corrupt_share in last["corrupt-shares"] ] ]]]) - return ctx.tag[p] + return tag(p) + + def format_recovered(self, sr, a): + def maybe(d): + if d is None: + return "?" + return "%d" % d + return "%s shares, %s buckets (%s mutable / %s immutable), %s (%s / %s)" % \ + (maybe(sr["%s-shares" % a]), + maybe(sr["%s-buckets" % a]), + maybe(sr["%s-buckets-mutable" % a]), + maybe(sr["%s-buckets-immutable" % a]), + abbreviate_space(sr["%s-diskbytes" % a]), + abbreviate_space(sr["%s-diskbytes-mutable" % a]), + abbreviate_space(sr["%s-diskbytes-immutable" % a]), + ) + +class StorageStatus(MultiFormatResource): + def __init__(self, storage, nickname=""): + super(StorageStatus, self).__init__() + self.storage = storage + self.nickname = nickname + + def render_HTML(self, req): + return renderElement(req, StorageStatusElement(self.storage, self.nickname)) + + def render_JSON(self, req): + req.setHeader("content-type", "text/plain") + d = {"stats": self.storage.get_stats(), + "bucket-counter": self.storage.bucket_counter.get_state(), + "lease-checker": self.storage.lease_checker.get_state(), + "lease-checker-progress": self.storage.lease_checker.get_progress(), + } + return json.dumps(d, indent=1) + "\n" diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index d97daf9af..cfd7a860c 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -1,4 +1,4 @@ - + Tahoe-LAFS - Storage Server Status @@ -7,19 +7,19 @@ -
+

Storage Server Status

- +
- - + + - - + + - - + + - - + + - - + + - - + +
Total disk space:()()
Disk space used:- ()- ()
@@ -28,18 +28,18 @@
Disk space free (root):()() [see 1]
Disk space free (non-root):()() [see 2]
Reserved space:- ()- ()
@@ -48,23 +48,31 @@
Space Available to Tahoe:()()
    -
  • Server Nickname:
    • -
  • Server Nodeid:
    • -
  • Accepting new shares: -
    • +
  • Server Nickname: + + + +
    • +
  • Server Nodeid: + + + +
    • +
  • Accepting new shares: +
  • Total buckets: - + (the number of files and directories for which this server is holding a share)
      -
    • +
@@ -72,11 +80,11 @@

Lease Expiration Crawler

    -
  • Expiration
    • -
  • -
  • -
  • -
  • +
  • Expiration
    • +
  • +
  • +
  • +
From 75488c4aff704f84ad061230ab18ca2f48dca0a1 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 4 Feb 2020 22:59:45 -0500 Subject: [PATCH 058/148] Add `StorageStatus::renderSynchronously` Related to ticket:3247 test_storage.py wants a `StorageStatus::renderSynchronously()` method and a `StorageStatus::renderHTTP()` method. Let us begin with the goofy first-cut. Both these methods are not only wrong, but they will also not please the test suite. However error messages produced in CI can be shared, and that way I can hopefully get unstuck. --- src/allmydata/web/storage.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 3000fb70d..8f36cf44d 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -1,7 +1,8 @@ import time, json from twisted.python.filepath import FilePath -from twisted.web.template import tags as T, renderer, Element, renderElement, XMLFile +from twisted.web.template import tags as T, \ + renderer, Element, renderElement, XMLFile from allmydata.web.common import ( abbreviate_time, MultiFormatResource @@ -303,3 +304,14 @@ class StorageStatus(MultiFormatResource): "lease-checker-progress": self.storage.lease_checker.get_progress(), } return json.dumps(d, indent=1) + "\n" + + def renderSynchronously(self): + # to appease the test suite. + elem = StorageStatusElement(self.storage, self.nickname) + result = [] + flattenString(None, elem).addCallback(result.append) + return result + + def renderHTTP(self, ctx=None): + # to appease the test suite. + self.renderSynchronously() From 805b880c2eee1c46abf98728991e12213a5f35c1 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 4 Feb 2020 23:16:48 -0500 Subject: [PATCH 059/148] Add missing `flattenString` import --- src/allmydata/web/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 8f36cf44d..a912d79f6 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -2,7 +2,7 @@ import time, json from twisted.python.filepath import FilePath from twisted.web.template import tags as T, \ - renderer, Element, renderElement, XMLFile + renderer, Element, renderElement, XMLFile, flattenString from allmydata.web.common import ( abbreviate_time, MultiFormatResource From d1f17e1019f71dfa5a514fdf71dcd169e0c4b97b Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 5 Feb 2020 22:05:33 -0500 Subject: [PATCH 060/148] Second version of renderSynchronously --- src/allmydata/web/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index a912d79f6..4ad5a2baf 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -310,7 +310,7 @@ class StorageStatus(MultiFormatResource): elem = StorageStatusElement(self.storage, self.nickname) result = [] flattenString(None, elem).addCallback(result.append) - return result + return result[0] def renderHTTP(self, ctx=None): # to appease the test suite. From 42c3e276a351f6cb62b6f8617cbb2ba11f77af7f Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 5 Feb 2020 22:09:16 -0500 Subject: [PATCH 061/148] Check storage server status before using it --- src/allmydata/web/storage.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 4ad5a2baf..9a99286bf 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -31,9 +31,13 @@ class StorageStatusElement(Element): @renderer def nodeid(self, req, tag): + if not self.storage: + return tag("No storage server running.") return idlib.nodeid_b2a(self.storage.my_nodeid) def get_stat(self, key): + if not self.storage: + return None return self.storage.get_stats().get(key) def str(self, tag, val): @@ -101,6 +105,8 @@ class StorageStatusElement(Element): @renderer def last_complete_bucket_count(self, req, tag): + if not self.storage: + return tag("No storage server running.") s = self.storage.bucket_counter.get_state() count = s.get("last-complete-bucket-count") if count is None: @@ -109,6 +115,8 @@ class StorageStatusElement(Element): @renderer def count_crawler_status(self, req, tag): + if not self.storage: + return tag("No storage server running.") p = self.storage.bucket_counter.get_progress() return self.format_crawler_progress(p) @@ -145,6 +153,8 @@ class StorageStatusElement(Element): @renderer def lease_expiration_enabled(self, req, tag): + if not self.storage: + return tag("No storage server running.") lc = self.storage.lease_checker if lc.expiration_enabled: return tag("Enabled: expired leases will be removed") @@ -153,6 +163,8 @@ class StorageStatusElement(Element): @renderer def lease_expiration_mode(self, req, tag): + if not self.storage: + return tag("No storage server running.") lc = self.storage.lease_checker if lc.mode == "age": if lc.override_lease_duration is None: @@ -176,12 +188,16 @@ class StorageStatusElement(Element): @renderer def lease_current_cycle_progress(self, req, tag): + if not self.storage: + return tag("No storage server running.") lc = self.storage.lease_checker p = lc.get_progress() return tag(self.format_crawler_progress(p)) @renderer def lease_current_cycle_results(self, req, tag): + if not self.storage: + return tag("No storage server running.") lc = self.storage.lease_checker p = lc.get_progress() if not p["cycle-in-progress"]: @@ -239,6 +255,8 @@ class StorageStatusElement(Element): @renderer def lease_last_cycle_results(self, req, tag): + if not self.storage: + return tag("No storage server running.") lc = self.storage.lease_checker h = lc.get_state()["history"] if not h: From 923219e54a850e678c0e512ddac8b7614e4f08e4 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 5 Feb 2020 23:09:39 -0500 Subject: [PATCH 062/148] Use right syntax for twisted.web.template tags --- src/allmydata/web/storage.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 9a99286bf..1e206d8cf 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -212,7 +212,7 @@ class StorageStatusElement(Element): p = T.ul() def add(*pieces): - p[T.li[pieces]] + p(T.li(pieces)) def maybe(d): if d is None: @@ -248,9 +248,9 @@ class StorageStatusElement(Element): if so_far["corrupt-shares"]: add("Corrupt shares:", - T.ul[ [T.li[ ["SI %s shnum %d" % corrupt_share + T.ul( (T.li( ["SI %s shnum %d" % corrupt_share for corrupt_share in so_far["corrupt-shares"] ] - ]]]) + )))) return tag("Current cycle:", p) @renderer @@ -284,9 +284,9 @@ class StorageStatusElement(Element): if last["corrupt-shares"]: add("Corrupt shares:", - T.ul[ [T.li[ ["SI %s shnum %d" % corrupt_share + T.ul( (T.li( ["SI %s shnum %d" % corrupt_share for corrupt_share in last["corrupt-shares"] ] - ]]]) + )))) return tag(p) From d5af7c0d2e9492302db57a41de23c869ea5a1a52 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 05:17:35 -0500 Subject: [PATCH 063/148] Refactor so that test_util pass --- src/allmydata/test/test_storage.py | 5 +++-- src/allmydata/web/storage.py | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 73c183a38..56dba406e 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -38,7 +38,8 @@ from allmydata.interfaces import BadWriteEnablerError from allmydata.test.common import LoggingServiceParent, ShouldFailMixin from allmydata.test.common_web import WebRenderingMixin from allmydata.test.no_network import NoNetworkServer -from allmydata.web.storage import StorageStatus, remove_prefix +from allmydata.web.storage import StorageStatus, StorageStatusElement, \ + remove_prefix from allmydata.storage_client import ( _StorageServer, ) @@ -4221,7 +4222,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnlessIn("Reserved space: - 10.00 MB (10000000)", s) def test_util(self): - w = StorageStatus(None) + w = StorageStatusElement(None, None) self.failUnlessEqual(w.render_space(None, None), "?") self.failUnlessEqual(w.render_space(None, 10e6), "10000000") self.failUnlessEqual(w.render_abbrev_space(None, None), "?") diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 1e206d8cf..6b033bc5a 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -50,6 +50,16 @@ class StorageStatusElement(Element): return tag("?") return tag(abbreviate_space(val)) + def render_abbrev_space(self, ctx, size): + if size is None: + return "?" + return abbreviate_space(size) + + def render_space(self, ctx, size): + if size is None: + return "?" + return "%d" % size + @renderer def disk_total(self, req, tag): return self.str(tag, self.get_stat("storage_server.disk_total")) From dbc6b2afa00fb2b0e8be59b745f8b75366a02f01 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 05:20:45 -0500 Subject: [PATCH 064/148] Drop unused `ctx` argument from render_space methods --- src/allmydata/test/test_storage.py | 8 ++++---- src/allmydata/web/storage.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 56dba406e..b7d1b6e9f 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -4223,9 +4223,9 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): def test_util(self): w = StorageStatusElement(None, None) - self.failUnlessEqual(w.render_space(None, None), "?") - self.failUnlessEqual(w.render_space(None, 10e6), "10000000") - self.failUnlessEqual(w.render_abbrev_space(None, None), "?") - self.failUnlessEqual(w.render_abbrev_space(None, 10e6), "10.00 MB") + self.failUnlessEqual(w.render_space(None), "?") + self.failUnlessEqual(w.render_space(10e6), "10000000") + self.failUnlessEqual(w.render_abbrev_space(None), "?") + self.failUnlessEqual(w.render_abbrev_space(10e6), "10.00 MB") self.failUnlessEqual(remove_prefix("foo.bar", "foo."), "bar") self.failUnlessEqual(remove_prefix("foo.bar", "baz."), None) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 6b033bc5a..88f099b24 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -50,12 +50,12 @@ class StorageStatusElement(Element): return tag("?") return tag(abbreviate_space(val)) - def render_abbrev_space(self, ctx, size): + def render_abbrev_space(self, size): if size is None: return "?" return abbreviate_space(size) - def render_space(self, ctx, size): + def render_space(self, size): if size is None: return "?" return "%d" % size From 6ccb9d29cbfc49efa1fd594fed802df2ba2f6a01 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 05:49:11 -0500 Subject: [PATCH 065/148] Use render_space methods to render space --- src/allmydata/web/storage.py | 46 +++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 88f099b24..496766b92 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -40,16 +40,6 @@ class StorageStatusElement(Element): return None return self.storage.get_stats().get(key) - def str(self, tag, val): - if val is None: - return tag("?") - return tag(str(val)) - - def abbr(self, tag, val): - if val is None: - return tag("?") - return tag(abbreviate_space(val)) - def render_abbrev_space(self, size): if size is None: return "?" @@ -62,51 +52,63 @@ class StorageStatusElement(Element): @renderer def disk_total(self, req, tag): - return self.str(tag, self.get_stat("storage_server.disk_total")) + val = self.get_stat("storage_server.disk_total") + return tag(self.render_space(val)) @renderer def disk_total_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.disk_total")) + val = self.get_stat("storage_server.disk_total") + return tag(self.render_abbrev_space(val)) @renderer def disk_used(self, req, tag): - return self.str(tag, self.get_stat("storage_server.disk_used")) + val = self.get_stat("storage_server.disk_used") + return tag(self.render_space(val)) @renderer def disk_used_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.disk_used")) + val = self.get_stat("storage_server.disk_used") + return tag(self.render_abbrev_space(val)) @renderer def disk_free_for_root(self, req, tag): - return self.str(tag, self.get_stat("storage_server.disk_free_for_root")) + val = self.get_stat("storage_server.disk_free_for_root") + return tag(self.render_space(val)) @renderer def disk_free_for_root_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.disk_free_for_root")) + val = self.get_stat("storage_server.disk_free_for_root") + return tag(self.render_abbrev_space(val)) @renderer def disk_free_for_nonroot(self, req, tag): - return self.str(tag, self.get_stat("storage_server.disk_free_for_nonroot")) + val = self.get_stat("storage_server.disk_free_for_nonroot") + return tag(self.render_space(val)) @renderer def disk_free_for_nonroot_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.disk_free_for_nonroot")) + val = self.get_stat("storage_server.disk_free_for_nonroot") + return tag(self.render_abbrev_space(val)) @renderer def reserved_space(self, req, tag): - return self.str(tag, self.get_stat("storage_server.reserved_space")) + val = self.get_stat("storage_server.reserved_space") + return tag(self.render_space(val)) @renderer def reserved_space_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.reserved_space")) + val = self.get_stat("storage_server.reserved_space") + return tag(self.render_abbrev_space(val)) @renderer def disk_avail(self, req, tag): - return self.str(tag, self.get_stat("storage_server.disk_avail")) + val = self.get_stat("storage_server.disk_avail") + return tag(self.render_space(val)) @renderer def disk_avail_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.disk_avail")) + val = self.get_stat("storage_server.disk_avail") + return tag(self.render_abbrev_space(val)) @renderer def accepting_immutable_shares(self, req, tag): From 8dd4029428d352662452d4521b0ab1f38b391181 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 06:22:25 -0500 Subject: [PATCH 066/148] Use within table cells --- src/allmydata/web/storage_status.xhtml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index cfd7a860c..6e0e26008 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -13,13 +13,13 @@ - - + + - - + + - - + + - - + + - - + + - - + +
Total disk space:()()
Disk space used:- ()- ()
@@ -28,18 +28,18 @@
Disk space free (root):()() [see 1]
Disk space free (non-root):()() [see 2]
Reserved space:- ()- ()
@@ -48,8 +48,8 @@
Space Available to Tahoe:()()
From 1be736e4d1c04a04028253630e1b4f8db9437a57 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 07:34:55 -0500 Subject: [PATCH 067/148] Fix test_storage.WebStatus.test_no_server failure --- src/allmydata/web/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 496766b92..c2e517dce 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -161,7 +161,7 @@ class StorageStatusElement(Element): def storage_running(self, req, tag): if self.storage: return tag - return tag("No Storage Server Running") + return T.h1("No Storage Server Running") @renderer def lease_expiration_enabled(self, req, tag): From 62a997ce205e08468efb622f152d4168b5eceab6 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 08:19:29 -0500 Subject: [PATCH 068/148] Use parentheses in import statement --- src/allmydata/web/storage.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index c2e517dce..d45541457 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -1,8 +1,14 @@ import time, json from twisted.python.filepath import FilePath -from twisted.web.template import tags as T, \ - renderer, Element, renderElement, XMLFile, flattenString +from twisted.web.template import ( + Element, + XMLFile, + tags as T, + renderer, + renderElement, + flattenString +) from allmydata.web.common import ( abbreviate_time, MultiFormatResource From ac9a73eddb39a81c2548ba32c45433eaae9d482c Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 09:55:33 -0500 Subject: [PATCH 069/148] Use parentheses in test suite's import statement --- src/allmydata/test/test_storage.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index b7d1b6e9f..a507596e8 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -38,8 +38,11 @@ from allmydata.interfaces import BadWriteEnablerError from allmydata.test.common import LoggingServiceParent, ShouldFailMixin from allmydata.test.common_web import WebRenderingMixin from allmydata.test.no_network import NoNetworkServer -from allmydata.web.storage import StorageStatus, StorageStatusElement, \ +from allmydata.web.storage import ( + StorageStatus, + StorageStatusElement, remove_prefix +) from allmydata.storage_client import ( _StorageServer, ) From e67fa7cd95e590f88cda1d69df2d7154155932ef Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 17:02:14 -0500 Subject: [PATCH 070/148] Add docstrings to StorageStatusElement --- src/allmydata/web/storage.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index d45541457..df8354a38 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -24,9 +24,15 @@ def remove_prefix(s, prefix): class StorageStatusElement(Element): + """Class to render a storage status page.""" + loader = XMLFile(FilePath(__file__).sibling("storage_status.xhtml")) - def __init__(self, storage, nickname): + def __init__(self, storage, nickname=""): + """ + :param _StorageServer storage: data about storage. + :param string nickname: friendly name for storage. + """ super(StorageStatusElement, self).__init__() self.storage = storage self.nick = nickname From abb8834be1930f85cffe254ba9ad422f8668153c Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 12 Feb 2020 12:14:24 -0500 Subject: [PATCH 071/148] Rename function for clarity --- src/allmydata/web/storage.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index df8354a38..a5d65fa85 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -47,7 +47,7 @@ class StorageStatusElement(Element): return tag("No storage server running.") return idlib.nodeid_b2a(self.storage.my_nodeid) - def get_stat(self, key): + def _get_storage_stat(self, key): if not self.storage: return None return self.storage.get_stats().get(key) @@ -64,67 +64,67 @@ class StorageStatusElement(Element): @renderer def disk_total(self, req, tag): - val = self.get_stat("storage_server.disk_total") + val = self._get_storage_stat("storage_server.disk_total") return tag(self.render_space(val)) @renderer def disk_total_abbrev(self, req, tag): - val = self.get_stat("storage_server.disk_total") + val = self._get_storage_stat("storage_server.disk_total") return tag(self.render_abbrev_space(val)) @renderer def disk_used(self, req, tag): - val = self.get_stat("storage_server.disk_used") + val = self._get_storage_stat("storage_server.disk_used") return tag(self.render_space(val)) @renderer def disk_used_abbrev(self, req, tag): - val = self.get_stat("storage_server.disk_used") + val = self._get_storage_stat("storage_server.disk_used") return tag(self.render_abbrev_space(val)) @renderer def disk_free_for_root(self, req, tag): - val = self.get_stat("storage_server.disk_free_for_root") + val = self._get_storage_stat("storage_server.disk_free_for_root") return tag(self.render_space(val)) @renderer def disk_free_for_root_abbrev(self, req, tag): - val = self.get_stat("storage_server.disk_free_for_root") + val = self._get_storage_stat("storage_server.disk_free_for_root") return tag(self.render_abbrev_space(val)) @renderer def disk_free_for_nonroot(self, req, tag): - val = self.get_stat("storage_server.disk_free_for_nonroot") + val = self._get_storage_stat("storage_server.disk_free_for_nonroot") return tag(self.render_space(val)) @renderer def disk_free_for_nonroot_abbrev(self, req, tag): - val = self.get_stat("storage_server.disk_free_for_nonroot") + val = self._get_storage_stat("storage_server.disk_free_for_nonroot") return tag(self.render_abbrev_space(val)) @renderer def reserved_space(self, req, tag): - val = self.get_stat("storage_server.reserved_space") + val = self._get_storage_stat("storage_server.reserved_space") return tag(self.render_space(val)) @renderer def reserved_space_abbrev(self, req, tag): - val = self.get_stat("storage_server.reserved_space") + val = self._get_storage_stat("storage_server.reserved_space") return tag(self.render_abbrev_space(val)) @renderer def disk_avail(self, req, tag): - val = self.get_stat("storage_server.disk_avail") + val = self._get_storage_stat("storage_server.disk_avail") return tag(self.render_space(val)) @renderer def disk_avail_abbrev(self, req, tag): - val = self.get_stat("storage_server.disk_avail") + val = self._get_storage_stat("storage_server.disk_avail") return tag(self.render_abbrev_space(val)) @renderer def accepting_immutable_shares(self, req, tag): - accepting = self.get_stat("storage_server.accepting_immutable_shares") + accepting = self._get_storage_stat("storage_server.accepting_immutable_shares") return {True: "Yes", False: "No"}[bool(accepting)] @renderer From 6b5e530cb381b74d23fd1ee611e5be2bd9feb1b4 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 12 Feb 2020 12:15:26 -0500 Subject: [PATCH 072/148] Add a docstring --- src/allmydata/web/storage.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index a5d65fa85..263fa9344 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -48,6 +48,30 @@ class StorageStatusElement(Element): return idlib.nodeid_b2a(self.storage.my_nodeid) def _get_storage_stat(self, key): + """Get storage server statistics. + + Storage Server keeps a dict that contains various usage and + latency statistics. The dict looks like this: + + { + 'storage_server.accepting_immutable_shares': 1, + 'storage_server.allocated': 0, + 'storage_server.disk_avail': 106539192320, + 'storage_server.disk_free_for_nonroot': 106539192320, + 'storage_server.disk_free_for_root': 154415284224, + 'storage_server.disk_total': 941088460800, + 'storage_server.disk_used': 786673176576, + 'storage_server.latencies.add-lease.01_0_percentile': None, + 'storage_server.latencies.add-lease.10_0_percentile': None, + ... + } + + ``StorageServer.get_stats()`` returns the above dict. Storage + status page uses a subset of the items in the dict, concerning + disk usage. + + :param str key: storage server statistic we want to know. + """ if not self.storage: return None return self.storage.get_stats().get(key) From affe10b3e9982edbd4def2f9b67d777df6a08f06 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 12:25:20 -0500 Subject: [PATCH 073/148] Return result from renderHTTP Bogus renderHTTP, but test failures are now down to three. We just need to handle requests for JSON now. --- src/allmydata/web/storage.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 263fa9344..1656df70b 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -378,6 +378,18 @@ class StorageStatus(MultiFormatResource): flattenString(None, elem).addCallback(result.append) return result[0] + # to appease the test suite def renderHTTP(self, ctx=None): - # to appease the test suite. - self.renderSynchronously() + """Send HTML or JSON formatted data, based on request. + + This function contains a bit of nevow-ism, but since this is + only called from the test suite, the nevow-ism should go away + as we update things. + + :param _nevow.context.WovenContext ctx: context is passed on + from the test suite. We get a request out of this + context, and use the request to render a result. + + """ + from nevow.inevow import IRequest + return self.render(IRequest(ctx)) From 92f6e4bfe5b7dd98c794b946a1bb8796e5516ab3 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 15:29:42 -0500 Subject: [PATCH 074/148] Use explicit `None`-check on self.storage --- src/allmydata/web/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 1656df70b..76a09fda3 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -43,7 +43,7 @@ class StorageStatusElement(Element): @renderer def nodeid(self, req, tag): - if not self.storage: + if self.storage is None: return tag("No storage server running.") return idlib.nodeid_b2a(self.storage.my_nodeid) From ee8e7307275f2285d1474cefcb8bfba2fa430bf9 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 16:49:15 -0500 Subject: [PATCH 075/148] Move `renderSynchronously` to test suite --- src/allmydata/test/test_storage.py | 36 +++++++++++++++++++----------- src/allmydata/web/storage.py | 10 +-------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index a507596e8..88b4ab57c 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -12,6 +12,7 @@ from twisted.trial import unittest from twisted.internet import defer from twisted.application import service +from twisted.web.template import flattenString from foolscap.api import fireEventually import itertools from allmydata import interfaces @@ -2976,6 +2977,15 @@ def remove_tags(s): s = re.sub(r'\s+', ' ', s) return s +def renderSynchronously(ss): + """ + :param _StorageStatus ss: a StorageStatus instance. + """ + elem = StorageStatusElement(ss.storage, ss.nickname) + result = [] + flattenString(None, elem).addCallback(result.append) + return result[0] + class MyBucketCountingCrawler(BucketCountingCrawler): def finished_prefix(self, cycle, prefix): BucketCountingCrawler.finished_prefix(self, cycle, prefix) @@ -3012,7 +3022,7 @@ class BucketCounter(unittest.TestCase, pollmixin.PollMixin): w = StorageStatus(ss) # this sample is before the crawler has started doing anything - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Accepting new shares: Yes", s) @@ -3035,7 +3045,7 @@ class BucketCounter(unittest.TestCase, pollmixin.PollMixin): self.failUnlessEqual(state["last-complete-prefix"], ss.bucket_counter.prefixes[0]) ss.bucket_counter.cpu_slice = 100.0 # finish as fast as possible - html = w.renderSynchronously() + html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn(" Current crawl ", s) self.failUnlessIn(" (next work in ", s) @@ -3047,7 +3057,7 @@ class BucketCounter(unittest.TestCase, pollmixin.PollMixin): d.addCallback(lambda ignored: self.poll(_watch)) def _check2(ignored): ss.bucket_counter.cpu_slice = orig_cpu_slice - html = w.renderSynchronously() + html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn("Total buckets: 0 (the number of", s) self.failUnless("Next crawl in 59 minutes" in s or "Next crawl in 60 minutes" in s, s) @@ -3109,20 +3119,20 @@ class BucketCounter(unittest.TestCase, pollmixin.PollMixin): def _check_1(ignored): # no ETA is available yet - html = w.renderSynchronously() + html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn("complete (next work", s) def _check_2(ignored): # one prefix has finished, so an ETA based upon that elapsed time # should be available. - html = w.renderSynchronously() + html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn("complete (ETA ", s) def _check_3(ignored): # two prefixes have finished - html = w.renderSynchronously() + html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn("complete (ETA ", s) d.callback("done") @@ -4077,7 +4087,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): def test_no_server(self): w = StorageStatus(None) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

No Storage Server Running

", html) def test_status(self): @@ -4123,7 +4133,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, "\x00" * 20) ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Accepting new shares: Yes", s) @@ -4143,7 +4153,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, "\x00" * 20) ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Accepting new shares: No", s) @@ -4179,7 +4189,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) @@ -4197,7 +4207,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, "\x00" * 20, readonly_storage=True) ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Accepting new shares: No", s) @@ -4208,7 +4218,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, "\x00" * 20, reserved_space=10e6) ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Reserved space: - 10.00 MB (10000000)", s) @@ -4219,7 +4229,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, "\x00" * 20, reserved_space=10e6) ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Reserved space: - 10.00 MB (10000000)", s) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 76a09fda3..e9c2f6bb5 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -6,8 +6,7 @@ from twisted.web.template import ( XMLFile, tags as T, renderer, - renderElement, - flattenString + renderElement ) from allmydata.web.common import ( abbreviate_time, @@ -371,13 +370,6 @@ class StorageStatus(MultiFormatResource): } return json.dumps(d, indent=1) + "\n" - def renderSynchronously(self): - # to appease the test suite. - elem = StorageStatusElement(self.storage, self.nickname) - result = [] - flattenString(None, elem).addCallback(result.append) - return result[0] - # to appease the test suite def renderHTTP(self, ctx=None): """Send HTML or JSON formatted data, based on request. From db0e6407dc0309ee4825d936dd6dfd706f0620fd Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 17:00:54 -0500 Subject: [PATCH 076/148] Use `successResultOf` in `renderSynchronously` Get rid of [].append trick when dealing with the deferred. --- src/allmydata/test/test_storage.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 88b4ab57c..2bedffac2 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -2982,9 +2982,8 @@ def renderSynchronously(ss): :param _StorageStatus ss: a StorageStatus instance. """ elem = StorageStatusElement(ss.storage, ss.nickname) - result = [] - flattenString(None, elem).addCallback(result.append) - return result[0] + deferred = flattenString(None, elem) + return unittest.TestCase().successResultOf(deferred) class MyBucketCountingCrawler(BucketCountingCrawler): def finished_prefix(self, cycle, prefix): From 0b4c235fbd3197626bf051899ef16f1d6b49519e Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 17:18:37 -0500 Subject: [PATCH 077/148] Remove redundant `None`-checks on `self.storage` When no storage is up, `storage_running()` renderer will return a big honking `no storage server running` message, and no further renderers will be invoked. Therefore the extra defense is probably not required. (I tested this hypothesis. The extra defense is not required, unless there's something I have not seen.) --- src/allmydata/web/storage.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index e9c2f6bb5..cbee2780f 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -42,8 +42,6 @@ class StorageStatusElement(Element): @renderer def nodeid(self, req, tag): - if self.storage is None: - return tag("No storage server running.") return idlib.nodeid_b2a(self.storage.my_nodeid) def _get_storage_stat(self, key): @@ -71,8 +69,6 @@ class StorageStatusElement(Element): :param str key: storage server statistic we want to know. """ - if not self.storage: - return None return self.storage.get_stats().get(key) def render_abbrev_space(self, size): @@ -152,8 +148,6 @@ class StorageStatusElement(Element): @renderer def last_complete_bucket_count(self, req, tag): - if not self.storage: - return tag("No storage server running.") s = self.storage.bucket_counter.get_state() count = s.get("last-complete-bucket-count") if count is None: @@ -162,8 +156,6 @@ class StorageStatusElement(Element): @renderer def count_crawler_status(self, req, tag): - if not self.storage: - return tag("No storage server running.") p = self.storage.bucket_counter.get_progress() return self.format_crawler_progress(p) @@ -200,8 +192,6 @@ class StorageStatusElement(Element): @renderer def lease_expiration_enabled(self, req, tag): - if not self.storage: - return tag("No storage server running.") lc = self.storage.lease_checker if lc.expiration_enabled: return tag("Enabled: expired leases will be removed") @@ -210,8 +200,6 @@ class StorageStatusElement(Element): @renderer def lease_expiration_mode(self, req, tag): - if not self.storage: - return tag("No storage server running.") lc = self.storage.lease_checker if lc.mode == "age": if lc.override_lease_duration is None: @@ -235,16 +223,12 @@ class StorageStatusElement(Element): @renderer def lease_current_cycle_progress(self, req, tag): - if not self.storage: - return tag("No storage server running.") lc = self.storage.lease_checker p = lc.get_progress() return tag(self.format_crawler_progress(p)) @renderer def lease_current_cycle_results(self, req, tag): - if not self.storage: - return tag("No storage server running.") lc = self.storage.lease_checker p = lc.get_progress() if not p["cycle-in-progress"]: @@ -302,8 +286,6 @@ class StorageStatusElement(Element): @renderer def lease_last_cycle_results(self, req, tag): - if not self.storage: - return tag("No storage server running.") lc = self.storage.lease_checker h = lc.get_state()["history"] if not h: From bc8a86e1065c3c37286cd3e7a0378376dddbb6cb Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 17:23:35 -0500 Subject: [PATCH 078/148] Use Unicode strings to render space --- src/allmydata/web/storage.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index cbee2780f..607eeff79 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -73,13 +73,13 @@ class StorageStatusElement(Element): def render_abbrev_space(self, size): if size is None: - return "?" + return u"?" return abbreviate_space(size) def render_space(self, size): if size is None: - return "?" - return "%d" % size + return u"?" + return u"%d" % size @renderer def disk_total(self, req, tag): From 0bd386384e3a83d29c4aca9e00d02bfe829431c1 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 22:02:30 -0500 Subject: [PATCH 079/148] Mark `format_recovered` as static method --- src/allmydata/web/storage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 607eeff79..d670c4231 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -319,7 +319,8 @@ class StorageStatusElement(Element): return tag(p) - def format_recovered(self, sr, a): + @staticmethod + def format_recovered(sr, a): def maybe(d): if d is None: return "?" From 0423e49a8e01021ede8f5b97e956cd794a046b44 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 14 Feb 2020 07:25:49 -0500 Subject: [PATCH 080/148] Add news fragment --- newsfragments/3247.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3247.minor diff --git a/newsfragments/3247.minor b/newsfragments/3247.minor new file mode 100644 index 000000000..e69de29bb From 56e2fa3f2409fc2a87912d8be445212c33ec6dfc Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 14 Feb 2020 08:26:44 -0500 Subject: [PATCH 081/148] Use to render node nickname and id CI did not like the old way, but it passed in my system. Odd. --- src/allmydata/web/storage_status.xhtml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index 6e0e26008..8f74c478c 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -57,12 +57,12 @@
  • Server Nickname: - +
  • Server Nodeid: - +
  • Accepting new shares: From 1469af0f3bcad60ea58c008e8da8ccf6c381c03f Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 14 Feb 2020 10:10:48 -0500 Subject: [PATCH 082/148] Use BeautifulSoup to check favicon in storage page --- src/allmydata/test/web/test_web.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index c3779c0be..1f4f4fdbe 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -954,8 +954,9 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi def test_storage(self): d = self.GET("/storage") def _check(res): - self.failUnlessIn('Storage Server Status', res) - self.failUnlessIn(FAVICON_MARKUP, res) + soup = BeautifulSoup(res, 'html5lib') + assert_soup_has_text(self, soup, 'Storage Server Status') + assert_soup_has_favicon(self, soup) res_u = res.decode('utf-8') self.failUnlessIn(u'
  • Server Nickname: fake_nickname \u263A
    • ', res_u) d.addCallback(_check) From 9dd9d98b8ca32b365a828d355a79e05b8012765b Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 14 Feb 2020 11:42:42 -0500 Subject: [PATCH 083/148] Give the suite the precise string it wants --- src/allmydata/web/storage_status.xhtml | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index 8f74c478c..d052fbabd 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -55,16 +55,8 @@
      -
    • Server Nickname: - - - -
      • -
    • Server Nodeid: - - - -
      • +
    • Server Nickname:
      • +
    • Server Nodeid:
    • Accepting new shares:
    • Total buckets: From 0d9ec2b206f14c5473ccb286c793cde149485f09 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 17 Feb 2020 12:52:48 -0500 Subject: [PATCH 084/148] Use slots to render storage stats table --- src/allmydata/web/storage.py | 80 ++++++++------------------ src/allmydata/web/storage_status.xhtml | 26 ++++----- 2 files changed, 36 insertions(+), 70 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index d670c4231..d26f4a5dd 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -82,64 +82,30 @@ class StorageStatusElement(Element): return u"%d" % size @renderer - def disk_total(self, req, tag): - val = self._get_storage_stat("storage_server.disk_total") - return tag(self.render_space(val)) + def storage_stats(self, req, tag): + # Render storage status table that appears near the top of the page. + total = self._get_storage_stat("storage_server.disk_total") + used = self._get_storage_stat("storage_server.disk_used") + free_root = self._get_storage_stat("storage_server.disk_free_for_root") + free_nonroot = self._get_storage_stat("storage_server.disk_free_for_nonroot") + reserved = self._get_storage_stat("storage_server.reserved_space") + available = self._get_storage_stat("storage_server.disk_avail") - @renderer - def disk_total_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.disk_total") - return tag(self.render_abbrev_space(val)) - - @renderer - def disk_used(self, req, tag): - val = self._get_storage_stat("storage_server.disk_used") - return tag(self.render_space(val)) - - @renderer - def disk_used_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.disk_used") - return tag(self.render_abbrev_space(val)) - - @renderer - def disk_free_for_root(self, req, tag): - val = self._get_storage_stat("storage_server.disk_free_for_root") - return tag(self.render_space(val)) - - @renderer - def disk_free_for_root_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.disk_free_for_root") - return tag(self.render_abbrev_space(val)) - - @renderer - def disk_free_for_nonroot(self, req, tag): - val = self._get_storage_stat("storage_server.disk_free_for_nonroot") - return tag(self.render_space(val)) - - @renderer - def disk_free_for_nonroot_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.disk_free_for_nonroot") - return tag(self.render_abbrev_space(val)) - - @renderer - def reserved_space(self, req, tag): - val = self._get_storage_stat("storage_server.reserved_space") - return tag(self.render_space(val)) - - @renderer - def reserved_space_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.reserved_space") - return tag(self.render_abbrev_space(val)) - - @renderer - def disk_avail(self, req, tag): - val = self._get_storage_stat("storage_server.disk_avail") - return tag(self.render_space(val)) - - @renderer - def disk_avail_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.disk_avail") - return tag(self.render_abbrev_space(val)) + tag.fillSlots( + disk_total = self.render_space(total), + disk_total_abbrev = self.render_abbrev_space(total), + disk_used = self.render_space(used), + disk_used_abbrev = self.render_abbrev_space(used), + disk_free_for_root = self.render_space(free_root), + disk_free_for_root_abbrev = self.render_abbrev_space(free_root), + disk_free_for_nonroot = self.render_space(free_nonroot), + disk_free_for_nonroot_abbrev = self.render_abbrev_space(free_nonroot), + reserved_space = self.render_space(reserved), + reserved_space_abbrev = self.render_abbrev_space(reserved), + disk_avail = self.render_space(available), + disk_avail_abbrev = self.render_abbrev_space(available) + ) + return tag @renderer def accepting_immutable_shares(self, req, tag): diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index d052fbabd..bbf3d2c8a 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -11,15 +11,15 @@

      Storage Server Status

      - +
      - - + + - - + + - - + + - - + + - - + + - - + +
      Total disk space:()()
      Disk space used:- ()- ()
      @@ -28,18 +28,18 @@
      Disk space free (root):()() [see 1]
      Disk space free (non-root):()() [see 2]
      Reserved space:- ()- ()
      @@ -48,8 +48,8 @@
      Space Available to Tahoe:()()
      From 7126d3247ca51bf4a0d36680ee6093d29fce4728 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 25 Feb 2020 08:52:45 -0500 Subject: [PATCH 085/148] Prefix member variable with "_" --- src/allmydata/test/test_storage.py | 2 +- src/allmydata/web/storage.py | 40 +++++++++++++++--------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 2bedffac2..f4527d8b4 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -2981,7 +2981,7 @@ def renderSynchronously(ss): """ :param _StorageStatus ss: a StorageStatus instance. """ - elem = StorageStatusElement(ss.storage, ss.nickname) + elem = StorageStatusElement(ss._storage, ss._nickname) deferred = flattenString(None, elem) return unittest.TestCase().successResultOf(deferred) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index d26f4a5dd..9b3daa0cb 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -33,16 +33,16 @@ class StorageStatusElement(Element): :param string nickname: friendly name for storage. """ super(StorageStatusElement, self).__init__() - self.storage = storage - self.nick = nickname + self._storage = storage + self._nickname = nickname @renderer def nickname(self, req, tag): - return self.nick + return self._nickname @renderer def nodeid(self, req, tag): - return idlib.nodeid_b2a(self.storage.my_nodeid) + return idlib.nodeid_b2a(self._storage.my_nodeid) def _get_storage_stat(self, key): """Get storage server statistics. @@ -69,7 +69,7 @@ class StorageStatusElement(Element): :param str key: storage server statistic we want to know. """ - return self.storage.get_stats().get(key) + return self._storage.get_stats().get(key) def render_abbrev_space(self, size): if size is None: @@ -114,7 +114,7 @@ class StorageStatusElement(Element): @renderer def last_complete_bucket_count(self, req, tag): - s = self.storage.bucket_counter.get_state() + s = self._storage.bucket_counter.get_state() count = s.get("last-complete-bucket-count") if count is None: return "Not computed yet" @@ -122,7 +122,7 @@ class StorageStatusElement(Element): @renderer def count_crawler_status(self, req, tag): - p = self.storage.bucket_counter.get_progress() + p = self._storage.bucket_counter.get_progress() return self.format_crawler_progress(p) def format_crawler_progress(self, p): @@ -152,13 +152,13 @@ class StorageStatusElement(Element): @renderer def storage_running(self, req, tag): - if self.storage: + if self._storage: return tag return T.h1("No Storage Server Running") @renderer def lease_expiration_enabled(self, req, tag): - lc = self.storage.lease_checker + lc = self._storage.lease_checker if lc.expiration_enabled: return tag("Enabled: expired leases will be removed") else: @@ -166,7 +166,7 @@ class StorageStatusElement(Element): @renderer def lease_expiration_mode(self, req, tag): - lc = self.storage.lease_checker + lc = self._storage.lease_checker if lc.mode == "age": if lc.override_lease_duration is None: tag("Leases will expire naturally, probably 31 days after " @@ -189,13 +189,13 @@ class StorageStatusElement(Element): @renderer def lease_current_cycle_progress(self, req, tag): - lc = self.storage.lease_checker + lc = self._storage.lease_checker p = lc.get_progress() return tag(self.format_crawler_progress(p)) @renderer def lease_current_cycle_results(self, req, tag): - lc = self.storage.lease_checker + lc = self._storage.lease_checker p = lc.get_progress() if not p["cycle-in-progress"]: return "" @@ -252,7 +252,7 @@ class StorageStatusElement(Element): @renderer def lease_last_cycle_results(self, req, tag): - lc = self.storage.lease_checker + lc = self._storage.lease_checker h = lc.get_state()["history"] if not h: return "" @@ -304,18 +304,18 @@ class StorageStatusElement(Element): class StorageStatus(MultiFormatResource): def __init__(self, storage, nickname=""): super(StorageStatus, self).__init__() - self.storage = storage - self.nickname = nickname + self._storage = storage + self._nickname = nickname def render_HTML(self, req): - return renderElement(req, StorageStatusElement(self.storage, self.nickname)) + return renderElement(req, StorageStatusElement(self._storage, self._nickname)) def render_JSON(self, req): req.setHeader("content-type", "text/plain") - d = {"stats": self.storage.get_stats(), - "bucket-counter": self.storage.bucket_counter.get_state(), - "lease-checker": self.storage.lease_checker.get_state(), - "lease-checker-progress": self.storage.lease_checker.get_progress(), + d = {"stats": self._storage.get_stats(), + "bucket-counter": self._storage.bucket_counter.get_state(), + "lease-checker": self._storage.lease_checker.get_state(), + "lease-checker-progress": self._storage.lease_checker.get_progress(), } return json.dumps(d, indent=1) + "\n" From 8d3179194032b398efbea6489dd552ded92dc372 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 5 Mar 2020 15:45:18 -0500 Subject: [PATCH 086/148] Use a helper to exercise render() in storage test cases --- src/allmydata/test/test_storage.py | 50 ++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index f4527d8b4..63b341517 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -38,6 +38,7 @@ from allmydata.mutable.layout import MDMFSlotWriteProxy, MDMFSlotReadProxy, \ from allmydata.interfaces import BadWriteEnablerError from allmydata.test.common import LoggingServiceParent, ShouldFailMixin from allmydata.test.common_web import WebRenderingMixin +from nevow.testutil import FakeRequest from allmydata.test.no_network import NoNetworkServer from allmydata.web.storage import ( StorageStatus, @@ -2985,6 +2986,29 @@ def renderSynchronously(ss): deferred = flattenString(None, elem) return unittest.TestCase().successResultOf(deferred) +def renderDeferred(resource, **kwargs): + """ + Use this to exercise an overridden MultiFormatResource.render(), + usually for output=json or render_GET. It returns a Deferred. + + :param _MultiFormatResource resource: an HTTP resource to be rendered. + + """ + # We should be using twisted.web's DummyRequest here instead of + # nevow's FakeRequest, but right now it is a bit of a problem: see + # web/common.py. MultiFormatResource.render() makes a get_arg() + # call, which does a IRequest(ctx_or_req). IRequest can handle + # FakeRequest, but it can't handle DummyRequest. + req = FakeRequest(**kwargs) + req.fields = None + d = defer.maybeDeferred(resource.render, req) + def _done(res): + if isinstance(res, str): + return res + req.v + return req.v + d.addCallback(_done) + return d + class MyBucketCountingCrawler(BucketCountingCrawler): def finished_prefix(self, cycle, prefix): BucketCountingCrawler.finished_prefix(self, cycle, prefix) @@ -3304,7 +3328,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failIfEqual(sr2["configured-diskbytes"], None) self.failIfEqual(sr2["original-sharebytes"], None) d.addCallback(_after_first_bucket) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html_in_cycle(html): s = remove_tags(html) self.failUnlessIn("So far, this cycle has examined " @@ -3379,7 +3403,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnlessEqual(count_leases(mutable_si_2), 1) self.failUnlessEqual(count_leases(mutable_si_3), 2) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("recovered: 0 shares, 0 buckets " @@ -3479,7 +3503,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): d2.addCallback(_after_first_bucket) return d2 d.addCallback(_after_first_bucket) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html_in_cycle(html): s = remove_tags(html) # the first bucket encountered gets deleted, and its prefix @@ -3538,7 +3562,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnless(rec["configured-diskbytes"] >= 0, rec["configured-diskbytes"]) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("Expiration Enabled: expired leases will be removed", s) @@ -3623,7 +3647,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): d2.addCallback(_after_first_bucket) return d2 d.addCallback(_after_first_bucket) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html_in_cycle(html): s = remove_tags(html) # the first bucket encountered gets deleted, and its prefix @@ -3684,7 +3708,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnless(rec["configured-diskbytes"] >= 0, rec["configured-diskbytes"]) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("Expiration Enabled:" @@ -3746,7 +3770,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnlessEqual(count_shares(mutable_si_3), 1) self.failUnlessEqual(count_leases(mutable_si_3), 2) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("The following sharetypes will be expired: immutable.", s) @@ -3803,7 +3827,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnlessEqual(count_shares(mutable_si_2), 0) self.failUnlessEqual(count_shares(mutable_si_3), 0) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("The following sharetypes will be expired: mutable.", s) @@ -4034,7 +4058,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): # it also turns all tuples into lists self.failUnlessEqual(corrupt_shares, [[first_b32, 0]]) d.addCallback(_check_json) - d.addCallback(lambda ign: self.render1(w)) + d.addCallback(lambda ign: renderDeferred(w)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("Corrupt shares: SI %s shnum 0" % first_b32, s) @@ -4059,7 +4083,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): corrupt_shares = last["corrupt-shares"] self.failUnlessEqual(corrupt_shares, [[first_b32, 0]]) d.addCallback(_check_json_history) - d.addCallback(lambda ign: self.render1(w)) + d.addCallback(lambda ign: renderDeferred(w)) def _check_html_history(html): s = remove_tags(html) self.failUnlessIn("Corrupt shares: SI %s shnum 0" % first_b32, s) @@ -4073,7 +4097,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): return d def render_json(self, page): - d = self.render1(page, args={"t": ["json"]}) + d = renderDeferred(page, args={"t": ["json"]}) return d class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): @@ -4096,7 +4120,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, nodeid) ss.setServiceParent(self.s) w = StorageStatus(ss, "nickname") - d = self.render1(w) + d = renderDeferred(w) def _check_html(html): self.failUnlessIn("

      Storage Server Status

      ", html) s = remove_tags(html) @@ -4117,7 +4141,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): return d def render_json(self, page): - d = self.render1(page, args={"t": ["json"]}) + d = renderDeferred(page, args={"t": ["json"]}) return d def test_status_no_disk_stats(self): From a6e6c5cee03b8675d47bb71be87211e99e7d84a3 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 5 Mar 2020 15:46:09 -0500 Subject: [PATCH 087/148] Get rid of WebRenderingMixin in storage test --- src/allmydata/test/test_storage.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 63b341517..0105a5253 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -37,7 +37,6 @@ from allmydata.mutable.layout import MDMFSlotWriteProxy, MDMFSlotReadProxy, \ SHARE_HASH_CHAIN_SIZE from allmydata.interfaces import BadWriteEnablerError from allmydata.test.common import LoggingServiceParent, ShouldFailMixin -from allmydata.test.common_web import WebRenderingMixin from nevow.testutil import FakeRequest from allmydata.test.no_network import NoNetworkServer from allmydata.web.storage import ( @@ -3198,7 +3197,7 @@ class InstrumentedStorageServer(StorageServer): class No_ST_BLOCKS_StorageServer(StorageServer): LeaseCheckerClass = No_ST_BLOCKS_LeaseCheckingCrawler -class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): +class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): def setUp(self): self.s = service.MultiService() @@ -4100,7 +4099,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): d = renderDeferred(page, args={"t": ["json"]}) return d -class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): +class WebStatus(unittest.TestCase, pollmixin.PollMixin): def setUp(self): self.s = service.MultiService() From a056f18ca9c09e3e344e50dd5fbb7ddf3415d26b Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 5 Mar 2020 15:57:12 -0500 Subject: [PATCH 088/148] Remove StorageStatus.renderHTTP This was added to please the test suite. Pleased to remove it! --- src/allmydata/web/storage.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 9b3daa0cb..63ac47282 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -318,19 +318,3 @@ class StorageStatus(MultiFormatResource): "lease-checker-progress": self._storage.lease_checker.get_progress(), } return json.dumps(d, indent=1) + "\n" - - # to appease the test suite - def renderHTTP(self, ctx=None): - """Send HTML or JSON formatted data, based on request. - - This function contains a bit of nevow-ism, but since this is - only called from the test suite, the nevow-ism should go away - as we update things. - - :param _nevow.context.WovenContext ctx: context is passed on - from the test suite. We get a request out of this - context, and use the request to render a result. - - """ - from nevow.inevow import IRequest - return self.render(IRequest(ctx)) From de5d04ab79cf325a769f0d4499bb555cc8661fb2 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 30 Mar 2020 16:15:44 -0400 Subject: [PATCH 089/148] Remove an extraneous directive `t:data` is not really a Twisted template directive. Added my mistake, removing now. --- src/allmydata/web/storage_status.xhtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index bbf3d2c8a..354f9f177 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -57,7 +57,7 @@
      • Server Nickname:
      • Server Nodeid:
        • -
      • Accepting new shares: +
      • Accepting new shares:
      • Total buckets: From 0af744610f329ccae4621bb8e9099b09cccfd1b6 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 19:00:55 -0400 Subject: [PATCH 090/148] Avoid using nevow FakeRequest in storage test. Use twisted.web.server.Request instead, with a DummyChannel. There's still one line of inevitable nevow now, because of code in web/common.py; but that should be easily replaceable once we switch that over. --- src/allmydata/test/test_storage.py | 47 +++++++++++++++--------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 0105a5253..e3472e856 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -13,6 +13,16 @@ from twisted.trial import unittest from twisted.internet import defer from twisted.application import service from twisted.web.template import flattenString + +# We need to use `nevow.inevow.IRequest` for now for compatibility +# with the code in web/common.py. Once nevow bits are gone from +# web/common.py, we can use `twisted.web.iweb.IRequest` here. +from nevow.inevow import IRequest + +from twisted.web.server import Request +from twisted.web.test.test_web import DummyChannel +from zope.interface import implements + from foolscap.api import fireEventually import itertools from allmydata import interfaces @@ -37,7 +47,6 @@ from allmydata.mutable.layout import MDMFSlotWriteProxy, MDMFSlotReadProxy, \ SHARE_HASH_CHAIN_SIZE from allmydata.interfaces import BadWriteEnablerError from allmydata.test.common import LoggingServiceParent, ShouldFailMixin -from nevow.testutil import FakeRequest from allmydata.test.no_network import NoNetworkServer from allmydata.web.storage import ( StorageStatus, @@ -2985,28 +2994,20 @@ def renderSynchronously(ss): deferred = flattenString(None, elem) return unittest.TestCase().successResultOf(deferred) -def renderDeferred(resource, **kwargs): - """ - Use this to exercise an overridden MultiFormatResource.render(), - usually for output=json or render_GET. It returns a Deferred. +def renderDeferred(ss): + elem = StorageStatusElement(ss._storage, ss._nickname) + return flattenString(None, elem) - :param _MultiFormatResource resource: an HTTP resource to be rendered. +class JSONRequest(Request): + implements(IRequest) - """ - # We should be using twisted.web's DummyRequest here instead of - # nevow's FakeRequest, but right now it is a bit of a problem: see - # web/common.py. MultiFormatResource.render() makes a get_arg() - # call, which does a IRequest(ctx_or_req). IRequest can handle - # FakeRequest, but it can't handle DummyRequest. - req = FakeRequest(**kwargs) - req.fields = None - d = defer.maybeDeferred(resource.render, req) - def _done(res): - if isinstance(res, str): - return res + req.v - return req.v - d.addCallback(_done) - return d + def __init__(self, **kwargs): + Request.__init__(self, DummyChannel(), **kwargs) + self.args = {"t": ["json"]} + self.fields = {} + +def renderJSON(resource): + return resource.render(JSONRequest()) class MyBucketCountingCrawler(BucketCountingCrawler): def finished_prefix(self, cycle, prefix): @@ -4096,7 +4097,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): return d def render_json(self, page): - d = renderDeferred(page, args={"t": ["json"]}) + d = renderJSON(page) return d class WebStatus(unittest.TestCase, pollmixin.PollMixin): @@ -4140,7 +4141,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin): return d def render_json(self, page): - d = renderDeferred(page, args={"t": ["json"]}) + d = renderJSON(page) return d def test_status_no_disk_stats(self): From 6379601722f0ed823eda9d719fc10f563a41149d Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 19:01:22 -0400 Subject: [PATCH 091/148] Wrap renderer results in tags --- src/allmydata/web/storage.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 63ac47282..ba6609456 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -38,11 +38,11 @@ class StorageStatusElement(Element): @renderer def nickname(self, req, tag): - return self._nickname + return tag(self._nickname) @renderer def nodeid(self, req, tag): - return idlib.nodeid_b2a(self._storage.my_nodeid) + return tag(idlib.nodeid_b2a(self._storage.my_nodeid)) def _get_storage_stat(self, key): """Get storage server statistics. @@ -110,20 +110,20 @@ class StorageStatusElement(Element): @renderer def accepting_immutable_shares(self, req, tag): accepting = self._get_storage_stat("storage_server.accepting_immutable_shares") - return {True: "Yes", False: "No"}[bool(accepting)] + return tag({True: "Yes", False: "No"}[bool(accepting)]) @renderer def last_complete_bucket_count(self, req, tag): s = self._storage.bucket_counter.get_state() count = s.get("last-complete-bucket-count") if count is None: - return "Not computed yet" - return str(count) + return tag("Not computed yet") + return tag(str(count)) @renderer def count_crawler_status(self, req, tag): p = self._storage.bucket_counter.get_progress() - return self.format_crawler_progress(p) + return tag(self.format_crawler_progress(p)) def format_crawler_progress(self, p): cycletime = p["estimated-time-per-cycle"] From 7e26dd42e9260fef4ac7f0da44473d77889c96d2 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 19:05:17 -0400 Subject: [PATCH 092/148] Remove redundant render_json() method --- src/allmydata/test/test_storage.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index e3472e856..62878bffd 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -3412,7 +3412,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): "(2 mutable / 2 immutable),", s) self.failUnlessIn("but expiration was not enabled", s) d.addCallback(_check_html) - d.addCallback(lambda ign: self.render_json(webstatus)) + d.addCallback(lambda ign: renderJSON(webstatus)) def _check_json(raw): data = json.loads(raw) self.failUnlessIn("lease-checker", data) @@ -4049,7 +4049,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): self.failUnlessEqual(so_far["corrupt-shares"], [(first_b32, 0)]) d.addCallback(_after_first_bucket) - d.addCallback(lambda ign: self.render_json(w)) + d.addCallback(lambda ign: renderJSON(w)) def _check_json(raw): data = json.loads(raw) # grr. json turns all dict keys into strings. @@ -4076,7 +4076,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): self.failUnlessEqual(rec["examined-shares"], 3) self.failUnlessEqual(last["corrupt-shares"], [(first_b32, 0)]) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render_json(w)) + d.addCallback(lambda ign: renderJSON(w)) def _check_json_history(raw): data = json.loads(raw) last = data["lease-checker"]["history"]["0"] @@ -4096,9 +4096,6 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): d.addBoth(_cleanup) return d - def render_json(self, page): - d = renderJSON(page) - return d class WebStatus(unittest.TestCase, pollmixin.PollMixin): @@ -4129,7 +4126,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin): self.failUnlessIn("Accepting new shares: Yes", s) self.failUnlessIn("Reserved space: - 0 B (0)", s) d.addCallback(_check_html) - d.addCallback(lambda ign: self.render_json(w)) + d.addCallback(lambda ign: renderJSON(w)) def _check_json(raw): data = json.loads(raw) s = data["stats"] @@ -4140,9 +4137,6 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin): d.addCallback(_check_json) return d - def render_json(self, page): - d = renderJSON(page) - return d def test_status_no_disk_stats(self): def call_get_disk_stats(whichdir, reserved_space=0): From bdfa667a22d36b57287317c34e6637a44631750a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 19:13:08 -0400 Subject: [PATCH 093/148] Add docstrings to storage test helpers --- src/allmydata/test/test_storage.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 62878bffd..10d74e9ce 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -2988,6 +2988,8 @@ def remove_tags(s): def renderSynchronously(ss): """ + Return fully rendered HTML document. + :param _StorageStatus ss: a StorageStatus instance. """ elem = StorageStatusElement(ss._storage, ss._nickname) @@ -2995,10 +2997,20 @@ def renderSynchronously(ss): return unittest.TestCase().successResultOf(deferred) def renderDeferred(ss): + """ + Return a `Deferred` HTML renderer. + + :param _StorageStatus ss: a StorageStatus instance. + """ elem = StorageStatusElement(ss._storage, ss._nickname) return flattenString(None, elem) class JSONRequest(Request): + """ + A Request with t=json argument added to it. + + This is useful to invoke a Resouce.render_JSON() method. + """ implements(IRequest) def __init__(self, **kwargs): @@ -3007,6 +3019,11 @@ class JSONRequest(Request): self.fields = {} def renderJSON(resource): + """Exercise resouce.render_JSON() + + :param _MultiFormatResource resouce: A `twisted.web.resouce.Resource` + that contains a render_JSON() method. + """ return resource.render(JSONRequest()) class MyBucketCountingCrawler(BucketCountingCrawler): From 6b61b75658b2f710d44e7d502de09b0a6e3338e5 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 19:18:15 -0400 Subject: [PATCH 094/148] Refactor storage test helpers Rewrite `renderSynchronously()` to use `renderDeferred()` --- src/allmydata/test/test_storage.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 10d74e9ce..d0df30684 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -2992,9 +2992,7 @@ def renderSynchronously(ss): :param _StorageStatus ss: a StorageStatus instance. """ - elem = StorageStatusElement(ss._storage, ss._nickname) - deferred = flattenString(None, elem) - return unittest.TestCase().successResultOf(deferred) + return unittest.TestCase().successResultOf(renderDeferred(ss)) def renderDeferred(ss): """ From d449306b1900e954748f6f5183f8a76bba9cb962 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 21:48:49 -0400 Subject: [PATCH 095/148] Use DummyRequest in storage tests Using twisted.web.server.Request causes test_new_style_classes to fail like so: Traceback (most recent call last): Failure: testtools.testresult.real._StringException: Traceback (most recent call last): File ".tox/coverage/lib/python2.7/site-packages/allmydata/test/test_python2_regressions.py", line 69, in test_new_style_classes "Expected to find no classic classes.", File ".tox/coverage/lib/python2.7/site-packages/testtools/testcase.py", line 502, in assertThat raise mismatch_error testtools.matchers._impl.MismatchError: !=: reference = set([]) actual = set([]) : Expected to find no classic classes. Seems that `DummyRequest` is an acceptable new style class. --- src/allmydata/test/test_storage.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index d0df30684..06ae1708e 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -19,8 +19,8 @@ from twisted.web.template import flattenString # web/common.py, we can use `twisted.web.iweb.IRequest` here. from nevow.inevow import IRequest -from twisted.web.server import Request -from twisted.web.test.test_web import DummyChannel +# from twisted.web.server import Request +from twisted.web.test.test_web import DummyRequest from zope.interface import implements from foolscap.api import fireEventually @@ -3003,7 +3003,7 @@ def renderDeferred(ss): elem = StorageStatusElement(ss._storage, ss._nickname) return flattenString(None, elem) -class JSONRequest(Request): +class JSONRequest(DummyRequest): """ A Request with t=json argument added to it. @@ -3012,7 +3012,7 @@ class JSONRequest(Request): implements(IRequest) def __init__(self, **kwargs): - Request.__init__(self, DummyChannel(), **kwargs) + DummyRequest.__init__(self, b"/", **kwargs) self.args = {"t": ["json"]} self.fields = {} From 92e83f268bbb59b9c52a7b56fc692884471460d0 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 20 Apr 2020 14:35:32 -0400 Subject: [PATCH 096/148] Declare JSON request interface using @implementer "zope.interface.implements(IRequest)" is deprectated in favor of "@zope.interface.implementer(IRequest)" decorator. --- src/allmydata/test/test_storage.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 06ae1708e..783debd28 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -19,9 +19,8 @@ from twisted.web.template import flattenString # web/common.py, we can use `twisted.web.iweb.IRequest` here. from nevow.inevow import IRequest -# from twisted.web.server import Request -from twisted.web.test.test_web import DummyRequest -from zope.interface import implements +from twisted.web.test.requesthelper import DummyRequest +from zope.interface import implementer from foolscap.api import fireEventually import itertools @@ -3003,14 +3002,13 @@ def renderDeferred(ss): elem = StorageStatusElement(ss._storage, ss._nickname) return flattenString(None, elem) +@implementer(IRequest) class JSONRequest(DummyRequest): """ A Request with t=json argument added to it. This is useful to invoke a Resouce.render_JSON() method. """ - implements(IRequest) - def __init__(self, **kwargs): DummyRequest.__init__(self, b"/", **kwargs) self.args = {"t": ["json"]} From 96fbff684f0bbef122c67c34d3a9f3e2fb58b2cb Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 21 Apr 2020 10:29:18 -0400 Subject: [PATCH 097/148] Use an inner JSONRequest class with renderJSON Once nevow is removed from web/common.py, we can simplify renderJSON(), like so: def renderJSON(): req = Request() req.args = {"t": ["json"]} req.fields = {} return resource.render(req) But for now we have to live with an inner class that implements the nevow.inevow.IRequest interface. --- src/allmydata/test/test_storage.py | 31 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 783debd28..0acf60152 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -19,7 +19,8 @@ from twisted.web.template import flattenString # web/common.py, we can use `twisted.web.iweb.IRequest` here. from nevow.inevow import IRequest -from twisted.web.test.requesthelper import DummyRequest +from twisted.web.server import Request +from twisted.web.test.requesthelper import DummyChannel from zope.interface import implementer from foolscap.api import fireEventually @@ -3002,24 +3003,20 @@ def renderDeferred(ss): elem = StorageStatusElement(ss._storage, ss._nickname) return flattenString(None, elem) -@implementer(IRequest) -class JSONRequest(DummyRequest): - """ - A Request with t=json argument added to it. - - This is useful to invoke a Resouce.render_JSON() method. - """ - def __init__(self, **kwargs): - DummyRequest.__init__(self, b"/", **kwargs) - self.args = {"t": ["json"]} - self.fields = {} - def renderJSON(resource): - """Exercise resouce.render_JSON() + """Render a JSON from the given resource.""" + + @implementer(IRequest) + class JSONRequest(Request): + """ + A Request with t=json argument added to it. This is useful to + invoke a Resouce.render_JSON() method. + """ + def __init__(self): + Request.__init__(self, DummyChannel()) + self.args = {"t": ["json"]} + self.fields = {} - :param _MultiFormatResource resouce: A `twisted.web.resouce.Resource` - that contains a render_JSON() method. - """ return resource.render(JSONRequest()) class MyBucketCountingCrawler(BucketCountingCrawler): From d24babe873a09abd6e3aa15cb76b8832cb267a64 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 18 May 2020 17:07:47 -0400 Subject: [PATCH 098/148] Use twisted template tags in upload status page template --- src/allmydata/web/upload-status.xhtml | 62 +++++++++++++-------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/allmydata/web/upload-status.xhtml b/src/allmydata/web/upload-status.xhtml index 8aaecd47a..95a5f7692 100644 --- a/src/allmydata/web/upload-status.xhtml +++ b/src/allmydata/web/upload-status.xhtml @@ -1,4 +1,4 @@ - + Tahoe-LAFS - File Upload Status @@ -10,46 +10,46 @@

        File Upload Status

          -
        • Started:
          • -
        • Storage Index:
          • -
        • Helper?:
          • -
        • Total Size:
          • -
        • Progress (Hash):
          • -
        • Progress (Ciphertext):
          • -
        • Progress (Encode+Push):
          • -
        • Status:
          • +
        • Started:
          • +
        • Storage Index:
          • +
        • Helper?:
          • +
        • Total Size:
          • +
        • Progress (Hash):
          • +
        • Progress (Ciphertext):
          • +
        • Progress (Encode+Push):
          • +
        • Status:
        -
        +

        Upload Results

          -
        • Shares Pushed:
          • -
        • Shares Already Present:
          • -
        • Sharemap:
          • -
        • Servermap:
          • +
        • Shares Pushed:
          • +
        • Shares Already Present:
          • +
        • Sharemap:
          • +
        • Servermap:
        • Timings:
            • -
          • File Size: bytes
            • -
          • Total: - ()
            • +
          • File Size: bytes
            • +
          • Total: + ()
              • -
            • Storage Index: - ()
              • -
            • [Contacting Helper]:
              • -
            • [Upload Ciphertext To Helper]: - ()
              • +
            • Storage Index: + ()
              • +
            • [Contacting Helper]:
              • +
            • [Upload Ciphertext To Helper]: + ()
              • -
            • Peer Selection:
              • -
            • Encode And Push: - ()
              • +
            • Peer Selection:
              • +
            • Encode And Push: + ()
                • -
              • Cumulative Encoding: - ()
                • -
              • Cumulative Pushing: - ()
                • -
              • Send Hashes And Close:
                • +
              • Cumulative Encoding: + ()
                • +
              • Cumulative Pushing: + ()
                • +
              • Send Hashes And Close:
              -
            • [Helper Total]:
              • +
            • [Helper Total]:
        From e6f0f9d03877a8e3241195ba182807cb7230dfbe Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 18 May 2020 17:08:31 -0400 Subject: [PATCH 099/148] Make UploadStatusPage a MultiFormatResource --- src/allmydata/web/status.py | 85 +++++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 31 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 0c9e1ebb1..0990f45ab 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -169,62 +169,85 @@ class UploadResultsRendererMixin(RateAndTimeMixin): d.addCallback(_convert) return d -class UploadStatusPage(UploadResultsRendererMixin, rend.Page): - docFactory = getxmlfile("upload-status.xhtml") - def __init__(self, data): - rend.Page.__init__(self, data) - self.upload_status = data +class UploadStatusPage(MultiFormatResource): + + def __init__(self, upload_status): + super(UploadStatusPage, self).__init__() + self._upload_status = upload_status + + def render_HTML(self, req): + elem = UploadStatusElement(self._upload_status) + return renderElement(req, elem) + + +class UploadStatusElement(Element, UploadResultsRendererMixin): + + loader = XMLFile(FilePath(__file__).sibling("upload-status.xhtml")) + + def __init__(self, upload_status): + super(UploadStatusElement, self).__init__() + self._upload_status = upload_status def upload_results(self): - return defer.maybeDeferred(self.upload_status.get_results) + return defer.maybeDeferred(self._upload_status.get_results) - def render_results(self, ctx, data): + @renderer + def results(self, req, tag): d = self.upload_results() def _got_results(results): if results: - return ctx.tag + return tag return "" d.addCallback(_got_results) return d - def render_started(self, ctx, data): - started_s = render_time(data.get_started()) - return started_s + @renderer + def started(self, req, tag): + started_s = render_time(self._upload_status.get_started()) + return tag(started_s) - def render_si(self, ctx, data): - si_s = base32.b2a_or_none(data.get_storage_index()) + @renderer + def si(self, req, tag): + si_s = base32.b2a_or_none(self._upload_status.get_storage_index()) if si_s is None: si_s = "(None)" - return si_s + return tag(str(si_s)) - def render_helper(self, ctx, data): - return {True: "Yes", - False: "No"}[data.using_helper()] + @renderer + def helper(self, req, tag): + return tag({True: "Yes", + False: "No"}[self._upload_status.using_helper()]) - def render_total_size(self, ctx, data): - size = data.get_size() + @renderer + def total_size(self, req, tag): + size = self._upload_status.get_size() if size is None: return "(unknown)" - return size + return tag(str(size)) - def render_progress_hash(self, ctx, data): - progress = data.get_progress()[0] + @renderer + def progress_hash(self, req, tag): + progress = self._upload_status.get_progress()[0] + # TODO: make an ascii-art bar + return tag("%.1f%%" % (100.0 * progress)) + + @renderer + def progress_ciphertext(self, req, tag): + progress = self._upload_status.get_progress()[1] # TODO: make an ascii-art bar return "%.1f%%" % (100.0 * progress) - def render_progress_ciphertext(self, ctx, data): - progress = data.get_progress()[1] + @renderer + def progress_encode_push(self, req, tag): + progress = self._upload_status.get_progress()[2] # TODO: make an ascii-art bar - return "%.1f%%" % (100.0 * progress) + return tag("%.1f%%" % (100.0 * progress)) - def render_progress_encode_push(self, ctx, data): - progress = data.get_progress()[2] - # TODO: make an ascii-art bar - return "%.1f%%" % (100.0 * progress) + @renderer + def status(self, req, tag): + return tag(self._upload_status.get_status()) - def render_status(self, ctx, data): - return data.get_status() class DownloadResultsRendererMixin(RateAndTimeMixin): # this requires a method named 'download_results' From 71fd4f5d109af06ab7bd594bf9436da51f010992 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 18 May 2020 17:50:32 -0400 Subject: [PATCH 100/148] Use twisted template tags in upload results page template --- src/allmydata/web/upload-results.xhtml | 46 +++++++++++++------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/allmydata/web/upload-results.xhtml b/src/allmydata/web/upload-results.xhtml index fa8894dcd..290c9bd42 100644 --- a/src/allmydata/web/upload-results.xhtml +++ b/src/allmydata/web/upload-results.xhtml @@ -1,4 +1,4 @@ - + Tahoe-LAFS - File Uploaded @@ -7,37 +7,37 @@ -

        Uploading File...

        +

        Uploading File...

        Upload Results:

          -
        • URI:
          • -
        • Download link:
          • -
        • Sharemap:
          • -
        • Servermap:
          • +
        • URI:
          • +
        • Download link:
          • +
        • Sharemap:
          • +
        • Servermap:
        • Timings:
            • -
          • File Size: bytes
            • -
          • Total: - ()
            • +
          • File Size: bytes
            • +
          • Total: + ()
              • -
            • Storage Index: - ()
              • -
            • [Contacting Helper]:
              • -
            • [Upload Ciphertext To Helper]: - ()
              • +
            • Storage Index: + ()
              • +
            • [Contacting Helper]:
              • +
            • [Upload Ciphertext To Helper]: + ()
              • -
            • Peer Selection:
              • -
            • Encode And Push: - ()
              • +
            • Peer Selection:
              • +
            • Encode And Push: + ()
                • -
              • Cumulative Encoding: - ()
                • -
              • Cumulative Pushing: - ()
                • -
              • Send Hashes And Close:
                • +
              • Cumulative Encoding: + ()
                • +
              • Cumulative Pushing: + ()
                • +
              • Send Hashes And Close:
              -
            • [Helper Total]:
              • +
            • [Helper Total]:
        From 2ab7b15f8b6e7662a4205f3ecd98d80a42003605 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 18 May 2020 17:50:54 -0400 Subject: [PATCH 101/148] Make UploadResultsPage a MultiFormatResource --- src/allmydata/web/unlinked.py | 58 +++++++++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index 8a84a4021..0abf6cc6d 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -2,11 +2,27 @@ import urllib from twisted.web import http from twisted.internet import defer -from nevow import rend, url, tags as T +from twisted.python.filepath import FilePath +from twisted.web.template import ( + Element, + XMLFile, + renderer, + renderElement, + tags, +) +from nevow import url, tags as T from allmydata.immutable.upload import FileHandle from allmydata.mutable.publish import MutableFileHandle -from allmydata.web.common import getxmlfile, get_arg, boolean_of_arg, \ - convert_children_json, WebError, get_format, get_mutable_type +from allmydata.web.common import ( + getxmlfile, + get_arg, + boolean_of_arg, + convert_children_json, + WebError, + get_format, + get_mutable_type, + MultiFormatResource, +) from allmydata.web import status def PUTUnlinkedCHK(req, client): @@ -59,34 +75,50 @@ def POSTUnlinkedCHK(req, client): return d -class UploadResultsPage(status.UploadResultsRendererMixin, rend.Page): +class UploadResultsPage(MultiFormatResource): """'POST /uri', to create an unlinked file.""" - docFactory = getxmlfile("upload-results.xhtml") def __init__(self, upload_results): - rend.Page.__init__(self) - self.results = upload_results + super(UploadResultsPage, self).__init__() + self._upload_results = upload_results + + def render_HTML(self, req): + elem = UploadResultsElement(self._upload_results) + return renderElement(req, elem) + + +class UploadResultsElement(Element, status.UploadResultsRendererMixin): + + loader = XMLFile(FilePath(__file__).sibling("upload-results.xhtml")) + + def __init__(self, upload_results): + super(UploadResultsElement, self).__init__() + self._upload_results = upload_results def upload_results(self): - return defer.succeed(self.results) + return defer.succeed(self._upload_results) - def data_done(self, ctx, data): + @renderer + def done(self, req, tag): d = self.upload_results() d.addCallback(lambda res: "done!") return d - def data_uri(self, ctx, data): + @renderer + def uri(self, req, tag): d = self.upload_results() d.addCallback(lambda res: res.get_uri()) return d - def render_download_link(self, ctx, data): + @renderer + def download_link(self, req, tag): d = self.upload_results() d.addCallback(lambda res: - T.a(href="/uri/" + urllib.quote(res.get_uri())) - ["/uri/" + res.get_uri()]) + tags.a("/uri/" + res.get_uri(), + href="/uri/" + urllib.quote(res.get_uri()))) return d + def POSTUnlinkedSSK(req, client, version): # "POST /uri", to create an unlinked file. # SDMF: files are small, and we can only upload data From 6b4d27bec1b0ebd573bb6b9b19db9733290ecebc Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 18 May 2020 17:53:38 -0400 Subject: [PATCH 102/148] Use twisted web template in UploadResultsRendererMixin --- src/allmydata/web/status.py | 120 +++++++++++++++++++++--------------- 1 file changed, 71 insertions(+), 49 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 0990f45ab..5a9714dd0 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -44,128 +44,150 @@ class RateAndTimeMixin(object): def render_rate(self, ctx, data): return abbreviate_rate(data) + class UploadResultsRendererMixin(RateAndTimeMixin): # this requires a method named 'upload_results' - def render_pushed_shares(self, ctx, data): + @renderer + def pushed_shares(self, req, tag): d = self.upload_results() - d.addCallback(lambda res: res.get_pushed_shares()) + d.addCallback(lambda res: str(res.get_pushed_shares())) return d - def render_preexisting_shares(self, ctx, data): + @renderer + def preexisting_shares(self, req, tag): d = self.upload_results() - d.addCallback(lambda res: res.get_preexisting_shares()) + d.addCallback(lambda res: str(res.get_preexisting_shares())) return d - def render_sharemap(self, ctx, data): + @renderer + def sharemap(self, req, tag): d = self.upload_results() d.addCallback(lambda res: res.get_sharemap()) def _render(sharemap): if sharemap is None: return "None" - l = T.ul() + ul = tags.ul() for shnum, servers in sorted(sharemap.items()): server_names = ', '.join([s.get_name() for s in servers]) - l[T.li["%d -> placed on [%s]" % (shnum, server_names)]] - return l + ul(tags.li("%d -> placed on [%s]" % (shnum, server_names))) + return ul d.addCallback(_render) return d - def render_servermap(self, ctx, data): + @renderer + def servermap(self, req, tag): d = self.upload_results() d.addCallback(lambda res: res.get_servermap()) def _render(servermap): if servermap is None: return "None" - l = T.ul() + ul = tags.ul() for server, shnums in sorted(servermap.items()): shares_s = ",".join(["#%d" % shnum for shnum in shnums]) - l[T.li["[%s] got share%s: %s" % (server.get_name(), - plural(shnums), shares_s)]] - return l + ul(tags.li("[%s] got share%s: %s" % (server.get_name(), + plural(shnums), shares_s))) + return ul d.addCallback(_render) return d - def data_file_size(self, ctx, data): + @renderer + def file_size(self, req, tag): d = self.upload_results() - d.addCallback(lambda res: res.get_file_size()) + d.addCallback(lambda res: str(res.get_file_size())) return d def _get_time(self, name): d = self.upload_results() - d.addCallback(lambda res: res.get_timings().get(name)) + d.addCallback(lambda res: str(res.get_timings().get(name))) return d - def data_time_total(self, ctx, data): - return self._get_time("total") + @renderer + def time_total(self, req, tag): + return tag(self._get_time("total")) - def data_time_storage_index(self, ctx, data): - return self._get_time("storage_index") + @renderer + def time_storage_index(self, req, tag): + return tag(self._get_time("storage_index")) - def data_time_contacting_helper(self, ctx, data): - return self._get_time("contacting_helper") + @renderer + def time_contacting_helper(self, req, tag): + return tag(self._get_time("contacting_helper")) - def data_time_cumulative_fetch(self, ctx, data): - return self._get_time("cumulative_fetch") + @renderer + def time_cumulative_fetch(self, req, tag): + return tag(self._get_time("cumulative_fetch")) - def data_time_helper_total(self, ctx, data): - return self._get_time("helper_total") + @renderer + def time_helper_total(self, req, tag): + return tag(self._get_time("helper_total")) - def data_time_peer_selection(self, ctx, data): - return self._get_time("peer_selection") + @renderer + def time_peer_selection(self, req, tag): + return tag(self._get_time("peer_selection")) - def data_time_total_encode_and_push(self, ctx, data): - return self._get_time("total_encode_and_push") + @renderer + def time_total_encode_and_push(self, req, tag): + return tag(self._get_time("total_encode_and_push")) - def data_time_cumulative_encoding(self, ctx, data): - return self._get_time("cumulative_encoding") + @renderer + def time_cumulative_encoding(self, req, tag): + return tag(self._get_time("cumulative_encoding")) - def data_time_cumulative_sending(self, ctx, data): - return self._get_time("cumulative_sending") + @renderer + def time_cumulative_sending(self, req, tag): + return tag(self._get_time("cumulative_sending")) - def data_time_hashes_and_close(self, ctx, data): - return self._get_time("hashes_and_close") + @renderer + def time_hashes_and_close(self, req, tag): + return tag(self._get_time("hashes_and_close")) def _get_rate(self, name): d = self.upload_results() def _convert(r): file_size = r.get_file_size() duration = r.get_timings().get(name) - return compute_rate(file_size, duration) + return str(compute_rate(file_size, duration)) d.addCallback(_convert) return d - def data_rate_total(self, ctx, data): - return self._get_rate("total") + @renderer + def rate_total(self, req, tag): + return tag(self._get_rate("total")) - def data_rate_storage_index(self, ctx, data): - return self._get_rate("storage_index") + @renderer + def rate_storage_index(self, req, tag): + return tag(self._get_rate("storage_index")) - def data_rate_encode(self, ctx, data): - return self._get_rate("cumulative_encoding") + @renderer + def rate_encode(self, req, tag): + return tag(self._get_rate("cumulative_encoding")) - def data_rate_push(self, ctx, data): + @renderer + def rate_push(self, req, tag): return self._get_rate("cumulative_sending") - def data_rate_encode_and_push(self, ctx, data): + @renderer + def rate_encode_and_push(self, req, tag): d = self.upload_results() def _convert(r): file_size = r.get_file_size() time1 = r.get_timings().get("cumulative_encoding") time2 = r.get_timings().get("cumulative_sending") if (time1 is None or time2 is None): - return None + return str(None) else: - return compute_rate(file_size, time1+time2) + return str(compute_rate(file_size, time1+time2)) d.addCallback(_convert) return d - def data_rate_ciphertext_fetch(self, ctx, data): + @renderer + def rate_ciphertext_fetch(self, req, tag): d = self.upload_results() def _convert(r): fetch_size = r.get_ciphertext_fetched() duration = r.get_timings().get("cumulative_fetch") - return compute_rate(fetch_size, duration) + return str(compute_rate(fetch_size, duration)) d.addCallback(_convert) return d From a0655c18d6b3a589885c2b883c058e4f39f55496 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 18 May 2020 18:00:20 -0400 Subject: [PATCH 103/148] Handle "t=upload" in UploadResultsPage POST handling --- src/allmydata/web/unlinked.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index 0abf6cc6d..283d8d586 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -86,6 +86,23 @@ class UploadResultsPage(MultiFormatResource): elem = UploadResultsElement(self._upload_results) return renderElement(req, elem) + # This is weird but necessary because: + # + # 1. MultiFormatResource.render() uses argument "t" to figure out + # its output format. + # + # 2. Upload request is of the form "POST /uri?t=upload&file=newfile". + # See URIHandler.render_POST(). + # + # MultiFormatResource.render() looks up "t" argument, which in + # this case has the value "upload", and then it would look for a + # render_UPLOAD() method. + # + # We could change upload request to use more descriptive names + # that do not cause name collisions like this. That should be a + # separate change though. + render_UPLOAD = render_HTML + class UploadResultsElement(Element, status.UploadResultsRendererMixin): From 855668b1885fc87373f83d326b7d1489614944be Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 13:23:23 -0400 Subject: [PATCH 104/148] Remove unused imports --- src/allmydata/web/unlinked.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index 283d8d586..33fb7bafa 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -10,11 +10,10 @@ from twisted.web.template import ( renderElement, tags, ) -from nevow import url, tags as T +from nevow import url from allmydata.immutable.upload import FileHandle from allmydata.mutable.publish import MutableFileHandle from allmydata.web.common import ( - getxmlfile, get_arg, boolean_of_arg, convert_children_json, From 0af8df72030018f8280a55b7a14eb5b66c44fac1 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 13:25:09 -0400 Subject: [PATCH 105/148] Document UploadStatusPage --- src/allmydata/web/status.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 5a9714dd0..ede0ae7be 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -193,8 +193,12 @@ class UploadResultsRendererMixin(RateAndTimeMixin): class UploadStatusPage(MultiFormatResource): + """Renders /status/up-%d.""" def __init__(self, upload_status): + """ + :param IUploadStatus upload_status: stats provider. + """ super(UploadStatusPage, self).__init__() self._upload_status = upload_status From c0d83def22f8a61da6fe0edf38955b193e89b68d Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 13:28:16 -0400 Subject: [PATCH 106/148] Document UploadResultsPage parameter --- src/allmydata/web/unlinked.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index 33fb7bafa..6ca219c8b 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -78,6 +78,9 @@ class UploadResultsPage(MultiFormatResource): """'POST /uri', to create an unlinked file.""" def __init__(self, upload_results): + """ + :param IUploadResults upload_results: stats provider. + """ super(UploadResultsPage, self).__init__() self._upload_results = upload_results From 118ea4267cc2c8de3e458988313a51141fcfed72 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 14:53:33 -0400 Subject: [PATCH 107/148] Render abbreviated rate and time values --- src/allmydata/web/status.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index ede0ae7be..1bd469555 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -99,7 +99,7 @@ class UploadResultsRendererMixin(RateAndTimeMixin): def _get_time(self, name): d = self.upload_results() - d.addCallback(lambda res: str(res.get_timings().get(name))) + d.addCallback(lambda res: abbreviate_time(res.get_timings().get(name))) return d @renderer @@ -147,7 +147,7 @@ class UploadResultsRendererMixin(RateAndTimeMixin): def _convert(r): file_size = r.get_file_size() duration = r.get_timings().get(name) - return str(compute_rate(file_size, duration)) + return abbreviate_rate(compute_rate(file_size, duration)) d.addCallback(_convert) return d @@ -175,9 +175,9 @@ class UploadResultsRendererMixin(RateAndTimeMixin): time1 = r.get_timings().get("cumulative_encoding") time2 = r.get_timings().get("cumulative_sending") if (time1 is None or time2 is None): - return str(None) + return abbreviate_rate(None) else: - return str(compute_rate(file_size, time1+time2)) + return abbreviate_rate(compute_rate(file_size, time1+time2)) d.addCallback(_convert) return d @@ -187,7 +187,7 @@ class UploadResultsRendererMixin(RateAndTimeMixin): def _convert(r): fetch_size = r.get_ciphertext_fetched() duration = r.get_timings().get("cumulative_fetch") - return str(compute_rate(fetch_size, duration)) + return abbreviate_rate(compute_rate(fetch_size, duration)) d.addCallback(_convert) return d From 100c4b1cdedb893f396ff608f119949e2e610eee Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 20:08:34 -0400 Subject: [PATCH 108/148] Add newsfragment --- newsfragments/3287.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3287.minor diff --git a/newsfragments/3287.minor b/newsfragments/3287.minor new file mode 100644 index 000000000..e69de29bb From 9c14eec1e05874b63403b0dec45faff9934d9254 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 20:22:59 -0400 Subject: [PATCH 109/148] Avoid use of RateAndTimeMixin in UploadResultsRendererMixin --- src/allmydata/web/status.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 1bd469555..46805fb99 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -45,7 +45,7 @@ class RateAndTimeMixin(object): return abbreviate_rate(data) -class UploadResultsRendererMixin(RateAndTimeMixin): +class UploadResultsRendererMixin(object): # this requires a method named 'upload_results' @renderer @@ -392,7 +392,7 @@ class DownloadResultsRendererMixin(RateAndTimeMixin): l = T.ul() for peerid in sorted(per_server.keys()): peerid_s = idlib.shortnodeid_b2a(peerid) - times_s = ", ".join([self.render_time(None, t) + times_s = ", ".join([abbreviate_time(t) for t in per_server[peerid]]) l[T.li["[%s]: %s" % (peerid_s, times_s)]] return T.li["Per-Server Segment Fetch Response Times: ", l] From 4b560d10cc37477629c5c49f066ff1cd3b995e55 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 20:49:34 -0400 Subject: [PATCH 110/148] Move time formatter tests UploadResultsRendererMixin no longer has render_time(); moving tests for render_time() as tests for abbreviate_time() to a more appropriate place. --- src/allmydata/test/web/test_util.py | 10 ++++++++++ src/allmydata/test/web/test_web.py | 11 ----------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/allmydata/test/web/test_util.py b/src/allmydata/test/web/test_util.py index 2a6f67abc..a378c1447 100644 --- a/src/allmydata/test/web/test_util.py +++ b/src/allmydata/test/web/test_util.py @@ -23,6 +23,16 @@ class Util(ShouldFailMixin, testutil.ReallyEqualMixin, unittest.TestCase): self.failUnlessReallyEqual(common.abbreviate_time(0.000123), "123us") self.failUnlessReallyEqual(common.abbreviate_time(-123000), "-123000000000us") + self.failUnlessReallyEqual(common.abbreviate_time(None), "") + self.failUnlessReallyEqual(common.abbreviate_time(2.5), "2.50s") + self.failUnlessReallyEqual(common.abbreviate_time(0.25), "250ms") + self.failUnlessReallyEqual(common.abbreviate_time(0.0021), "2.1ms") + self.failUnlessReallyEqual(common.abbreviate_time(0.000123), "123us") + self.failUnlessReallyEqual(common.abbreviate_rate(None), "") + self.failUnlessReallyEqual(common.abbreviate_rate(2500000), "2.50MBps") + self.failUnlessReallyEqual(common.abbreviate_rate(30100), "30.1kBps") + self.failUnlessReallyEqual(common.abbreviate_rate(123), "123Bps") + def test_compute_rate(self): self.failUnlessReallyEqual(common.compute_rate(None, None), None) self.failUnlessReallyEqual(common.compute_rate(None, 1), None) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 1f4f4fdbe..bbd44b454 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1052,17 +1052,6 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self.failUnlessReallyEqual(drrm.render_rate(None, 30100), "30.1kBps") self.failUnlessReallyEqual(drrm.render_rate(None, 123), "123Bps") - urrm = status.UploadResultsRendererMixin() - self.failUnlessReallyEqual(urrm.render_time(None, None), "") - self.failUnlessReallyEqual(urrm.render_time(None, 2.5), "2.50s") - self.failUnlessReallyEqual(urrm.render_time(None, 0.25), "250ms") - self.failUnlessReallyEqual(urrm.render_time(None, 0.0021), "2.1ms") - self.failUnlessReallyEqual(urrm.render_time(None, 0.000123), "123us") - self.failUnlessReallyEqual(urrm.render_rate(None, None), "") - self.failUnlessReallyEqual(urrm.render_rate(None, 2500000), "2.50MBps") - self.failUnlessReallyEqual(urrm.render_rate(None, 30100), "30.1kBps") - self.failUnlessReallyEqual(urrm.render_rate(None, 123), "123Bps") - def test_GET_FILEURL(self): d = self.GET(self.public_url + "/foo/bar.txt") d.addCallback(self.failUnlessIsBarDotTxt) From f30dcc7ec6268a01724b3676356a608153b09326 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 21:11:42 -0400 Subject: [PATCH 111/148] Make Element the superclass of UploadResultsRendererMixin --- src/allmydata/web/status.py | 4 ++-- src/allmydata/web/unlinked.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 46805fb99..6f3ea3a46 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -45,7 +45,7 @@ class RateAndTimeMixin(object): return abbreviate_rate(data) -class UploadResultsRendererMixin(object): +class UploadResultsRendererMixin(Element): # this requires a method named 'upload_results' @renderer @@ -207,7 +207,7 @@ class UploadStatusPage(MultiFormatResource): return renderElement(req, elem) -class UploadStatusElement(Element, UploadResultsRendererMixin): +class UploadStatusElement(UploadResultsRendererMixin): loader = XMLFile(FilePath(__file__).sibling("upload-status.xhtml")) diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index 6ca219c8b..d938691a4 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -106,7 +106,7 @@ class UploadResultsPage(MultiFormatResource): render_UPLOAD = render_HTML -class UploadResultsElement(Element, status.UploadResultsRendererMixin): +class UploadResultsElement(status.UploadResultsRendererMixin): loader = XMLFile(FilePath(__file__).sibling("upload-results.xhtml")) From 38fe7dd3f59b29ac5f0f8ef0e9dc2456e3c46b91 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 22 May 2020 21:33:30 -0400 Subject: [PATCH 112/148] Remove unused import --- src/allmydata/web/unlinked.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index d938691a4..91efff2c4 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -4,7 +4,6 @@ from twisted.web import http from twisted.internet import defer from twisted.python.filepath import FilePath from twisted.web.template import ( - Element, XMLFile, renderer, renderElement, From 62e803db2560e838a679df1627723a460127b586 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 23 Jun 2020 21:28:31 -0400 Subject: [PATCH 113/148] Make UploadStatusPage a Resource --- src/allmydata/web/status.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 6f3ea3a46..39b19cb0b 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -192,7 +192,7 @@ class UploadResultsRendererMixin(Element): return d -class UploadStatusPage(MultiFormatResource): +class UploadStatusPage(Resource, object): """Renders /status/up-%d.""" def __init__(self, upload_status): @@ -202,7 +202,7 @@ class UploadStatusPage(MultiFormatResource): super(UploadStatusPage, self).__init__() self._upload_status = upload_status - def render_HTML(self, req): + def render_GET(self, req): elem = UploadStatusElement(self._upload_status) return renderElement(req, elem) From 44ecfdad0955460d51848f5f6ca505e1b7e930ad Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 23 Jun 2020 21:53:39 -0400 Subject: [PATCH 114/148] Make UploadResultsPage a Resource --- src/allmydata/web/unlinked.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index 91efff2c4..09795101a 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -3,6 +3,7 @@ import urllib from twisted.web import http from twisted.internet import defer from twisted.python.filepath import FilePath +from twisted.web.resource import Resource from twisted.web.template import ( XMLFile, renderer, @@ -19,7 +20,6 @@ from allmydata.web.common import ( WebError, get_format, get_mutable_type, - MultiFormatResource, ) from allmydata.web import status @@ -73,7 +73,7 @@ def POSTUnlinkedCHK(req, client): return d -class UploadResultsPage(MultiFormatResource): +class UploadResultsPage(Resource, object): """'POST /uri', to create an unlinked file.""" def __init__(self, upload_results): @@ -83,7 +83,7 @@ class UploadResultsPage(MultiFormatResource): super(UploadResultsPage, self).__init__() self._upload_results = upload_results - def render_HTML(self, req): + def render_POST(self, req): elem = UploadResultsElement(self._upload_results) return renderElement(req, elem) From 8331e7d8ec11f5080255c46dbdc0c90d43c46e6f Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 23 Jun 2020 21:54:23 -0400 Subject: [PATCH 115/148] Remove now-unnecessary UploadResultsPage.render_UPLOAD --- src/allmydata/web/unlinked.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index 09795101a..df342d7dd 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -87,23 +87,6 @@ class UploadResultsPage(Resource, object): elem = UploadResultsElement(self._upload_results) return renderElement(req, elem) - # This is weird but necessary because: - # - # 1. MultiFormatResource.render() uses argument "t" to figure out - # its output format. - # - # 2. Upload request is of the form "POST /uri?t=upload&file=newfile". - # See URIHandler.render_POST(). - # - # MultiFormatResource.render() looks up "t" argument, which in - # this case has the value "upload", and then it would look for a - # render_UPLOAD() method. - # - # We could change upload request to use more descriptive names - # that do not cause name collisions like this. That should be a - # separate change though. - render_UPLOAD = render_HTML - class UploadResultsElement(status.UploadResultsRendererMixin): From 83f36ab6c22b79996571085ff0781739c7de6aa2 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 6 Jul 2020 13:50:06 -0400 Subject: [PATCH 116/148] Switch to flake8 as a better way of running pyflakes. --- Makefile | 2 +- newsfragments/3329.other | 0 setup.cfg | 5 +++++ setup.py | 3 ++- tox.ini | 2 +- 5 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 newsfragments/3329.other diff --git a/Makefile b/Makefile index 6e0009ddf..6dd87409e 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ default: PYTHON=python export PYTHON -PYFLAKES=pyflakes +PYFLAKES=flake8 export PYFLAKES SOURCES=src/allmydata static misc setup.py diff --git a/newsfragments/3329.other b/newsfragments/3329.other new file mode 100644 index 000000000..e69de29bb diff --git a/setup.cfg b/setup.cfg index de40c9e23..c2d224225 100644 --- a/setup.cfg +++ b/setup.cfg @@ -5,3 +5,8 @@ install = update_version install develop = update_version develop bdist_egg = update_version bdist_egg bdist_wheel = update_version bdist_wheel + +[flake8] +# For now, only use pyflakes errors; flake8 is still helpful because it allows +# ignoring specific errors/warnings when needed. +select = F \ No newline at end of file diff --git a/setup.py b/setup.py index caa37f16e..0d43294cc 100644 --- a/setup.py +++ b/setup.py @@ -353,11 +353,12 @@ setup(name="tahoe-lafs", # also set in __init__.py # discussion. ':sys_platform=="win32"': ["pywin32 != 226"], "test": [ + "flake8", # Pin a specific pyflakes so we don't have different folks # disagreeing on what is or is not a lint issue. We can bump # this version from time to time, but we will do it # intentionally. - "pyflakes == 2.1.0", + "pyflakes == 2.2.0", # coverage 5.0 breaks the integration tests in some opaque way. # This probably needs to be addressed in a more permanent way # eventually... diff --git a/tox.ini b/tox.ini index 3134a0d9f..44b583baa 100644 --- a/tox.ini +++ b/tox.ini @@ -75,7 +75,7 @@ commands = whitelist_externals = /bin/mv commands = - pyflakes src static misc setup.py + flake8 src static misc setup.py python misc/coding_tools/check-umids.py src python misc/coding_tools/check-debugging.py python misc/coding_tools/find-trailing-spaces.py -r src static misc setup.py From 4e2cc6e1fd2cb3eac45f2d1b5a4f99470be6f26e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 1 Jul 2020 15:41:17 -0400 Subject: [PATCH 117/148] Here's a start --- docs/CODE_OF_CONDUCT.md | 53 +++++++++++++++++++++++++++++++++++++++++ docs/index.rst | 1 + 2 files changed, 54 insertions(+) create mode 100644 docs/CODE_OF_CONDUCT.md diff --git a/docs/CODE_OF_CONDUCT.md b/docs/CODE_OF_CONDUCT.md new file mode 100644 index 000000000..868f59c6a --- /dev/null +++ b/docs/CODE_OF_CONDUCT.md @@ -0,0 +1,53 @@ +# Contributor Code of Conduct + +As contributors and maintainers of this project, and in the interest of +fostering an open and welcoming community, we pledge to respect all people who +contribute through reporting issues, posting feature requests, updating +documentation, submitting pull requests or patches, and other activities. + +We are committed to making participation in this project a harassment-free +experience for everyone, regardless of level of experience, gender, gender +identity and expression, sexual orientation, disability, personal appearance, +body size, race, ethnicity, age, religion, or nationality. + +Examples of unacceptable behavior by participants include: + +* The use of sexualized language or imagery +* Personal attacks +* Trolling or insulting/derogatory comments +* Public or private harassment +* Publishing other's private information, such as physical or electronic + addresses, without explicit permission +* Other unethical or unprofessional conduct + +Project maintainers have the right and responsibility to remove, edit, or +reject comments, commits, code, wiki edits, issues, and other contributions +that are not aligned to this Code of Conduct, or to ban temporarily or +permanently any contributor for other behaviors that they deem inappropriate, +threatening, offensive, or harmful. + +By adopting this Code of Conduct, project maintainers commit themselves to +fairly and consistently applying these principles to every aspect of managing +this project. Project maintainers who do not follow or enforce the Code of +Conduct may be permanently removed from the project team. + +This Code of Conduct applies both within project spaces and in public spaces +when an individual is representing the project or its community. + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported by contacting a project maintainer (see below). All +complaints will be reviewed and investigated and will result in a response that +is deemed necessary and appropriate to the circumstances. Maintainers are +obligated to maintain confidentiality with regard to the reporter of an +incident. + +The following community members have made themselves available for conduct issues: + +- Jean-Paul Calderone (jean-paul at leastauthority dot com) + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], +version 1.3.0, available at +[http://contributor-covenant.org/version/1/3/0/][version] + +[homepage]: http://contributor-covenant.org +[version]: http://contributor-covenant.org/version/1/3/0/ diff --git a/docs/index.rst b/docs/index.rst index 98b4f241b..b125cdd4e 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -24,6 +24,7 @@ Contents: known_issues ../.github/CONTRIBUTING + ../.github/CODE_OF_CONDUCT servers helper From ff8785acbe93a83e8d631faabd89595dd0866213 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 1 Jul 2020 15:48:39 -0400 Subject: [PATCH 118/148] as far as I can tell there is nothing magic about putting it in .github --- docs/index.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/index.rst b/docs/index.rst index b125cdd4e..581e74bbb 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -24,7 +24,7 @@ Contents: known_issues ../.github/CONTRIBUTING - ../.github/CODE_OF_CONDUCT + CODE_OF_CONDUCT servers helper From 07064f096fa580399bcaaff964fbc1a35f6c640a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 1 Jul 2020 15:50:47 -0400 Subject: [PATCH 119/148] news fragment --- newsfragments/2755.other | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/2755.other diff --git a/newsfragments/2755.other b/newsfragments/2755.other new file mode 100644 index 000000000..e3e31465d --- /dev/null +++ b/newsfragments/2755.other @@ -0,0 +1 @@ +The Tahoe-LAFS project has adopted a formal code of conduct. From defac12b192a366996ad809d702d43ead7fd27d7 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 6 Jul 2020 10:41:38 -0400 Subject: [PATCH 120/148] add a contributor covenant badge --- README.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index d53fa0104..274cee41d 100644 --- a/README.rst +++ b/README.rst @@ -10,7 +10,8 @@ function correctly, preserving your privacy and security. For full documentation, please see http://tahoe-lafs.readthedocs.io/en/latest/ . -|readthedocs| |travis| |circleci| |codecov| +|Contributor Covenant| |readthedocs| |travis| |circleci| |codecov| + INSTALLING ========== @@ -105,3 +106,5 @@ slides. .. |codecov| image:: https://codecov.io/github/tahoe-lafs/tahoe-lafs/coverage.svg?branch=master :alt: test coverage percentage :target: https://codecov.io/github/tahoe-lafs/tahoe-lafs?branch=master + +.. |Contributor Covenant| image:: https://img.shields.io/badge/Contributor%20Covenant-v2.0%20adopted-ff69b4.svg :target: docs/CODE_OF_CONDUCT.md From 7b8ab9cfa6c9ec70592141f18f3de90ca89be490 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 6 Jul 2020 13:49:07 -0600 Subject: [PATCH 121/148] add meejah to code-of-conduct --- docs/CODE_OF_CONDUCT.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/CODE_OF_CONDUCT.md b/docs/CODE_OF_CONDUCT.md index 868f59c6a..3b14f2d70 100644 --- a/docs/CODE_OF_CONDUCT.md +++ b/docs/CODE_OF_CONDUCT.md @@ -44,6 +44,7 @@ incident. The following community members have made themselves available for conduct issues: - Jean-Paul Calderone (jean-paul at leastauthority dot com) +- meejah (meejah at meejah dot ca) This Code of Conduct is adapted from the [Contributor Covenant][homepage], version 1.3.0, available at From 50ce511cf34efa97659643ed305504b3f278a33b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 7 Jul 2020 09:18:11 -0400 Subject: [PATCH 122/148] Perhaps this is the preferred formatting? --- README.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 274cee41d..082925670 100644 --- a/README.rst +++ b/README.rst @@ -107,4 +107,5 @@ slides. :alt: test coverage percentage :target: https://codecov.io/github/tahoe-lafs/tahoe-lafs?branch=master -.. |Contributor Covenant| image:: https://img.shields.io/badge/Contributor%20Covenant-v2.0%20adopted-ff69b4.svg :target: docs/CODE_OF_CONDUCT.md +.. |Contributor Covenant| image:: https://img.shields.io/badge/Contributor%20Covenant-v2.0%20adopted-ff69b4.svg + :target: docs/CODE_OF_CONDUCT.md From 349d34b099e29be4d1c82365822bfae8a0bb61ce Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 7 Jul 2020 09:19:30 -0400 Subject: [PATCH 123/148] add some alt text --- README.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/README.rst b/README.rst index 082925670..d3b089186 100644 --- a/README.rst +++ b/README.rst @@ -108,4 +108,5 @@ slides. :target: https://codecov.io/github/tahoe-lafs/tahoe-lafs?branch=master .. |Contributor Covenant| image:: https://img.shields.io/badge/Contributor%20Covenant-v2.0%20adopted-ff69b4.svg + :alt: code of conduct :target: docs/CODE_OF_CONDUCT.md From c4437bd19401a7f9b62111dbebc7e94a585a82ee Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 8 Jul 2020 13:20:07 -0400 Subject: [PATCH 124/148] Package Future 0.18.2 for us --- nix/future.nix | 35 +++++++++++++++++++++++++++++++++++ nix/overlays.nix | 5 +++++ nix/tahoe-lafs.nix | 1 + 3 files changed, 41 insertions(+) create mode 100644 nix/future.nix diff --git a/nix/future.nix b/nix/future.nix new file mode 100644 index 000000000..814b7c1b5 --- /dev/null +++ b/nix/future.nix @@ -0,0 +1,35 @@ +{ lib +, buildPythonPackage +, fetchPypi +}: + +buildPythonPackage rec { + pname = "future"; + version = "0.18.2"; + + src = fetchPypi { + inherit pname version; + sha256 = "sha256:0zakvfj87gy6mn1nba06sdha63rn4njm7bhh0wzyrxhcny8avgmi"; + }; + + doCheck = false; + + meta = { + description = "Clean single-source support for Python 3 and 2"; + longDescription = '' + python-future is the missing compatibility layer between Python 2 and + Python 3. It allows you to use a single, clean Python 3.x-compatible + codebase to support both Python 2 and Python 3 with minimal overhead. + + It provides future and past packages with backports and forward ports + of features from Python 3 and 2. It also comes with futurize and + pasteurize, customized 2to3-based scripts that helps you to convert + either Py2 or Py3 code easily to support both Python 2 and 3 in a + single clean Py3-style codebase, module by module. + ''; + homepage = https://python-future.org; + downloadPage = https://github.com/PythonCharmers/python-future/releases; + license = with lib.licenses; [ mit ]; + maintainers = with lib.maintainers; [ prikhi ]; + }; +} diff --git a/nix/overlays.nix b/nix/overlays.nix index 2a9fe8e5e..7f19a8f1a 100644 --- a/nix/overlays.nix +++ b/nix/overlays.nix @@ -10,6 +10,11 @@ self: super: { # NixOS autobahn package has trollius as a dependency, although # it is optional. Trollius is unmaintained and fails on CI. autobahn = python-super.callPackage ./autobahn.nix { }; + + # Porting to Python 3 is greatly aided by the future package. A + # slightly newer version than appears in nixos 19.09 is helpful. + future = python-super.callPackage ./future.nix { }; + }; }; } diff --git a/nix/tahoe-lafs.nix b/nix/tahoe-lafs.nix index cc0eea479..ae5c05f70 100644 --- a/nix/tahoe-lafs.nix +++ b/nix/tahoe-lafs.nix @@ -50,6 +50,7 @@ python.pkgs.buildPythonPackage rec { setuptoolsTrial pyasn1 zope_interface service-identity pyyaml magic-wormhole treq eliot autobahn cryptography setuptools + future ]; checkInputs = with python.pkgs; [ From 133e1bfb6f83e0f1a98e6aabf30974a2065a52b2 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 8 Jul 2020 13:20:42 -0400 Subject: [PATCH 125/148] news fragment --- newsfragments/3338.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3338.minor diff --git a/newsfragments/3338.minor b/newsfragments/3338.minor new file mode 100644 index 000000000..e69de29bb From a56d077d3dfc1b73a0f476354d521a29f6c43265 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 9 Jul 2020 16:59:04 -0400 Subject: [PATCH 126/148] Test HTML document generated by web.status.Status --- src/allmydata/test/web/test_status.py | 73 +++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 src/allmydata/test/web/test_status.py diff --git a/src/allmydata/test/web/test_status.py b/src/allmydata/test/web/test_status.py new file mode 100644 index 000000000..a09edad59 --- /dev/null +++ b/src/allmydata/test/web/test_status.py @@ -0,0 +1,73 @@ +# Tests for code in allmydata.web.status + +from bs4 import BeautifulSoup +from twisted.web.template import flattenString + +from allmydata.web.status import ( + Status, + StatusElement, +) + +from .common import ( + assert_soup_has_favicon, + assert_soup_has_text, + assert_soup_has_tag_with_content, +) +from ..common import TrialTestCase + +from .test_web import FakeHistory + +# Test that status.StatusElement can render HTML. +class StatusTests(TrialTestCase): + + def _render_status_page(self, active, recent): + elem = StatusElement(active, recent) + d = flattenString(None, elem) + return self.successResultOf(d) + + def test_status_page(self): + status = Status(FakeHistory()) + doc = self._render_status_page( + status._get_active_operations(), + status._get_recent_operations() + ) + soup = BeautifulSoup(doc, 'html5lib') + + assert_soup_has_favicon(self, soup) + + assert_soup_has_tag_with_content( + self, soup, u"title", + u"Tahoe-LAFS - Recent and Active Operations" + ) + + assert_soup_has_tag_with_content( + self, soup, u"h2", + u"Active Operations:" + ) + + assert_soup_has_tag_with_content( + self, soup, u"td", + u"retrieve" + ) + + assert_soup_has_tag_with_content( + self, soup, u"td", + u"publish" + ) + + assert_soup_has_tag_with_content( + self, soup, u"td", + u"download" + ) + + assert_soup_has_tag_with_content( + self, soup, u"td", + u"upload" + ) + + assert_soup_has_tag_with_content( + self, soup, u"h2", + "Recent Operations:" + ) + + print doc From 956c2c535d5503291dab5f4541fb5fe2ee8dd75a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 9 Jul 2020 17:09:12 -0400 Subject: [PATCH 127/148] Remove unused import --- src/allmydata/test/web/test_status.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/test/web/test_status.py b/src/allmydata/test/web/test_status.py index a09edad59..e351c13c8 100644 --- a/src/allmydata/test/web/test_status.py +++ b/src/allmydata/test/web/test_status.py @@ -10,7 +10,6 @@ from allmydata.web.status import ( from .common import ( assert_soup_has_favicon, - assert_soup_has_text, assert_soup_has_tag_with_content, ) from ..common import TrialTestCase From d4a02aa1cdcdc62a77ebd2a129d3a2d899ab1083 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 9 Jul 2020 17:14:55 -0400 Subject: [PATCH 128/148] Remove a print statement --- src/allmydata/test/web/test_status.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/allmydata/test/web/test_status.py b/src/allmydata/test/web/test_status.py index e351c13c8..0779e7d80 100644 --- a/src/allmydata/test/web/test_status.py +++ b/src/allmydata/test/web/test_status.py @@ -68,5 +68,3 @@ class StatusTests(TrialTestCase): self, soup, u"h2", "Recent Operations:" ) - - print doc From 144001f3808c86bfdc22cfe2b5c28510d3321591 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Sat, 11 Jul 2020 08:30:39 -0400 Subject: [PATCH 129/148] Use a better assert in status subpath test --- src/allmydata/test/web/test_web.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index bbd44b454..7899c696e 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1038,7 +1038,13 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi def test_status_path_error(self): # Expect an error, because path is expected to be of the form # "/status/{up,down,..}-%number", with a hyphen. - return self.assertFailure(self.GET("/storage/nodash"), error.Error) + d = self.shouldFail2(error.Error, + "test_status_path_error", + "400 Bad Request", + "no '-' in 'nodash'", + self.GET, + "/status/nodash") + return d def test_status_numbers(self): drrm = status.DownloadResultsRendererMixin() From b48556c2cb4a4c59ff5448d1035bcfee69724834 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 13 Jul 2020 20:44:30 -0400 Subject: [PATCH 130/148] Add tests for status subpages Coverage tests for the coverage gods --- src/allmydata/test/web/test_web.py | 96 ++++++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 6 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 7899c696e..05e423e05 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1038,12 +1038,96 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi def test_status_path_error(self): # Expect an error, because path is expected to be of the form # "/status/{up,down,..}-%number", with a hyphen. - d = self.shouldFail2(error.Error, - "test_status_path_error", - "400 Bad Request", - "no '-' in 'nodash'", - self.GET, - "/status/nodash") + return self.shouldFail2(error.Error, + "test_status_path_error", + "400 Bad Request", + "no '-' in 'nodash'", + self.GET, + "/status/nodash") + + def test_status_up_subpath(self): + # See that "GET /status/up-0" works. + h = self.s.get_history() + ul_num = h.list_all_upload_statuses()[0].get_counter() + d = self.GET("/status/up-{}".format(ul_num), return_response=True) + def _upload_status(result): + body, status, _ = result + self.failUnlessReallyEqual(int(status), 200) + soup = BeautifulSoup(body, 'html5lib') + assert_soup_has_favicon(self, soup) + assert_soup_has_tag_with_content( + self, soup, u"title", + u"Tahoe-LAFS - File Upload Status" + ) + d.addCallback(_upload_status) + return d + + def test_status_down_subpath(self): + # See that "GET /status/down-0" works. + h = self.s.get_history() + dl_num = h.list_all_download_statuses()[0].get_counter() + d = self.GET("/status/down-{}".format(dl_num), return_response=True) + def _download_status(result): + body, status, _ = result + self.failUnlessReallyEqual(int(status), 200) + soup = BeautifulSoup(body, 'html5lib') + assert_soup_has_favicon(self, soup) + assert_soup_has_tag_with_content( + self, soup, u"title", + u"Tahoe-LAFS - File Download Status" + ) + d.addCallback(_download_status) + return d + + def test_status_mapupdate_subpath(self): + # See that "GET /status/mapupdate-0" works. + h = self.s.get_history() + mu_num = h.list_all_mapupdate_statuses()[0].get_counter() + d = self.GET("/status/mapupdate-{}".format(mu_num), return_response=True) + def _mapupdate_status(result): + body, status, _ = result + self.failUnlessReallyEqual(int(status), 200) + soup = BeautifulSoup(body, 'html5lib') + assert_soup_has_favicon(self, soup) + assert_soup_has_tag_with_content( + self, soup, u"title", + u"Tahoe-LAFS - Mutable File Servermap Update Status" + ) + d.addCallback(_mapupdate_status) + return d + + def test_status_publish_subpath(self): + # See that "GET /status/publish-0" works. + h = self.s.get_history() + pub_num = h.list_all_publish_statuses()[0].get_counter() + d = self.GET("/status/publish-{}".format(pub_num), return_response=True) + def _mapupdate_status(result): + body, status, _ = result + self.failUnlessReallyEqual(int(status), 200) + soup = BeautifulSoup(body, 'html5lib') + assert_soup_has_favicon(self, soup) + assert_soup_has_tag_with_content( + self, soup, u"title", + u"Tahoe-LAFS - Mutable File Publish Status" + ) + d.addCallback(_mapupdate_status) + return d + + def test_status_retrieve_subpath(self): + # See that "GET /status/retrieve-0" works. + h = self.s.get_history() + ret_num = h.list_all_retrieve_statuses()[0].get_counter() + d = self.GET("/status/retrieve-{}".format(ret_num), return_response=True) + def _mapupdate_status(result): + body, status, _ = result + self.failUnlessReallyEqual(int(status), 200) + soup = BeautifulSoup(body, 'html5lib') + assert_soup_has_favicon(self, soup) + assert_soup_has_tag_with_content( + self, soup, u"title", + u"Tahoe-LAFS - Mutable File Retrieve Status" + ) + d.addCallback(_mapupdate_status) return d def test_status_numbers(self): From abd539588f63ab77e63b00cd32ae81cd58b0d833 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 13 Jul 2020 20:50:55 -0400 Subject: [PATCH 131/148] Rename nested functions for clarity ... and also to remove embarrassing copy-pasta mistakes. --- src/allmydata/test/web/test_web.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 05e423e05..aa53c7d34 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1050,7 +1050,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi h = self.s.get_history() ul_num = h.list_all_upload_statuses()[0].get_counter() d = self.GET("/status/up-{}".format(ul_num), return_response=True) - def _upload_status(result): + def _upload_status_check(result): body, status, _ = result self.failUnlessReallyEqual(int(status), 200) soup = BeautifulSoup(body, 'html5lib') @@ -1059,7 +1059,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self, soup, u"title", u"Tahoe-LAFS - File Upload Status" ) - d.addCallback(_upload_status) + d.addCallback(_upload_status_check) return d def test_status_down_subpath(self): @@ -1067,7 +1067,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi h = self.s.get_history() dl_num = h.list_all_download_statuses()[0].get_counter() d = self.GET("/status/down-{}".format(dl_num), return_response=True) - def _download_status(result): + def _download_status_check(result): body, status, _ = result self.failUnlessReallyEqual(int(status), 200) soup = BeautifulSoup(body, 'html5lib') @@ -1076,7 +1076,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self, soup, u"title", u"Tahoe-LAFS - File Download Status" ) - d.addCallback(_download_status) + d.addCallback(_download_status_check) return d def test_status_mapupdate_subpath(self): @@ -1084,7 +1084,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi h = self.s.get_history() mu_num = h.list_all_mapupdate_statuses()[0].get_counter() d = self.GET("/status/mapupdate-{}".format(mu_num), return_response=True) - def _mapupdate_status(result): + def _mapupdate_status_check(result): body, status, _ = result self.failUnlessReallyEqual(int(status), 200) soup = BeautifulSoup(body, 'html5lib') @@ -1093,7 +1093,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self, soup, u"title", u"Tahoe-LAFS - Mutable File Servermap Update Status" ) - d.addCallback(_mapupdate_status) + d.addCallback(_mapupdate_status_check) return d def test_status_publish_subpath(self): @@ -1101,7 +1101,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi h = self.s.get_history() pub_num = h.list_all_publish_statuses()[0].get_counter() d = self.GET("/status/publish-{}".format(pub_num), return_response=True) - def _mapupdate_status(result): + def _publish_status_check(result): body, status, _ = result self.failUnlessReallyEqual(int(status), 200) soup = BeautifulSoup(body, 'html5lib') @@ -1110,7 +1110,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self, soup, u"title", u"Tahoe-LAFS - Mutable File Publish Status" ) - d.addCallback(_mapupdate_status) + d.addCallback(_publish_status_check) return d def test_status_retrieve_subpath(self): @@ -1118,7 +1118,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi h = self.s.get_history() ret_num = h.list_all_retrieve_statuses()[0].get_counter() d = self.GET("/status/retrieve-{}".format(ret_num), return_response=True) - def _mapupdate_status(result): + def _retrieve_status_check(result): body, status, _ = result self.failUnlessReallyEqual(int(status), 200) soup = BeautifulSoup(body, 'html5lib') @@ -1127,7 +1127,7 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self, soup, u"title", u"Tahoe-LAFS - Mutable File Retrieve Status" ) - d.addCallback(_mapupdate_status) + d.addCallback(_retrieve_status_check) return d def test_status_numbers(self): From 54e952ceda381a0bef6ee27bc9457b83f10a7537 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 14 Jul 2020 09:01:31 -0400 Subject: [PATCH 132/148] Test for 404 responses from status page subpaths --- src/allmydata/test/web/test_web.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index aa53c7d34..f83c44b2f 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1045,6 +1045,27 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self.GET, "/status/nodash") + def test_status_path_404(self): + # The test suite (hopefully!) would not have done any setup + # for a very large number of statuses at this point, so these + # all should return 404. + d = self.GET("/status/up-9999999") + d.addBoth(self.should404, "test_status_path_error (up)") + + d = self.GET("/status/down-9999999") + d.addBoth(self.should404, "test_status_path_error (down)") + + d = self.GET("/status/mapupdate-9999999") + d.addBoth(self.should404, "test_status_path_error (mapupdate)") + + d = self.GET("/status/publish-9999999") + d.addBoth(self.should404, "test_status_path_error (publish)") + + d = self.GET("/status/retrieve-9999999") + d.addBoth(self.should404, "test_status_path_error (retrieve)") + + return d + def test_status_up_subpath(self): # See that "GET /status/up-0" works. h = self.s.get_history() From 6a2ce9efc6d897348d3eaac1209d8862e1ad65aa Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 16 Jul 2020 16:55:12 -0400 Subject: [PATCH 133/148] Use docstrings --- src/allmydata/test/web/test_status.py | 4 +++- src/allmydata/test/web/test_web.py | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/web/test_status.py b/src/allmydata/test/web/test_status.py index 0779e7d80..7f4227f17 100644 --- a/src/allmydata/test/web/test_status.py +++ b/src/allmydata/test/web/test_status.py @@ -1,4 +1,6 @@ -# Tests for code in allmydata.web.status +""" +Tests for ```allmydata.web.status```. +""" from bs4 import BeautifulSoup from twisted.web.template import flattenString diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index f83c44b2f..f0d5c10dc 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1036,8 +1036,10 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return d def test_status_path_error(self): - # Expect an error, because path is expected to be of the form - # "/status/{up,down,..}-%number", with a hyphen. + """ + Expect an error, because path is expected to be of the form + "/status/{up,down,..}-%number", with a hyphen. + """ return self.shouldFail2(error.Error, "test_status_path_error", "400 Bad Request", From e48d9306217007f5d7901a5ceac9539866963856 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 16 Jul 2020 16:56:41 -0400 Subject: [PATCH 134/148] Clarify intention of a test case --- src/allmydata/test/web/test_web.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index f0d5c10dc..11b024f4d 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1048,9 +1048,14 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi "/status/nodash") def test_status_path_404(self): - # The test suite (hopefully!) would not have done any setup - # for a very large number of statuses at this point, so these - # all should return 404. + """ + Looking for non-existent statuses under child paths should + exercises all the iterators in web.status.Status.getChild(). + + The test suite (hopefully!) would not have done any setup for + a very large number of statuses at this point, now or in the + future, so these all should always return 404. + """ d = self.GET("/status/up-9999999") d.addBoth(self.should404, "test_status_path_error (up)") From 95642ae02c36236ea21004f0ea0e1ec3b804002a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 16 Jul 2020 17:13:36 -0400 Subject: [PATCH 135/148] Use a helper to check status subpath results --- src/allmydata/test/web/test_web.py | 69 ++++++++---------------------- 1 file changed, 19 insertions(+), 50 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 11b024f4d..45bc7034f 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1073,21 +1073,22 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return d + def _check_status_subpath_result(self, result, expected_title): + body, status, _ = result + self.failUnlessReallyEqual(int(status), 200) + soup = BeautifulSoup(body, 'html5lib') + assert_soup_has_favicon(self, soup) + assert_soup_has_tag_with_content( + self, soup, u"title", expected_title + ) + def test_status_up_subpath(self): # See that "GET /status/up-0" works. h = self.s.get_history() ul_num = h.list_all_upload_statuses()[0].get_counter() d = self.GET("/status/up-{}".format(ul_num), return_response=True) - def _upload_status_check(result): - body, status, _ = result - self.failUnlessReallyEqual(int(status), 200) - soup = BeautifulSoup(body, 'html5lib') - assert_soup_has_favicon(self, soup) - assert_soup_has_tag_with_content( - self, soup, u"title", - u"Tahoe-LAFS - File Upload Status" - ) - d.addCallback(_upload_status_check) + d.addCallback(self._check_status_subpath_result, + u"Tahoe-LAFS - File Upload Status") return d def test_status_down_subpath(self): @@ -1095,16 +1096,8 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi h = self.s.get_history() dl_num = h.list_all_download_statuses()[0].get_counter() d = self.GET("/status/down-{}".format(dl_num), return_response=True) - def _download_status_check(result): - body, status, _ = result - self.failUnlessReallyEqual(int(status), 200) - soup = BeautifulSoup(body, 'html5lib') - assert_soup_has_favicon(self, soup) - assert_soup_has_tag_with_content( - self, soup, u"title", - u"Tahoe-LAFS - File Download Status" - ) - d.addCallback(_download_status_check) + d.addCallback(self._check_status_subpath_result, + u"Tahoe-LAFS - File Download Status") return d def test_status_mapupdate_subpath(self): @@ -1112,16 +1105,8 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi h = self.s.get_history() mu_num = h.list_all_mapupdate_statuses()[0].get_counter() d = self.GET("/status/mapupdate-{}".format(mu_num), return_response=True) - def _mapupdate_status_check(result): - body, status, _ = result - self.failUnlessReallyEqual(int(status), 200) - soup = BeautifulSoup(body, 'html5lib') - assert_soup_has_favicon(self, soup) - assert_soup_has_tag_with_content( - self, soup, u"title", - u"Tahoe-LAFS - Mutable File Servermap Update Status" - ) - d.addCallback(_mapupdate_status_check) + d.addCallback(self._check_status_subpath_result, + u"Tahoe-LAFS - Mutable File Servermap Update Status") return d def test_status_publish_subpath(self): @@ -1129,16 +1114,8 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi h = self.s.get_history() pub_num = h.list_all_publish_statuses()[0].get_counter() d = self.GET("/status/publish-{}".format(pub_num), return_response=True) - def _publish_status_check(result): - body, status, _ = result - self.failUnlessReallyEqual(int(status), 200) - soup = BeautifulSoup(body, 'html5lib') - assert_soup_has_favicon(self, soup) - assert_soup_has_tag_with_content( - self, soup, u"title", - u"Tahoe-LAFS - Mutable File Publish Status" - ) - d.addCallback(_publish_status_check) + d.addCallback(self._check_status_subpath_result, + u"Tahoe-LAFS - Mutable File Publish Status") return d def test_status_retrieve_subpath(self): @@ -1146,16 +1123,8 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi h = self.s.get_history() ret_num = h.list_all_retrieve_statuses()[0].get_counter() d = self.GET("/status/retrieve-{}".format(ret_num), return_response=True) - def _retrieve_status_check(result): - body, status, _ = result - self.failUnlessReallyEqual(int(status), 200) - soup = BeautifulSoup(body, 'html5lib') - assert_soup_has_favicon(self, soup) - assert_soup_has_tag_with_content( - self, soup, u"title", - u"Tahoe-LAFS - Mutable File Retrieve Status" - ) - d.addCallback(_retrieve_status_check) + d.addCallback(self._check_status_subpath_result, + u"Tahoe-LAFS - Mutable File Retrieve Status") return d def test_status_numbers(self): From 8db60e3486d78e1b628faaf8b408942251471d4a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 16 Jul 2020 17:16:18 -0400 Subject: [PATCH 136/148] Do the simple thing when testing status subpaths --- src/allmydata/test/web/test_web.py | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 45bc7034f..c8f954d01 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1084,45 +1084,35 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi def test_status_up_subpath(self): # See that "GET /status/up-0" works. - h = self.s.get_history() - ul_num = h.list_all_upload_statuses()[0].get_counter() - d = self.GET("/status/up-{}".format(ul_num), return_response=True) + d = self.GET("/status/up-0", return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - File Upload Status") return d def test_status_down_subpath(self): # See that "GET /status/down-0" works. - h = self.s.get_history() - dl_num = h.list_all_download_statuses()[0].get_counter() - d = self.GET("/status/down-{}".format(dl_num), return_response=True) + d = self.GET("/status/down-0", return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - File Download Status") return d def test_status_mapupdate_subpath(self): # See that "GET /status/mapupdate-0" works. - h = self.s.get_history() - mu_num = h.list_all_mapupdate_statuses()[0].get_counter() - d = self.GET("/status/mapupdate-{}".format(mu_num), return_response=True) + d = self.GET("/status/mapupdate-0", return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - Mutable File Servermap Update Status") return d def test_status_publish_subpath(self): # See that "GET /status/publish-0" works. - h = self.s.get_history() - pub_num = h.list_all_publish_statuses()[0].get_counter() - d = self.GET("/status/publish-{}".format(pub_num), return_response=True) + d = self.GET("/status/publish-0", return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - Mutable File Publish Status") return d def test_status_retrieve_subpath(self): # See that "GET /status/retrieve-0" works. - h = self.s.get_history() - ret_num = h.list_all_retrieve_statuses()[0].get_counter() - d = self.GET("/status/retrieve-{}".format(ret_num), return_response=True) + d = self.GET("/status/retrieve-0", return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - Mutable File Retrieve Status") return d From 66dd2d365829685edaa09f0c11fba99da0bf5b25 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 16 Jul 2020 17:19:50 -0400 Subject: [PATCH 137/148] Use docstrings --- src/allmydata/test/web/test_web.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index c8f954d01..e7832c81c 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1074,6 +1074,10 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return d def _check_status_subpath_result(self, result, expected_title): + """ + Helper to verify that results of "GET /status/up-0" and + similar are as expected. + """ body, status, _ = result self.failUnlessReallyEqual(int(status), 200) soup = BeautifulSoup(body, 'html5lib') @@ -1083,35 +1087,45 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi ) def test_status_up_subpath(self): - # See that "GET /status/up-0" works. + """ + See that "GET /status/up-0" works. + """ d = self.GET("/status/up-0", return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - File Upload Status") return d def test_status_down_subpath(self): - # See that "GET /status/down-0" works. + """ + See that "GET /status/down-0" works. + """ d = self.GET("/status/down-0", return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - File Download Status") return d def test_status_mapupdate_subpath(self): - # See that "GET /status/mapupdate-0" works. + """ + See that "GET /status/mapupdate-0" works. + """ d = self.GET("/status/mapupdate-0", return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - Mutable File Servermap Update Status") return d def test_status_publish_subpath(self): - # See that "GET /status/publish-0" works. + """ + See that "GET /status/publish-0" works. + """ d = self.GET("/status/publish-0", return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - Mutable File Publish Status") return d def test_status_retrieve_subpath(self): - # See that "GET /status/retrieve-0" works. + """ + See that "GET /status/retrieve-0" works. + """ d = self.GET("/status/retrieve-0", return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - Mutable File Retrieve Status") From 06c8bbfb19bb0194a6007305dca313322e2c652b Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 16 Jul 2020 17:25:25 -0400 Subject: [PATCH 138/148] Use clearer names for tests --- src/allmydata/test/web/test_web.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index e7832c81c..97a997aa5 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1035,19 +1035,19 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi return d - def test_status_path_error(self): + def test_status_path_nodash_error(self): """ Expect an error, because path is expected to be of the form "/status/{up,down,..}-%number", with a hyphen. """ return self.shouldFail2(error.Error, - "test_status_path_error", + "test_status_path_nodash", "400 Bad Request", "no '-' in 'nodash'", self.GET, "/status/nodash") - def test_status_path_404(self): + def test_status_path_404_error(self): """ Looking for non-existent statuses under child paths should exercises all the iterators in web.status.Status.getChild(). @@ -1057,19 +1057,19 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi future, so these all should always return 404. """ d = self.GET("/status/up-9999999") - d.addBoth(self.should404, "test_status_path_error (up)") + d.addBoth(self.should404, "test_status_path_404_error (up)") d = self.GET("/status/down-9999999") - d.addBoth(self.should404, "test_status_path_error (down)") + d.addBoth(self.should404, "test_status_path_404_error (down)") d = self.GET("/status/mapupdate-9999999") - d.addBoth(self.should404, "test_status_path_error (mapupdate)") + d.addBoth(self.should404, "test_status_path_404_error (mapupdate)") d = self.GET("/status/publish-9999999") - d.addBoth(self.should404, "test_status_path_error (publish)") + d.addBoth(self.should404, "test_status_path_404_error (publish)") d = self.GET("/status/retrieve-9999999") - d.addBoth(self.should404, "test_status_path_error (retrieve)") + d.addBoth(self.should404, "test_status_path_404_error (retrieve)") return d From fe165cc99d7534f0fc139dc9429bf0757ba901ad Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 16 Jul 2020 19:07:51 -0400 Subject: [PATCH 139/148] Do the right thing, not the simple thing Seems that NixOS CI is saying that the simple thing is not the right thing: "GET /status/down-0" etc all resulted in 404 responses on NixOS. So we perhaps should use the actual counter value. --- src/allmydata/test/web/test_web.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 97a997aa5..f7158918c 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1090,7 +1090,9 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi """ See that "GET /status/up-0" works. """ - d = self.GET("/status/up-0", return_response=True) + h = self.s.get_history() + ul_num = h.list_all_upload_statuses()[0].get_counter() + d = self.GET("/status/up-{}".format(ul_num), return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - File Upload Status") return d @@ -1099,7 +1101,9 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi """ See that "GET /status/down-0" works. """ - d = self.GET("/status/down-0", return_response=True) + h = self.s.get_history() + dl_num = h.list_all_download_statuses()[0].get_counter() + d = self.GET("/status/down-{}".format(dl_num), return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - File Download Status") return d @@ -1108,7 +1112,9 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi """ See that "GET /status/mapupdate-0" works. """ - d = self.GET("/status/mapupdate-0", return_response=True) + h = self.s.get_history() + mu_num = h.list_all_mapupdate_statuses()[0].get_counter() + d = self.GET("/status/mapupdate-{}".format(mu_num), return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - Mutable File Servermap Update Status") return d @@ -1117,7 +1123,9 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi """ See that "GET /status/publish-0" works. """ - d = self.GET("/status/publish-0", return_response=True) + h = self.s.get_history() + pub_num = h.list_all_publish_statuses()[0].get_counter() + d = self.GET("/status/publish-{}".format(pub_num), return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - Mutable File Publish Status") return d @@ -1126,7 +1134,9 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi """ See that "GET /status/retrieve-0" works. """ - d = self.GET("/status/retrieve-0", return_response=True) + h = self.s.get_history() + ret_num = h.list_all_retrieve_statuses()[0].get_counter() + d = self.GET("/status/retrieve-{}".format(ret_num), return_response=True) d.addCallback(self._check_status_subpath_result, u"Tahoe-LAFS - Mutable File Retrieve Status") return d From 372cc00a25d91c0e4076092d86da7d615b03645d Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 17 Jul 2020 09:46:21 -0400 Subject: [PATCH 140/148] Reject non-existent child nodes of /status Requests on both `/status` and `/status/` will be served with the same status page, but `/status//`, `/status///` and so on will be processed further down, resulting in a 40x response. --- src/allmydata/web/status.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 39b19cb0b..2c19c45dd 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -1214,7 +1214,7 @@ class Status(MultiFormatResource): # final URL segment will be an empty string. Resources can # thus know if they were requested with or without a final # slash." - if not path: + if not path and request.postpath != ['']: return self h = self.history From 02d8bd061afcfa85d3cf7d5e986ca4cba056de5a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 17 Jul 2020 10:49:30 -0400 Subject: [PATCH 141/148] Test for trailing slashes after `/status/` Both `/status` and `/status/` are treated alike; any additional trailing slashes will be rejected. --- src/allmydata/test/web/test_web.py | 40 ++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index f7158918c..850d494ab 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1047,6 +1047,46 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self.GET, "/status/nodash") + def test_status_path_trailing_slashes(self): + """ + Test that both `GET /status` and `GET /status/` are treated + alike, but reject any additional trailing slashes and other + non-existent child nodes. + """ + def _check_status(response): + (body, status, _) = response + + self.failUnlessReallyEqual(int(status), 200) + + soup = BeautifulSoup(body, 'html5lib') + assert_soup_has_favicon(self, soup) + assert_soup_has_tag_with_content( + self, soup, u"title", + u"Tahoe-LAFS - Recent and Active Operations" + ) + + d = self.GET("/status", return_response=True) + d.addCallback(_check_status) + + d = self.GET("/status/", return_response=True) + d.addCallback(_check_status) + + d = self.shouldFail2(error.Error, + "test_status_path_trailing_slashes", + "400 Bad Request", + "no '-' in ''", + self.GET, + "/status//") + + d = self.shouldFail2(error.Error, + "test_status_path_trailing_slashes", + "400 Bad Request", + "no '-' in ''", + self.GET, + "/status////////") + + return d + def test_status_path_404_error(self): """ Looking for non-existent statuses under child paths should From 2d19c383ca2ea78967537c54e98da6bb71907b4c Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 17 Jul 2020 10:58:01 -0400 Subject: [PATCH 142/148] Rename a variable for clarity --- src/allmydata/web/status.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 2c19c45dd..202fbaa61 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -1290,8 +1290,8 @@ class StatusElement(Element): @renderer def recent_operations(self, req, tag): - active = [self.get_op_state(op) for op in self._recent] - return SlotsSequenceElement(tag, active) + recent = [self.get_op_state(op) for op in self._recent] + return SlotsSequenceElement(tag, recent) @staticmethod def get_op_state(op): From 5c886b1b2d6e3b6929b12d5723afd8039e6cbcb5 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 17 Jul 2020 10:58:34 -0400 Subject: [PATCH 143/148] Format link to status page child node correctly Some Nevow-ism accidentally escaped earlier: link to `/status/up-0` and so on were actually rendered as `/up-0` and so on. --- src/allmydata/web/status.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 202fbaa61..6ff619504 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -1344,7 +1344,8 @@ class StatusElement(Element): link = "mapupdate-%d" % op.get_counter() result["progress"] = "%.1f%%" % (100.0 * progress) - result["status"] = tags.a(op.get_status(), href=link) + result["status"] = tags.a(op.get_status(), + href="/status/{}".format(link)) return result From 828ec53e24fb77c70e22550f2c15eb395a558ae1 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 17 Jul 2020 11:28:19 -0400 Subject: [PATCH 144/148] Update status page test case Link to upload status 0 should contain `/status/` prefix, or it will navigate to `/up-0` instead of `/status/up-0`, resulting in a 404 not found message. Same with other statuses too. --- src/allmydata/test/web/test_web.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 850d494ab..1c155f6ce 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -972,11 +972,11 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi d = self.GET("/status", followRedirect=True) def _check(res): self.failUnlessIn('Recent and Active Operations', res) - self.failUnlessIn('"down-%d"' % dl_num, res) - self.failUnlessIn('"up-%d"' % ul_num, res) - self.failUnlessIn('"mapupdate-%d"' % mu_num, res) - self.failUnlessIn('"publish-%d"' % pub_num, res) - self.failUnlessIn('"retrieve-%d"' % ret_num, res) + self.failUnlessIn('"/status/down-%d"' % dl_num, res) + self.failUnlessIn('"/status/up-%d"' % ul_num, res) + self.failUnlessIn('"/status/mapupdate-%d"' % mu_num, res) + self.failUnlessIn('"/status/publish-%d"' % pub_num, res) + self.failUnlessIn('"/status/retrieve-%d"' % ret_num, res) d.addCallback(_check) d.addCallback(lambda res: self.GET("/status/?t=json")) def _check_json(res): From 8fd0cf7eb6de6b57ad0e659f7694d89c1fa93ed3 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 17 Jul 2020 11:40:18 -0400 Subject: [PATCH 145/148] Add a test for checking status page links --- src/allmydata/test/web/test_web.py | 52 ++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 1c155f6ce..d787124eb 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1047,6 +1047,58 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self.GET, "/status/nodash") + def test_status_page_contains_links(self): + """ + Check that the rendered `/status` page contains all the + expected links. + """ + def _check_status_page_links(response): + (body, status, _) = response + + self.failUnlessReallyEqual(int(status), 200) + + soup = BeautifulSoup(body, 'html5lib') + + # Check for `Not started` + assert_soup_has_tag_with_attributes_and_content( + self, soup, u"a", + u"Not started", + {u"href": u"/status/retrieve-0"} + ) + + # Check for `Not started` + assert_soup_has_tag_with_attributes_and_content( + self, soup, u"a", + u"Not started", + {u"href": u"/status/publish-0"} + ) + + # Check for `Not started` + assert_soup_has_tag_with_attributes_and_content( + self, soup, u"a", + u"Not started", + {u"href": u"/status/mapupdate-0"} + ) + + # Check for `fetching segments + # 2,3; errors on segment 1`: see build_one_ds() above. + assert_soup_has_tag_with_attributes_and_content( + self, soup, u"a", + u"fetching segments 2,3; errors on segment 1", + {u"href": u"/status/down-0"} + ) + + # Check for `Not started` + assert_soup_has_tag_with_attributes_and_content( + self, soup, u"a", + u"Not started", + {u"href": u"/status/up-0"} + ) + + d = self.GET("/status", return_response=True) + d.addCallback(_check_status_page_links) + return d + def test_status_path_trailing_slashes(self): """ Test that both `GET /status` and `GET /status/` are treated From 293efd7c9fe929e3e6fd029e1f1f1da91440c387 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 17 Jul 2020 13:50:06 -0400 Subject: [PATCH 146/148] Use actual status counter number when testing --- src/allmydata/test/web/test_web.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index d787124eb..1d235af63 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1058,41 +1058,47 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi self.failUnlessReallyEqual(int(status), 200) soup = BeautifulSoup(body, 'html5lib') + h = self.s.get_history() # Check for `Not started` + ret_num = h.list_all_retrieve_statuses()[0].get_counter() assert_soup_has_tag_with_attributes_and_content( self, soup, u"a", u"Not started", - {u"href": u"/status/retrieve-0"} + {u"href": u"/status/retrieve-{}".format(ret_num)} ) # Check for `Not started` + pub_num = h.list_all_publish_statuses()[0].get_counter() assert_soup_has_tag_with_attributes_and_content( self, soup, u"a", u"Not started", - {u"href": u"/status/publish-0"} + {u"href": u"/status/publish-{}".format(pub_num)} ) # Check for `Not started` + mu_num = h.list_all_mapupdate_statuses()[0].get_counter() assert_soup_has_tag_with_attributes_and_content( self, soup, u"a", u"Not started", - {u"href": u"/status/mapupdate-0"} + {u"href": u"/status/mapupdate-{}".format(mu_num)} ) # Check for `fetching segments # 2,3; errors on segment 1`: see build_one_ds() above. + dl_num = h.list_all_download_statuses()[0].get_counter() assert_soup_has_tag_with_attributes_and_content( self, soup, u"a", u"fetching segments 2,3; errors on segment 1", - {u"href": u"/status/down-0"} + {u"href": u"/status/down-{}".format(dl_num)} ) # Check for `Not started` + ul_num = h.list_all_upload_statuses()[0].get_counter() assert_soup_has_tag_with_attributes_and_content( self, soup, u"a", u"Not started", - {u"href": u"/status/up-0"} + {u"href": u"/status/up-{}".format(ul_num)} ) d = self.GET("/status", return_response=True) From aab598c2f54b0beb3373400250ad8babff13149a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 17 Jul 2020 14:30:17 -0400 Subject: [PATCH 147/148] Update integration test for status page As a fallout of transition from nevow to twisted web, list of hrefs in the parsed status page will be [u'/status/down-0', u'/status/up-0', u'/'] (with an added '/status/' prefix) so we need to update our assumptions. See 5c886b1b2 for the change that necessitated this. --- integration/test_web.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/integration/test_web.py b/integration/test_web.py index 4ba0a6fd1..575e4fc1a 100644 --- a/integration/test_web.py +++ b/integration/test_web.py @@ -219,23 +219,21 @@ def test_status(alice): found_upload = False found_download = False for href in hrefs: - if href.startswith(u"/") or not href: + if href == u"/" or not href: continue - resp = requests.get( - util.node_url(alice.node_dir, u"status/{}".format(href)), - ) - if href.startswith(u'up'): + resp = requests.get(util.node_url(alice.node_dir, href)) + if href.startswith(u"/status/up"): assert "File Upload Status" in resp.content if "Total Size: {}".format(len(FILE_CONTENTS)) in resp.content: found_upload = True - elif href.startswith(u'down'): + elif href.startswith(u"/status/down"): assert "File Download Status" in resp.content if "Total Size: {}".format(len(FILE_CONTENTS)) in resp.content: found_download = True # download the specialized event information resp = requests.get( - util.node_url(alice.node_dir, u"status/{}/event_json".format(href)), + util.node_url(alice.node_dir, u"{}/event_json".format(href)), ) js = json.loads(resp.content) # there's usually just one "read" operation, but this can handle many .. From c58cf56e83569f46d845bd90d0ea66e2b1643196 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Sat, 18 Jul 2020 08:34:25 -0400 Subject: [PATCH 148/148] Remove unused imports --- src/allmydata/web/status.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 9637e2403..3a89e8f5e 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -13,10 +13,8 @@ from twisted.web.template import ( renderElement, tags, ) -from nevow import rend, tags as T from allmydata.util import base32, idlib from allmydata.web.common import ( - getxmlfile, abbreviate_time, abbreviate_rate, abbreviate_size,