[Patchew-devel] [PATCH 07/10] models: store plugin configuration in a single, separate JSONField
Paolo Bonzini
pbonzini at redhat.com
Thu Apr 18 16:21:43 UTC 2019
This is the next step in the splitting the properties table into
logically separate components. This time it's the turn of configuration
data coming from the plugin schemas, which is extracted into a new
JSON field.
The configuration editor is simplified a bit because all the properties
are sent to the server in a single shot and all configuration is
overwritten. As a result, we no longer need the deletion API
delete-project-properties-by-prefix.
---
api/migrations/0049_project_config.py | 22 +++
.../0050_populate_project_config.py | 56 +++++++
api/models.py | 1 +
api/views.py | 30 ++--
mod.py | 139 ++++--------------
mods/git.py | 27 ++--
mods/testing.py | 7 +-
tests/patchewtest.py | 12 +-
tests/test_git.py | 11 +-
tests/test_testing.py | 35 +++--
10 files changed, 185 insertions(+), 155 deletions(-)
create mode 100644 api/migrations/0049_project_config.py
create mode 100644 api/migrations/0050_populate_project_config.py
diff --git a/api/migrations/0049_project_config.py b/api/migrations/0049_project_config.py
new file mode 100644
index 0000000..69b5bd5
--- /dev/null
+++ b/api/migrations/0049_project_config.py
@@ -0,0 +1,22 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.20 on 2019-04-18 12:56
+from __future__ import unicode_literals
+
+from django.db import migrations
+import jsonfield.encoder
+import jsonfield.fields
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('api', '0048_populate_message_flags'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='project',
+ name='config',
+ field=jsonfield.fields.JSONField(default={}, dump_kwargs={'cls': jsonfield.encoder.JSONEncoder, 'separators': (',', ':')}, load_kwargs={}),
+ ),
+ ]
diff --git a/api/migrations/0050_populate_project_config.py b/api/migrations/0050_populate_project_config.py
new file mode 100644
index 0000000..4db2e8a
--- /dev/null
+++ b/api/migrations/0050_populate_project_config.py
@@ -0,0 +1,56 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations
+from django.db.models import Q
+
+def properties_to_config(apps, schema_editor):
+ Project = apps.get_model('api', 'Project')
+ for po in Project.objects.all():
+ q = Q(name__startswith='testing.tests.') | \
+ Q(name__startswith='testing.requirements.') | \
+ Q(name__startswith='email.notifications.') | \
+ Q(name__in=('git.push_to', 'git.url_template', 'git.public_repo'))
+ props = po.projectproperty_set.filter(q)
+ config = {}
+ for p in props:
+ *path, last = p.name.split('.')
+ parent = config
+ for item in path:
+ parent = parent.setdefault(item, {})
+ parent[last] = p.value
+ #print(po, config)
+ po.config = config
+ po.save()
+ props.delete()
+
+def flatten_properties(source, prefix, result=None):
+ if result is None:
+ result = {}
+ for k, v in source.items():
+ if isinstance(v, dict):
+ flatten_properties(v, prefix + k + '.', result)
+ else:
+ result[prefix + k] = v
+ return result
+
+def config_to_properties(apps, schema_editor):
+ Project = apps.get_model('api', 'Project')
+ ProjectProperty = apps.get_model('api', 'ProjectProperty')
+ for po in Project.objects.all():
+ props = flatten_properties(po.config, '')
+ for k, v in props.items():
+ new_prop = ProjectProperty(project=po, name=k, value=v)
+ new_prop.save()
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('api', '0049_project_config'),
+ ]
+
+ operations = [
+ migrations.RunPython(properties_to_config,
+ reverse_code=config_to_properties),
+ ]
diff --git a/api/models.py b/api/models.py
index 3176559..512bf50 100644
--- a/api/models.py
+++ b/api/models.py
@@ -171,6 +171,7 @@ class Project(models.Model):
"top project which has "
"parent_project=NULL"))
maintainers = models.ManyToManyField(User, blank=True)
+ config = jsonfield.JSONField(default={})
def __str__(self):
return self.name
diff --git a/api/views.py b/api/views.py
index 887f9ed..3e70ed7 100644
--- a/api/views.py
+++ b/api/views.py
@@ -13,6 +13,7 @@ from django.contrib.auth import authenticate, login, logout
from django.http import HttpResponse, Http404
from django.core.exceptions import PermissionDenied
from django.conf import settings
+from event import declare_event
from .models import Project, Message
import json
from .search import SearchEngine
@@ -129,28 +130,27 @@ class UpdateProjectHeadView(APILoginRequiredView):
return ret
-class SetProjectPropertiesView(APILoginRequiredView):
- name = "set-project-properties"
- allowed_groups = ["maintainers"]
-
- def handle(self, request, project, properties):
- po = Project.objects.get(name=project)
- if not po.maintained_by(request.user):
- raise PermissionDenied("Access denied to this project")
- for k, v in properties.items():
- po.set_property(k, v)
+declare_event("SetProjectConfig", obj="project whose configuration was updated")
-class DeleteProjectPropertiesByPrefixView(APILoginRequiredView):
- name = "delete-project-properties-by-prefix"
+class SetProjectConfigView(APILoginRequiredView):
+ name = "set-project-config"
allowed_groups = ["maintainers"]
- def handle(self, request, project, prefix):
+ def handle(self, request, project, config):
po = Project.objects.get(name=project)
if not po.maintained_by(request.user):
raise PermissionDenied("Access denied to this project")
- for k in [x for x in po.get_properties().keys() if x.startswith(prefix)]:
- po.set_property(k, None)
+ new_config = {}
+ for k, v in properties.items():
+ *path, last = k.split('.')
+ parent = new_config
+ for item in path:
+ parent = parent.setdefault(item, {})
+ parent[last] = v
+ po.config = new_config
+ po.save()
+ emit_event("SetProjectConfig", obj=po)
def prepare_patch(p):
diff --git a/mod.py b/mod.py
index d24e924..1b750d3 100644
--- a/mod.py
+++ b/mod.py
@@ -49,29 +49,13 @@ class PatchewModule(object):
data["module"] = self
return Template(tmpl).render(Context(data))
- def _build_map_scm(self, request, project, prefix, scm):
- prefix = prefix + scm.name + "."
- def _build_map_items():
- r = {}
- for p in project.get_properties().keys():
- if not p.startswith(prefix):
- continue
- name = p[len(prefix):]
- name = name[:name.rfind(".")]
- if name in r:
- continue
- pref = prefix + name + "."
- r[name] = {
- "name": name,
- "html": self._build_one(request, project,
- pref, scm.item)
- }
- return list(r.values())
-
- schema_html = self._build_one(request, project, prefix,
- scm.item)
+ def _build_map_scm(self, request, project, prefix, config, scm):
+ schema_html = self._build_one(request, project, prefix, scm.item)
item = {"html": schema_html}
- items = _build_map_items()
+ items = [{
+ "name": name,
+ "html": self._build_one(request, project, prefix + name + ".",
+ value, scm.item)} for name, value in config.items()]
return self._render_template(request, project, TMPL_MAP,
schema=scm,
item_schema=scm.item,
@@ -79,113 +63,69 @@ class PatchewModule(object):
items=items,
item=item)
- def _build_array_scm(self, request, project, prefix, scm):
+ def _build_array_scm(self, request, project, prefix, config, scm, show_save_button):
members = [self._build_one(request, project,
- prefix, x) for x in scm.members]
- show_save_button = False
- for m in scm.members:
- if type(m) == StringSchema:
- show_save_button = True
- break
+ prefix + x.name + ".",
+ config[x.name], x) for x in scm.members]
return self._render_template(request, project, TMPL_ARRAY,
schema=scm,
members=members,
show_save_button=show_save_button,
prefix=prefix)
- def _build_string_scm(self, request, project, prefix, scm):
- prop_name = prefix + scm.name
- prop_value = project.get_property(prop_name)
+ def _build_string_scm(self, request, project, prefix, config, scm):
return self._render_template(request, project, TMPL_STRING,
schema=scm,
name=scm.name,
prefix=prefix,
- value=prop_value)
+ value=config)
- def _build_integer_scm(self, request, project, prefix, scm):
- prop_name = prefix + scm.name
- prop_value = project.get_property(prop_name)
+ def _build_integer_scm(self, request, project, config, scm):
return self._render_template(request, project, TMPL_INTEGER,
schema=scm,
name=scm.name,
prefix=prefix,
- value=prop_value)
+ value=config)
- def _build_boolean_scm(self, request, project, prefix, scm):
- prop_name = prefix + scm.name
- prop_value = project.get_property(prop_name)
+ def _build_boolean_scm(self, request, project, config, scm):
return self._render_template(request, project, TMPL_BOOLEAN,
schema=scm,
name=scm.name,
prefix=prefix,
- value=prop_value)
+ value=config)
- def _build_enum_scm(self, request, project, prefix, scm):
- prop_name = prefix + scm.name
- prop_value = project.get_property(prop_name)
+ def _build_enum_scm(self, request, project, config, scm):
return self._render_template(request, project, TMPL_ENUM,
schema=scm,
name=scm.name,
prefix=prefix,
- value=prop_value)
+ value=config)
- def _build_one(self, request, project, prefix, scm):
+ def _build_one(self, request, project, prefix, config, scm, show_save_button=False):
if type(scm) == MapSchema:
- return self._build_map_scm(request, project, prefix, scm)
+ return self._build_map_scm(request, project, prefix, config, scm)
elif type(scm) == StringSchema:
- return self._build_string_scm(request, project, prefix, scm)
+ return self._build_string_scm(request, project, prefix, config, scm)
elif type(scm) == IntegerSchema:
- return self._build_integer_scm(request, project, prefix, scm)
+ return self._build_integer_scm(request, project, prefix, config, scm)
elif type(scm) == BooleanSchema:
- return self._build_boolean_scm(request, project, prefix, scm)
+ return self._build_boolean_scm(request, project, prefix, config, scm)
elif type(scm) == EnumSchema:
- return self._build_enum_scm(request, project, prefix, scm)
+ return self._build_enum_scm(request, project, prefix, config, scm)
elif type(scm) == ArraySchema:
- return self._build_array_scm(request, project, prefix, scm)
+ return self._build_array_scm(request, project, prefix, config, scm, show_save_button)
assert False
def build_config_html(self, request, project):
- assert not isinstance(self.project_config_schema, StringSchema)
- assert not isinstance(self.project_config_schema, IntegerSchema)
+ assert isinstance(self.project_config_schema, ArraySchema)
scm = self.project_config_schema
- tmpl = self._build_one(request, project, scm.name + ".", scm)
+ config = self.get_project_config(project)
+ tmpl = self._build_one(request, project, scm.name + ".", config, scm, True)
tmpl += self._render_template(request, project, TMPL_END)
return tmpl
- def _get_map_scm(self, project, prop_name, scm):
- prefix = prop_name + "."
- result = {}
- for p in project.get_properties().keys():
- if not p.startswith(prefix):
- continue
- name = p[len(prefix):]
- name = name[:name.rfind(".")]
- if name in result:
- continue
- assert scm.item.name == '{name}'
- value = self._get_one(project, prefix + name, scm.item)
- result[name] = value
- return result
-
- def _get_array_scm(self, project, prop_name, scm):
- prefix = prop_name + "."
- result = {}
- for i in scm.members:
- assert i.name != '{name}'
- result[i.name] = self._get_one(project, prefix + i.name, i)
- return result
-
- def _get_one(self, project, prop_name, scm):
- if type(scm) == MapSchema:
- return self._get_map_scm(project, prop_name, scm)
- elif type(scm) == ArraySchema:
- return self._get_array_scm(project, prop_name, scm)
- else:
- return project.get_property(prop_name)
-
def get_project_config(self, project):
- scm = self.project_config_schema
- return self._get_one(project, scm.name, scm)
+ return project.config.get(self.project_config_schema.name, {})
_loaded_modules = {}
@@ -423,9 +363,9 @@ function properties_save(btn) {
$(btn).addClass("disabled");
$(btn).text("Saving...");
$(btn).parent().find(".save-message").remove();
- patchew_api_do("set-project-properties",
+ patchew_api_do("set-project-config",
{ project: "{{ project.name }}",
- properties: props })
+ config: props })
.done(function (data) {
save_done(btn, true);
})
@@ -466,23 +406,8 @@ function map_delete_item(btn) {
if (!window.confirm("Really delete '" + name +"'?")) {
return;
}
- $(btn).addClass("disabled");
- $(btn).text("Deleting...");
- $(btn).parent().find(".delete-message").remove();
- patchew_api_do("delete-project-properties-by-prefix",
- { project: "{{ project.name }}",
- prefix: prefix })
- .done(function (data) {
- container = $(btn).parent().parent().parent();
- container.remove();
- })
- .fail(function (data, text, error) {
- $(btn).removeClass("disabled");
- $(btn).text("Delete");
- info = $("<div class=\\"alert alert-danger delete-message\\"></div>");
- info.html("Error: " + error);
- info.insertBefore($(btn));
- });
+ container = $(btn).parent().parent().parent();
+ container.remove();
}
function enum_change(which) {
val = $(which).val();
diff --git a/mods/git.py b/mods/git.py
index d459a0b..8e868c2 100644
--- a/mods/git.py
+++ b/mods/git.py
@@ -246,19 +246,20 @@ class ApplierGetView(APILoginRequiredView):
def handle(self, request, target_repo=None):
q = Message.objects.filter(results__name="git", results__status="pending")
if target_repo is not None and target_repo != '':
- props = ProjectProperty.objects.filter(name='git.push_to')
- # unfortunately startswith does not work with JSONFields, so we have to hack
- # around it and look at the raw database representation. This would fail
- # if we used PostgreSQL json fields!
- target_repo_escaped = json.dumps(target_repo)
- if target_repo[-1] != '/':
- props = props.extra(where=["value = %s or value like %s"],
- params=[target_repo_escaped,
- target_repo_escaped[0:-1] + '/%'])
- else:
- props = props.extra(where=["value like %s"],
- params=target_repo_escaped[0:-1] + '%')
- q = q.filter(project__in=[prop.project for prop in props.all()])
+ # Postgres could use JSON fields instead. Fortunately projects are
+ # few so this is cheap
+ def match_target_repo(config, target_repo):
+ push_to = config.get('git', {}).get('push_to')
+ if push_to is None:
+ return False
+ if target_repo[-1] != '/':
+ return push_to == target_repo or push_to.startswith(target_repo + '/')
+ else:
+ return push_to.startswith(target_repo)
+
+ projects = Project.objects.values_list('id', 'config').all()
+ projects = [pid for pid, config in projects if match_target_repo(config, target_repo)]
+ q = q.filter(project__pk__in=projects)
m = q.first()
if not m:
return None
diff --git a/mods/testing.py b/mods/testing.py
index 9e41307..969ce84 100644
--- a/mods/testing.py
+++ b/mods/testing.py
@@ -117,15 +117,16 @@ class TestingModule(PatchewModule):
html_log_url="URL to test log (HTML)",
is_timeout="whether the test has timeout")
register_handler("SetProperty", self.on_set_property)
+ register_handler("SetProjectConfig", self.on_set_config)
register_handler("ResultUpdate", self.on_result_update)
def on_set_property(self, evt, obj, name, value, old_value):
if isinstance(obj, Project) and name == "git.head" \
and old_value != value:
self.clear_and_start_testing(obj)
- elif isinstance(obj, Project) and name.startswith("testing.tests.") \
- and old_value != value:
- self.project_recalc_pending_tests(obj)
+
+ def on_set_config(self, evt, obj):
+ self.project_recalc_pending_tests(obj)
def get_msg_base_tags(self, msg):
return [t for t in msg.tags if t.lower().startswith("based-on:")]
diff --git a/tests/patchewtest.py b/tests/patchewtest.py
index ade039d..e58e5b9 100644
--- a/tests/patchewtest.py
+++ b/tests/patchewtest.py
@@ -127,11 +127,15 @@ class PatchewTestCase(dj_test.LiveServerTestCase):
def add_project(self, name, mailing_list="", git_repo=""):
p = Project(name=name, mailing_list=mailing_list, git=git_repo or self.create_git_repo(name))
- p.save()
push_repo = self.create_git_repo(name + "_push")
- p.set_property("git.push_to", push_repo)
- p.set_property("git.public_repo", push_repo)
- p.set_property("git.url_template", push_repo)
+ p.config = {
+ "git": {
+ "push_to": push_repo,
+ "public_repo": push_repo,
+ "url_template": push_repo
+ }
+ }
+ p.save()
return p
def api_login(self):
diff --git a/tests/test_git.py b/tests/test_git.py
index 09545df..0b9fa50 100755
--- a/tests/test_git.py
+++ b/tests/test_git.py
@@ -22,9 +22,14 @@ class GitTest(PatchewTestCase):
self.cli_login()
self.repo = self.create_git_repo()
self.p = self.add_project("QEMU", "qemu-devel at nongnu.org", git_repo=self.repo)
- self.p.set_property("git.push_to", self.repo)
- self.p.set_property("git.public_repo", self.repo)
- self.p.set_property("git.url_template", self.repo + " %t")
+ self.p.config = {
+ "git": {
+ "push_to": self.repo,
+ "public_repo": self.repo,
+ "url_template": self.repo + " %t"
+ }
+ }
+ self.p.save()
self.PROJECT_BASE = '%sprojects/%d/' % (self.REST_BASE, self.p.id)
def cleanUp(self):
diff --git a/tests/test_testing.py b/tests/test_testing.py
index 630e7f9..2d21654 100755
--- a/tests/test_testing.py
+++ b/tests/test_testing.py
@@ -13,16 +13,31 @@ import subprocess
from api.models import Message, Result
from api.search import FLAG_TESTED
+from event import emit_event
from .patchewtest import PatchewTestCase, main
def create_test(project, name, requirements="", script="#!/bin/bash\ntrue"):
- prefix = "testing.tests." + name + "."
- project.set_property(prefix + "timeout", 3600)
- project.set_property(prefix + "enabled", True)
- project.set_property(prefix + "script", script)
- project.set_property(prefix + "requirements", requirements)
+ testing = project.config.setdefault('testing', {})
+ tests = testing.setdefault('tests', {})
+ tests[name] = {
+ 'timeout': 3600,
+ 'enabled': True,
+ 'script': script,
+ 'requirements': requirements
+ }
+ project.save()
+ emit_event("SetProjectConfig", obj=project)
+
+
+def create_requirement(project, name, script="#!/bin/bash\ntrue"):
+ testing = project.config.setdefault('testing', {})
+ requirements = testing.setdefault('requirements', {})
+ requirements[name] = {
+ 'script': script,
+ }
+ project.save()
class TestingTestCase(PatchewTestCase, metaclass=abc.ABCMeta):
@@ -220,13 +235,11 @@ class TesterTest(PatchewTestCase):
create_test(self.p2, "b")
self.p3 = self.add_project("ALLOW", "qemu-devel at nongnu.org")
- self.p3.set_property("testing.requirements.allow.script",
- "#!/bin/sh\ntrue")
+ create_requirement(self.p3, 'allow', "#!/bin/sh\ntrue")
create_test(self.p3, "c", "allow")
self.p4 = self.add_project("DENY", "qemu-devel at nongnu.org")
- self.p4.set_property("testing.requirements.deny.script",
- "#!/bin/sh\nfalse")
+ create_requirement(self.p4, 'deny', "#!/bin/sh\nfalse")
create_test(self.p4, "d", "deny")
self.cli_login()
@@ -389,7 +402,9 @@ class TestingDisableTest(PatchewTestCase):
self.cli_login()
self.cli_import('0013-foo-patch.mbox.gz')
self.do_apply()
- self.p1.set_property("testing.tests.a.enabled", False)
+ self.p1.config['testing']['tests']['a']['enabled'] = False
+ self.p1.save()
+ emit_event("SetProjectConfig", obj=self.p1)
out, err = self.check_cli(["tester", "-p", "QEMU", "--no-wait"])
self.assertNotIn("Project: QEMU\n", out)
self.cli_logout()
--
2.21.0
More information about the Patchew-devel
mailing list