Django migration: #47 (closed)
tldr; MR to add object validation before saving to the DB.
I attempted to update https://debexpo.d.n to the latest version of the django branch, integrating the plugin MR. While importing a new package to test the distribution plugin, I ran into a database integrity error:
2020-03-25 17:45:58 UTC [19221-1] lyknode@lyknode-testing ERROR: value too long for type character varying(32)
2020-03-25 17:45:58 UTC [19221-2] lyknode@lyknode-testing STATEMENT: INSERT INTO "plugins_pluginresults" ("upload_id", "plugin", "test", "outcome", "json", "severity") VALUES (692, 'distribution', 'same-distribution-changes-and-changelog', 'changes and changelog distribution differs', '{"changes": "stable", "changelog": "unstable"}', 3) RETURNING "plugins_pluginresults"."id"
Imaging my surprise! I had the plugin test covered for this case. No way I could not have run into that particular issue while manually testing and while writing the tests. But here we are, the blunt truth spoken (written?) by that log.
Let's backtrack a bit and try to understand what happened. First the DBMS. I use a SQlite backend locally to avoid running a full fledged service. Let's have a look at the tables:
The definition:
CREATE TABLE IF NOT EXISTS "plugins_pluginresults" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "plugin" text NOT NULL, "test" varchar(32) NOT NULL, "outcome" text NOT NULL, "json" text NULL, "severity" smallint unsigned NOT NULL CHECK ("severity" >= 0), "upload_id" integer NOT NULL REFERENCES "packages_packageupload" ("id") DEFERRABLE INITIALLY DEFERRED);
CREATE INDEX "plugins_pluginresults_upload_id_256cc125" ON "plugins_pluginresults" ("upload_id");
Test is indeed a VARCHAR(32). Mmm. The data then?
> SELECT test FROM plugins_pluginresults LIMIT 1;
same-distribution-changes-and-changelog
> SELECT LENGTH(test) FROM plugins_pluginresults LIMIT 1;
39
Right.
Let's wrap around that. The other issue I take with all of that is: What the hell am I writing models for? Isn't django supposed to warn me about such integrity errors before actually trying to push to the DB?
Well, I was under the wrong assumption that Object.save() would validate the model. I had seen that working fine with all Forms from the UI, being validated and then save to the DB. However I had been tricked.
This is what we have for a Form:
if form.is_valid():
form.save()
Guess what? is_valid() does do exactly what is says... It does validate against the model. But, that being only applicable to the Forms, other objects created in the backend are supposed to be checked with Object.full_clean(), as clearly stated in the documentation.
All of that text to present this MR, that add the full_clean() when needed, and fixes the initial data and models.
That was fun, sorry for the long description