~sircmpwn/sr.ht-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3

[PATCH man.sr.ht 0/3] Support repo rename/delete for git-backed wikis

Details
Message ID
<20190801101333.25572-1-rycwo@posteo.net>
DKIM signature
missing
Download raw message
I've addressed a couple of the points ddevault mentioned on #sr.ht and squashed
a couple of additional bugs I found. Not 100% sure about deleting the wiki if
the repo is deleted but that _seems_ to make the most sense to me. Maybe there
needs to be an additional warning on the git.sr.ht side, "if this is used to
back a wiki, the wiki will be deleted too!"

These changes should be applied on top of the backend-overhaul branch.

Ryan Chan (3):
  Fix deletion of root wiki
  Subscribe to webhooks for repo update/delete
  Nullify commit fields for deleted refs

 ...3db6b65_add_resource_id_to_backing_repo.py | 24 +++++++
 .../e24445a6fabb_add_repo_webhooks_to_user.py | 24 +++++++
 mansrht/blueprints/create.py                  | 23 ++++--
 mansrht/blueprints/manage.py                  |  2 +
 mansrht/blueprints/notify.py                  | 71 +++++++++++++++----
 mansrht/repo.py                               | 71 ++++++++++++++-----
 mansrht/types/__init__.py                     |  4 +-
 mansrht/types/repo.py                         |  3 +-
 mansrht/wikis.py                              | 30 +++++---
 9 files changed, 203 insertions(+), 49 deletions(-)
 create mode 100644 mansrht/alembic/versions/c53623db6b65_add_resource_id_to_backing_repo.py
 create mode 100644 mansrht/alembic/versions/e24445a6fabb_add_repo_webhooks_to_user.py

--
2.22.0

[PATCH 1/3] Fix deletion of root wiki

Details
Message ID
<20190801101333.25572-2-rycwo@posteo.net>
In-Reply-To
<20190801101333.25572-1-rycwo@posteo.net> (view parent)
DKIM signature
missing
Download raw message
Patch: +3 -5
---
 mansrht/wikis.py | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/mansrht/wikis.py b/mansrht/wikis.py
index 0d050d0..6a80ee7 100644
--- a/mansrht/wikis.py
+++ b/mansrht/wikis.py
@@ -75,11 +75,9 @@ def delete_wiki(wiki, owner, delete_from_backend=False):
        backend.delete_repo(repo.name)

    root_wiki = RootWiki.query.all()
    if root_wiki:
        root_wiki = Wiki.query.filter(Wiki.id == root_wiki[0].id).first()
        if root_wiki and wiki == root_wiki:
            db.session.delete(root_wiki)
            db.session.flush()
    if root_wiki and root_wiki[0].id == wiki.id:
        db.session.delete(root_wiki[0])
        db.session.flush()

    db.session.delete(wiki)
    db.session.flush()
-- 
2.22.0

[PATCH 2/3] Subscribe to webhooks for repo update/delete

Details
Message ID
<20190801101333.25572-3-rycwo@posteo.net>
In-Reply-To
<20190801101333.25572-1-rycwo@posteo.net> (view parent)
DKIM signature
missing
Download raw message
Patch: +184 -36
Take repo:update and repo:delete events into account so that wikis are updated
accordingly. repo:post-update events now use repo ids as the callback so that
they are "name independent". Repos that are renamed are simply updated in the
db, whilst repos that are deleted also delete their corresponding repos.
---
 ...3db6b65_add_resource_id_to_backing_repo.py | 24 +++++++
 .../e24445a6fabb_add_repo_webhooks_to_user.py | 24 +++++++
 mansrht/blueprints/create.py                  | 23 ++++--
 mansrht/blueprints/manage.py                  |  2 +
 mansrht/blueprints/notify.py                  | 47 ++++++++++--
 mansrht/repo.py                               | 71 ++++++++++++++-----
 mansrht/types/__init__.py                     |  4 +-
 mansrht/types/repo.py                         |  3 +-
 mansrht/wikis.py                              | 22 ++++--
 9 files changed, 184 insertions(+), 36 deletions(-)
 create mode 100644 mansrht/alembic/versions/c53623db6b65_add_resource_id_to_backing_repo.py
 create mode 100644 mansrht/alembic/versions/e24445a6fabb_add_repo_webhooks_to_user.py

