[Patchew-devel] [PATCH 12/17] models: store plugin configuration in a single, separate JSONField
Paolo Bonzini
pbonzini at redhat.com
Thu May 2 11:17:59 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.
Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
---
api/migrations/0046_project_config.py | 22 ++++++++
.../0047_populate_project_config.py | 56 +++++++++++++++++++
api/models.py | 11 ++++
api/views.py | 29 ++++------
mod.py | 34 +----------
mods/git.py | 27 ++++-----
mods/testing.py | 7 ++-
static/js/config-editor.js | 23 ++------
tests/patchewtest.py | 12 ++--
tests/test_git.py | 11 +++-
tests/test_testing.py | 32 +++++++----
11 files changed, 162 insertions(+), 102 deletions(-)
create mode 100644 api/migrations/0046_project_config.py
create mode 100644 api/migrations/0047_populate_project_config.py
diff --git a/api/migrations/0046_project_config.py b/api/migrations/0046_project_config.py
new file mode 100644
index 0000000..f61c4c8
--- /dev/null
+++ b/api/migrations/0046_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', '0045_message_maintainers'),
+ ]
+
+ 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/0047_populate_project_config.py b/api/migrations/0047_populate_project_config.py
new file mode 100644
index 0000000..2a86e1f
--- /dev/null
+++ b/api/migrations/0047_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', '0046_project_config'),
+ ]
+
+ operations = [
+ migrations.RunPython(properties_to_config,
+ reverse_code=config_to_properties),
+ ]
diff --git a/api/models.py b/api/models.py
index d2dafde..68e90e1 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
@@ -179,6 +180,13 @@ class Project(models.Model):
def has_project(self, project):
return self.objects.filter(name=project).exists()
+ def save(self, *args, **kwargs):
+ old_project = Project.objects.filter(pk=self.pk).first()
+ old_config = old_project.config if old_project else None
+ super(Project, self).save(*args, **kwargs)
+ if old_config != self.config:
+ emit_event("SetProjectConfig", obj=self)
+
def get_property(self, prop, default=None):
a = ProjectProperty.objects.filter(project=self, name=prop).first()
if a:
@@ -311,6 +319,9 @@ declare_event("SeriesMerged", project="project object",
declare_event("MessageAdded", message="message object that is added")
+declare_event("SetProjectConfig", obj="project whose configuration was updated")
+
+
declare_event("SetProperty", obj="object to set the property",
name="name of the property",
value="value of the property",
diff --git a/api/views.py b/api/views.py
index 887f9ed..672cdc8 100644
--- a/api/views.py
+++ b/api/views.py
@@ -129,28 +129,23 @@ class UpdateProjectHeadView(APILoginRequiredView):
return ret
-class SetProjectPropertiesView(APILoginRequiredView):
- name = "set-project-properties"
+class SetProjectConfigView(APILoginRequiredView):
+ name = "set-project-config"
allowed_groups = ["maintainers"]
- def handle(self, request, project, properties):
+ 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, v in properties.items():
- po.set_property(k, v)
-
-
-class DeleteProjectPropertiesByPrefixView(APILoginRequiredView):
- name = "delete-project-properties-by-prefix"
- allowed_groups = ["maintainers"]
-
- def handle(self, request, project, prefix):
- 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 config.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()
def prepare_patch(p):
diff --git a/mod.py b/mod.py
index 1b7910c..f6db0ce 100644
--- a/mod.py
+++ b/mod.py
@@ -121,40 +121,8 @@ class PatchewModule(object):
config = self.get_project_config(project)
return self._build_one(request, project, scm.name, config, scm)
- 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 = {}
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 0125288..2c45001 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/static/js/config-editor.js b/static/js/config-editor.js
index 562077d..aae5302 100644
--- a/static/js/config-editor.js
+++ b/static/js/config-editor.js
@@ -70,9 +70,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: current_project(),
- properties: props })
+ config: props })
.done(function (data) {
save_done(btn, true);
})
@@ -118,26 +118,11 @@ function map_add_item(btn) {
function map_delete_item(btn) {
item = $(btn).parent().parent().parent();
name = item.find(".item-name").text();
- prefix = item.data('property-prefix') + ".";
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: current_project(),
- prefix: prefix })
- .done(function (data) {
- item.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));
- });
+ item.remove();
+ confirm_leaving_page(true);
}
function enum_change(which) {
val = $(which).val();
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 a5c22e1..2a01b7a 100755
--- a/tests/test_testing.py
+++ b/tests/test_testing.py
@@ -17,11 +17,24 @@ 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()
+
+
+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):
@@ -219,13 +232,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()
@@ -388,7 +399,8 @@ 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()
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