From 14409a32e74e2623706be11d44ef36553f798d97 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 15 Apr 2020 07:59:44 -0700 Subject: [PATCH] (stable) Update a very old Phriction migration which incorrectly uses "save()" Summary: See . It is broadly unsafe for migrations to use "save()". If the object gains new fields later, the query will include "SET newField = X", which will fail against the old schema which is in the process of being upgraded. Instead, migrations must issue raw SQL against the schema as it is expected to exist at the time the migration executes. Migrations have followed this rule for a long time, but this ~6 year old migration was overlooked. Update it to issue a raw query to perform the policy update. Test Plan: This is somewhat flimsy since rebuilding a genuine reproduction case is messy, but used "bin/storage --apply ..." to at least get the new query to execute against modern Phabricator without issues. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Differential Revision: https://secure.phabricator.com/D21124 --- .../20141107.phriction.policy.2.php | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/resources/sql/autopatches/20141107.phriction.policy.2.php b/resources/sql/autopatches/20141107.phriction.policy.2.php index 5e00bd7a40..821e9275f5 100644 --- a/resources/sql/autopatches/20141107.phriction.policy.2.php +++ b/resources/sql/autopatches/20141107.phriction.policy.2.php @@ -16,6 +16,9 @@ foreach (new LiskMigrationIterator($table) as $doc) { continue; } + $new_view_policy = $default_view_policy; + $new_edit_policy = $default_edit_policy; + // If this was previously a magical project wiki page (under projects/, but // not projects/ itself) we need to apply the project policies. Otherwise, // apply the default policies. @@ -35,26 +38,24 @@ foreach (new LiskMigrationIterator($table) as $doc) { ->executeOne(); if ($project) { - $view_policy = nonempty($project->getViewPolicy(), $default_view_policy); $edit_policy = nonempty($project->getEditPolicy(), $default_edit_policy); - $project_name = $project->getName(); - echo pht( - "Migrating document %d to project policy %s...\n", - $id, - $project_name); - $doc->setViewPolicy($view_policy); - $doc->setEditPolicy($edit_policy); - $doc->save(); - continue; + $new_view_policy = $view_policy; + $new_edit_policy = $edit_policy; } } - echo pht('Migrating document %d to default install policy...', $id)."\n"; - $doc->setViewPolicy($default_view_policy); - $doc->setEditPolicy($default_edit_policy); - $doc->save(); + echo pht('Migrating document %d to new policy...', $id)."\n"; + + queryfx( + $conn_w, + 'UPDATE %R SET viewPolicy = %s, editPolicy = %s + WHERE id = %d', + $table, + $new_view_policy, + $new_edit_policy, + $id); } echo pht('Done.')."\n";