diff --git a/mansrht/alembic/versions/c53623db6b65_add_resource_id_to_backing_repo.py b/mansrht/alembic/versions/c53623db6b65_add_resource_id_to_backing_repo.py
new file mode 100644
index 0000000..0ff169b
--- /dev/null
+++ b/mansrht/alembic/versions/c53623db6b65_add_resource_id_to_backing_repo.py
@@ -0,0 +1,24 @@
"""Add resource id to backing repo

Revision ID: c53623db6b65
Revises: e24445a6fabb
Create Date: 2019-07-31 16:02:46.940040

"""

# revision identifiers, used by Alembic.
revision = 'c53623db6b65'
down_revision = 'e24445a6fabb'

from alembic import op
import sqlalchemy as sa


def upgrade():
    op.add_column("backing_repo", sa.Column("resource_id", sa.Integer))
    op.alter_column("backing_repo", "webhook_id", nullable=True)


def downgrade():
    op.drop_column("backing_repo", "resource_id")
    op.alter_column("backing_repo", "webhook_id", nullable=False)
diff --git a/mansrht/alembic/versions/e24445a6fabb_add_repo_webhooks_to_user.py b/mansrht/alembic/versions/e24445a6fabb_add_repo_webhooks_to_user.py
new file mode 100644
index 0000000..2561965
--- /dev/null
+++ b/mansrht/alembic/versions/e24445a6fabb_add_repo_webhooks_to_user.py
@@ -0,0 +1,24 @@
"""Add repo webhooks to user

Revision ID: e24445a6fabb
Revises: 391282b09533
Create Date: 2019-07-31 16:02:19.155311

"""

# revision identifiers, used by Alembic.
revision = 'e24445a6fabb'
down_revision = '391282b09533'

from alembic import op
import sqlalchemy as sa


def upgrade():
    op.add_column("user", sa.Column("repo_update_webhook", sa.Integer))
    op.add_column("user", sa.Column("repo_delete_webhook", sa.Integer))


def downgrade():
    op.drop_column("user", "repo_update_webhook")
    op.drop_column("user", "repo_delete_webhook")
diff --git a/mansrht/blueprints/create.py b/mansrht/blueprints/create.py
index 1994a68..8189519 100644
--- a/mansrht/blueprints/create.py
+++ b/mansrht/blueprints/create.py
@@ -1,5 +1,6 @@
from flask import Blueprint, render_template, abort, request, redirect
from flask_login import current_user
from srht.database import db
from srht.flask import loginrequired, session
from srht.validation import Validation
from mansrht.repo import GitsrhtBackend
@@ -46,6 +47,16 @@ def select_ref(backend, wiki_name, repo_name, new_repo):
            "select.html", typename="ref", typename_pretty="ref",
            default="wiki", items=sorted(refs, key=lambda x: x.name))

def consolidate_repo_webhooks(owner):
    backend = GitsrhtBackend(owner)
    if owner.repo_update_webhook is None:
        hook = backend.subscribe_repo_update()
        owner.repo_update_webhook = hook["id"]
    if owner.repo_delete_webhook is None:
        hook = backend.subscribe_repo_delete()
        owner.repo_delete_webhook = hook["id"]
    db.session.commit()

@create.route("/wiki/create")
@loginrequired
def create_GET():
@@ -128,16 +139,17 @@ def select_ref_POST():
        return select_ref(backend, wiki_name, repo_name,
                new_repo, **valid.kwargs)

    repo_dict = backend.get_repo(repo_name)
    if new_repo:
        # Check if a repo with the same name already exists.
        # If it does, we treat it as an error.
        valid.expect(
                backend.get_repo(repo_name) is None,
                repo_dict is None,
                "Repository already exists.",
                field="repo")
        if not valid.ok:
            return select_repo(backend, wiki_name, **valid.kwargs)
        backend.create_repo(repo_name)
        repo_dict = backend.create_repo(repo_name)

    # Try to find the latest commit if we're using an existing repo + ref.
    new_ref = request.path.endswith("new")
@@ -145,10 +157,11 @@ def select_ref_POST():
    if not new_repo and not new_ref:
        commit = backend.get_latest_commit(repo_name, ref_name)

    hook = backend.subscribe_repo_update(repo_name)
    consolidate_repo_webhooks(current_user)

    repo = create_repo(
            new_repo, repo_name, ref_name,
            hook["id"], commit=commit)
            new_repo, repo_dict["name"], repo_dict["id"], ref_name,
            current_user, commit=commit)
    create_wiki(
            valid, current_user, wiki_name,
            repo, visibility, is_root=is_root)
diff --git a/mansrht/blueprints/manage.py b/mansrht/blueprints/manage.py
index b3b1692..8fde255 100644
--- a/mansrht/blueprints/manage.py
+++ b/mansrht/blueprints/manage.py
@@ -51,5 +51,7 @@ def delete_POST(owner_name, wiki_name):

    # check_access() guarantees owner and wiki are valid.
    owner, wiki = check_access(owner_name, wiki_name, UserAccess.manage)
    backend = GitsrhtBackend(owner)
    backend.unsubscribe_repo_postupdate(wiki.repo)
    delete_wiki(wiki, owner, delete_repo == "on")
    return redirect("/")
diff --git a/mansrht/blueprints/notify.py b/mansrht/blueprints/notify.py
index ffb84a8..045f095 100644
--- a/mansrht/blueprints/notify.py
+++ b/mansrht/blueprints/notify.py
@@ -2,25 +2,30 @@ from flask import Blueprint, request
from srht.database import db
from srht.flask import csrf_bypass
from mansrht.types import User, Wiki, BackingRepo
from mansrht.wikis import delete_wiki
import json

webhooks_notify = Blueprint("webhooks.notify", __name__)

@csrf_bypass
@webhooks_notify.route("/webhook/notify/<reponame>/refs", methods=["POST"])
def ref_update(reponame):
def check_event(request, expected):
    payload = json.loads(request.data.decode("utf-8"))
    event = request.headers.get("X-Webhook-Event")
    if event != "repo:post-update":
        return f"Unexpected event {event}"
    if event != expected:
        return payload, None
    return payload, event

@csrf_bypass
@webhooks_notify.route("/webhook/notify/<repo_id>/refs", methods=["POST"])
def ref_update(repo_id):
    payload, event = check_event(request, "repo:post-update")
    if not event:
        return f"Unexpected event {event}"
    owner = User.query.filter(
            User.username.like(payload["pusher"]["name"])).one_or_none()
    wiki = (Wiki.query.join(Wiki.repo)
            .filter(Wiki.owner_id == owner.id)
            .filter(BackingRepo.name == reponame)).one_or_none()
            .filter(BackingRepo.id == repo_id)).one_or_none()
    repo = wiki.repo

    for ref in payload["refs"]:
        if ref["name"] == f"refs/heads/{repo.ref}":
            commit = ref.get("new")
@@ -37,3 +42,31 @@ def ref_update(reponame):
    else:
        return "No wikis updated"
    return "Updated wiki SHA, thanks!"

@csrf_bypass
@webhooks_notify.route("/webhook/notify/repos/update", methods=["POST"])
def repo_update():
    payload, event = check_event(request, "repo:update")
    if not event:
        return f"Unexpected event {event}"
    repo = BackingRepo.query.filter(
            BackingRepo.resource_id == payload["id"]).one_or_none()
    if repo and payload["name"] != repo.name:
        repo.name = payload["name"]
        db.session.commit()
        return "Updated repo name"
    return "No repos updated"

@csrf_bypass
@webhooks_notify.route("/webhook/notify/repos/delete", methods=["POST"])
def repo_delete():
    payload, event = check_event(request, "repo:delete")
    if not event:
        return f"Unexpected event {event}"
    repo = BackingRepo.query.filter(
            BackingRepo.resource_id == payload["id"]).one_or_none()
    if not repo:
        return "No wikis updated"
    wiki = Wiki.query.filter(Wiki.repo_id == repo.id).one_or_none()
    delete_wiki(wiki, wiki.owner, delete_from_backend=False)
    return f"Deleted wiki {wiki.name}"
diff --git a/mansrht/repo.py b/mansrht/repo.py
index 7f70287..1b156cf 100644
--- a/mansrht/repo.py
+++ b/mansrht/repo.py
@@ -80,10 +80,22 @@ class RepoBackend(abc.ABC):
    def get_blob(self, repo_name, blob_id): pass

    @abc.abstractmethod
    def subscribe_repo_update(self, repo_name): pass
    def subscribe_repo_postupdate(self, repo_name): pass

    @abc.abstractmethod
    def unsubscribe_repo_update(self, repo): pass
    def unsubscribe_repo_postupdate(self, repo): pass

    @abc.abstractmethod
    def subscribe_repo_update(self): pass

    @abc.abstractmethod
    def unsubscribe_repo_update(self): pass

    @abc.abstractmethod
    def subscribe_repo_delete(self): pass

    @abc.abstractmethod
    def unsubscribe_repo_delete(self): pass

class GitsrhtBackend(RepoBackend):
    """
@@ -120,19 +132,14 @@ class GitsrhtBackend(RepoBackend):

    def create_repo(self, repo_name):
        if current_user == self.owner:
            # This assumes the logged-in user. So we double-check the
            # permissions match.
            url = f"{self.origin}/api/repos"
            _request_post(
                    url, current_user.oauth_token,
                    data={"name": repo_name})
            return _request_post(
                    url, self.owner.oauth_token, data={"name": repo_name})

    def delete_repo(self, repo_name):
        if current_user == self.owner:
            # This assumes the logged-in user. So we double-check the
            # permissions match.
            url = f"{self.origin}/api/repos/{repo_name}"
            _request_delete(url, current_user.oauth_token)
            _request_delete(url, self.owner.oauth_token)

    def get_repo_url(self, repo_name):
        return os.path.join(
@@ -171,15 +178,47 @@ class GitsrhtBackend(RepoBackend):
            return None
        return r.text

    def subscribe_repo_update(self, repo_name):
        url = f"{self.api_url}/repos/{repo_name}/webhooks"
    def subscribe_repo_postupdate(self, repo):
        url = f"{self.api_url}/repos/{repo.name}/webhooks"
        webhook_data = {
            "url": (origin
                + url_for("webhooks.notify.ref_update", reponame=repo_name)),
                + url_for("webhooks.notify.ref_update", repo_id=repo.id)),
            "events": ["repo:post-update"],
        }
        return _request_post(url, current_user.oauth_token, data=webhook_data)
        return _request_post(url, self.owner.oauth_token, data=webhook_data)

    def unsubscribe_repo_update(self, repo):
    def unsubscribe_repo_postupdate(self, repo):
        url = f"{self.api_url}/repos/{repo.name}/webhooks/{repo.webhook_id}"
        _request_delete(url, current_user.oauth_token)
        _request_delete(url, self.owner.oauth_token)

    def subscribe_repo_update(self):
        if current_user == self.owner:
            url = f"{self.origin}/api/user/webhooks"
            webhook_data = {
                "url": origin + url_for("webhooks.notify.repo_update"),
                "events": ["repo:update"],
            }
            return _request_post(
                    url, self.owner.oauth_token, data=webhook_data)

    def unsubscribe_repo_update(self):
        if current_user == self.owner:
            url = "{}/api/user/webhooks/{}".format(
                    self.origin, self.owner.repo_update_webhook)
            _request_delete(url, self.owner.oauth_token)

    def subscribe_repo_delete(self):
        if current_user == self.owner:
            url = f"{self.origin}/api/user/webhooks"
            webhook_data = {
                "url": origin + url_for("webhooks.notify.repo_delete"),
                "events": ["repo:delete"],
            }
            return _request_post(
                    url, self.owner.oauth_token, data=webhook_data)

    def unsubscribe_repo_delete(self):
        if current_user == self.owner:
            url = "{}/api/user/webhooks/{}".format(
                    self.origin, self.owner.repo_delete_webhook)
            _request_delete(url, self.owner.oauth_token)
diff --git a/mansrht/types/__init__.py b/mansrht/types/__init__.py
index 20a7da5..559a637 100644
--- a/mansrht/types/__init__.py
+++ b/mansrht/types/__init__.py
@@ -1,8 +1,10 @@
import sqlalchemy as sa
from srht.database import Base
from srht.oauth import ExternalUserMixin

class User(Base, ExternalUserMixin):
    pass
    repo_update_webhook = sa.Column(sa.Integer)
    repo_delete_webhook = sa.Column(sa.Integer)

from mansrht.types.repo import BackingRepo
from mansrht.types.wiki import Wiki, RootWiki, WikiVisibility
diff --git a/mansrht/types/repo.py b/mansrht/types/repo.py
index 94e90bd..b7b6838 100644
--- a/mansrht/types/repo.py
+++ b/mansrht/types/repo.py
@@ -4,6 +4,7 @@ from srht.database import Base
class BackingRepo(Base):
    __tablename__ = 'backing_repo'
    id = sa.Column(sa.Integer, primary_key=True)
    resource_id = sa.Column(sa.Integer)
    new = sa.Column(sa.Boolean, nullable=False)
    name = sa.Column(sa.Unicode(256), nullable=False)
    ref = sa.Column(sa.Unicode(1024), nullable=False)
@@ -13,4 +14,4 @@ class BackingRepo(Base):
    commit_time = sa.Column(sa.Unicode(256))
    commit_message = sa.Column(sa.Unicode(1024))
    tree_sha = sa.Column(sa.Unicode(256))
    webhook_id = sa.Column(sa.Integer, nullable=False)
    webhook_id = sa.Column(sa.Integer)
diff --git a/mansrht/wikis.py b/mansrht/wikis.py
index 6a80ee7..591d25a 100644
--- a/mansrht/wikis.py
+++ b/mansrht/wikis.py
@@ -27,10 +27,11 @@ def is_root_wiki(wiki):
        return root_wiki and wiki == root_wiki
    return False

def create_repo(is_new, name, ref, webhook_id, commit=None):
def create_repo(is_new, name, resource_id, ref, owner, commit=None):
    repo = BackingRepo()
    repo.new = is_new
    repo.name = name
    repo.resource_id = resource_id
    repo.ref = ref
    if commit:
        repo.commit_sha = commit["id"]
@@ -40,9 +41,14 @@ def create_repo(is_new, name, ref, webhook_id, commit=None):
        repo.commit_time = commit["timestamp"]
        repo.commit_message = commit["message"]
        repo.tree_sha = commit["tree"]
    repo.webhook_id = webhook_id

    db.session.add(repo)
    db.session.flush()

    backend = GitsrhtBackend(owner)
    hook = backend.subscribe_repo_postupdate(repo)
    repo.webhook_id = hook["id"]

    db.session.flush()
    db.session.commit()
    return repo
@@ -65,12 +71,16 @@ def create_wiki(valid, owner, wiki_name, repo, visibility, is_root=False):
    return wiki

def delete_wiki(wiki, owner, delete_from_backend=False):
    # The repo is always removed from the backend table. Deletion of the actual
    # repo is done separately if the user asks for that to be done.
    repo = wiki.repo
    backend = GitsrhtBackend(owner)
    backend.unsubscribe_repo_update(repo)
    repo = wiki.repo

    wiki_count = Wiki.query.filter(Wiki.owner_id == owner.id).count()
    if wiki_count == 0:
        backend.unsubscribe_repo_update()
        backend.unsubscribe_repo_delete()

    # The repo is always removed from the backend table. Deletion of the actual
    # repo is done separately if the user asks for that to be done.
    if delete_from_backend:
        backend.delete_repo(repo.name)

-- 
2.22.0

[PATCH 3/3] Nullify commit fields for deleted refs

Details
Message ID
<20190801101333.25572-4-rycwo@posteo.net>
In-Reply-To
<20190801101333.25572-1-rycwo@posteo.net> (view parent)
DKIM signature
missing
Download raw message
Patch: +16 -8
Wikis should display the "new-wiki" page when their corresponding refs are
deleted. This changes ensures that case is handled.
---
 mansrht/blueprints/notify.py | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/mansrht/blueprints/notify.py b/mansrht/blueprints/notify.py
index 045f095..d2ad381 100644
--- a/mansrht/blueprints/notify.py
+++ b/mansrht/blueprints/notify.py
@@ -29,14 +29,22 @@ def ref_update(repo_id):
    for ref in payload["refs"]:
        if ref["name"] == f"refs/heads/{repo.ref}":
            commit = ref.get("new")
            if not commit:
                break
            repo.commit_sha = commit["id"]
            repo.commit_author = commit["author"]["name"]
            repo.commit_email = commit["author"]["email"]
            repo.commit_time = commit["timestamp"]
            repo.commit_message = commit["message"]
            repo.tree_sha = commit["tree"]
            if commit:
                repo.commit_sha = commit["id"]
                repo.commit_author = commit["author"]["name"]
                repo.commit_email = commit["author"]["email"]
                repo.commit_time = commit["timestamp"]
                repo.commit_message = commit["message"]
                repo.tree_sha = commit["tree"]
            else:
                # Nullify all the fields so that the wiki defaults back to the
                # "new-wiki" page.
                repo.commit_sha = None
                repo.commit_author = None
                repo.commit_email = None
                repo.commit_time = None
                repo.commit_message = None
                repo.tree_sha = None
            db.session.commit()
            break
    else:
-- 
2.22.0
Reply to thread Export thread (mbox)