Skip to content

Commit 0d16102

Browse files
ferishiliferishili
authored
Fix playlist sync db issue (#1366)
* Enhance playlist migration by enforcing foreign key constraints and ensuring non-nullable columns * make sure the playlist table has no null config_id * Draft: check for invalid records! * add more error handling * try different fkey name * try separate db query for the same fkey
1 parent a48afda commit 0d16102

File tree

1 file changed

+117
-65
lines changed

1 file changed

+117
-65
lines changed

‎lib/Helpers/PlaylistMigration.php‎

Lines changed: 117 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -19,79 +19,131 @@ public static function convert()
1919
{
2020
$db = DBManager::get();
2121

22-
// Migrate existing playlists to Opencast
23-
$playlists = Playlists::findBySQL('service_playlist_id IS NULL');
24-
25-
// Try three times
26-
$tries = 0;
27-
while (!empty($playlists) && $tries < 3) {
28-
foreach ($playlists as $playlist) {
29-
$config_id = $playlist->config_id;
30-
if (empty($config_id)) {
31-
// Default opencast server if playlist belongs to no opencast server
32-
$config_id = \Config::get()->OPENCAST_DEFAULT_SERVER;
33-
}
22+
// Default opencast server if playlist belongs to no opencast server
23+
$default_config_id = \Config::get()->OPENCAST_DEFAULT_SERVER;
24+
25+
try {
26+
// Migrate existing playlists to Opencast
27+
$playlists = Playlists::findBySQL('service_playlist_id IS NULL');
3428

35-
// Get playlist videos
36-
$playlist_videos = self::getPlaylistVideos($playlist);
37-
38-
$api_playlists_client = ApiPlaylistsClient::getInstance($config_id);
39-
$oc_playlist = $api_playlists_client->createPlaylist(
40-
self::getOcPlaylistData($playlist, $playlist_videos)
41-
);
42-
43-
if ($oc_playlist) {
44-
// Store oc playlist reference in Stud.IP if successfully created
45-
$playlist->config_id = $config_id;
46-
$playlist->service_playlist_id = $oc_playlist->id;
47-
$playlist->store();
48-
49-
Playlists::checkPlaylistACL($oc_playlist, $playlist);
50-
51-
// Store entry ids
52-
for ($i = 0; $i < count($playlist_videos); $i++) {
53-
$stmt = $db->prepare("UPDATE oc_playlist_video
54-
SET `service_entry_id` = :service_entry_id
55-
WHERE playlist_id = :playlist_id AND video_id = :video_id
56-
");
57-
$stmt->execute($data = [
58-
'service_entry_id' => $oc_playlist->entries[$i]->id,
59-
'playlist_id' => $playlist->id,
60-
'video_id' => $playlist_videos[$i]['id']
61-
]);
29+
// Try three times
30+
$tries = 0;
31+
while (!empty($playlists) && $tries < 10) {
32+
foreach ($playlists as $playlist) {
33+
$config_id = $playlist->config_id ?? null;
34+
if (empty($config_id)) {
35+
// Default opencast server if playlist belongs to no opencast server
36+
$config_id = $default_config_id;
6237
}
63-
}
64-
}
6538

66-
$playlists = Playlists::findBySQL('service_playlist_id IS NULL');
67-
$tries++;
68-
}
39+
// Get playlist videos
40+
$playlist_videos = self::getPlaylistVideos($playlist);
41+
42+
$api_playlists_client = ApiPlaylistsClient::getInstance($config_id);
43+
$oc_playlist = $api_playlists_client->createPlaylist(
44+
self::getOcPlaylistData($playlist, $playlist_videos)
45+
);
46+
47+
if ($oc_playlist) {
48+
// Store oc playlist reference in Stud.IP if successfully created
49+
$playlist->config_id = $config_id;
50+
$playlist->service_playlist_id = $oc_playlist->id;
51+
$playlist->store();
52+
53+
Playlists::checkPlaylistACL($oc_playlist, $playlist);
54+
55+
// Store entry ids
56+
for ($i = 0; $i < count($playlist_videos); $i++) {
57+
$stmt = $db->prepare("UPDATE oc_playlist_video
58+
SET `service_entry_id` = :service_entry_id
59+
WHERE playlist_id = :playlist_id AND video_id = :video_id
60+
");
61+
$stmt->execute($data = [
62+
'service_entry_id' => $oc_playlist->entries[$i]->id,
63+
'playlist_id' => $playlist->id,
64+
'video_id' => $playlist_videos[$i]['id']
65+
]);
66+
}
67+
}
68+
}
6969

70-
// Forbid playlist without related oc playlist
71-
$db->exec('ALTER TABLE `oc_playlist`
72-
CHANGE COLUMN `config_id` `config_id` int NOT NULL,
73-
CHANGE COLUMN `service_playlist_id` `service_playlist_id` varchar(64) UNIQUE NOT NULL'
74-
);
70+
$playlists = Playlists::findBySQL('service_playlist_id IS NULL');
71+
$tries++;
72+
}
7573

76-
// Forbid playlist video without related oc playlist entry
77-
$db->exec('ALTER TABLE `oc_playlist_video`
78-
CHANGE COLUMN `service_entry_id` `service_entry_id` int NOT NULL'
79-
);
74+
// What is the point of letting the process continue if there are still playlists with null service_playlist_id of duplicated service_playlist_ids?
75+
$duplicate_service_playlist_ids = $db->query(
76+
"SELECT service_playlist_id, COUNT(*) as count
77+
FROM oc_playlist
78+
WHERE service_playlist_id IS NOT NULL
79+
GROUP BY service_playlist_id
80+
HAVING count > 1"
81+
)->fetchAll(\PDO::FETCH_ASSOC);
82+
83+
if (!empty($playlists) || !empty($duplicate_service_playlist_ids)) {
84+
$message = "Migration failed due to invalid data records:\n";
85+
if (!empty($playlists)) {
86+
$message .= "Playlists with null service_playlist_id:\n";
87+
foreach ($playlists as $playlist) {
88+
$message .= "Playlist ID: {$playlist->id}\n";
89+
}
90+
}
91+
if (!empty($duplicate_service_playlist_ids)) {
92+
$message .= "Duplicate service_playlist_ids:\n";
93+
foreach ($duplicate_service_playlist_ids as $record) {
94+
$message .= "Service Playlist ID: {$record['service_playlist_id']}, Count: {$record['count']}\n";
95+
}
96+
}
97+
throw new \Exception($message);
98+
}
8099

81-
\SimpleOrMap::expireTableScheme();
100+
// We need another step to make sure config id is set and it is not null before altering the table with not-null config_id.
101+
$null_config_playlists = Playlists::findBySQL('config_id IS NULL');
82102

83-
// Add playlists sync cronjob
84-
$scheduler = \CronjobScheduler::getInstance();
103+
while (!empty($null_config_playlists)) {
104+
foreach ($null_config_playlists as $null_config_playlist) {
105+
// Store config id with default config id.
106+
$null_config_playlist->config_id = $default_config_id;
107+
$null_config_playlist->store();
108+
}
109+
$null_config_playlists = Playlists::findBySQL('config_id IS NULL');
110+
}
85111

86-
if (!$task_id = \CronjobTask::findByFilename(self::CRONJOBS_DIR . 'opencast_sync_playlists.php')[0]->task_id) {
87-
$task_id = $scheduler->registerTask(self::CRONJOBS_DIR . 'opencast_sync_playlists.php', true);
88-
}
112+
// Forbid playlist without related oc playlist
113+
// First drop foreign key constraint
114+
$db->exec('ALTER TABLE `oc_playlist` DROP FOREIGN KEY `oc_playlist_ibfk_1`');
115+
// Then change column to not null
116+
$db->exec('ALTER TABLE `oc_playlist`
117+
CHANGE COLUMN `config_id` `config_id` int NOT NULL,
118+
CHANGE COLUMN `service_playlist_id` `service_playlist_id` varchar(64) UNIQUE NOT NULL'
119+
);
120+
// Then add foreign key constraint again
121+
$db->exec('ALTER TABLE `oc_playlist`
122+
ADD FOREIGN KEY `oc_playlist_ibfk_1` (`config_id`) REFERENCES `oc_config` (`id`)
123+
ON DELETE RESTRICT ON UPDATE RESTRICT'
124+
);
125+
// Forbid playlist video without related oc playlist entry
126+
$db->exec('ALTER TABLE `oc_playlist_video`
127+
CHANGE COLUMN `service_entry_id` `service_entry_id` int NOT NULL'
128+
);
129+
130+
\SimpleOrMap::expireTableScheme();
131+
132+
// Add playlists sync cronjob
133+
$scheduler = \CronjobScheduler::getInstance();
134+
135+
if (!$task_id = \CronjobTask::findByFilename(self::CRONJOBS_DIR . 'opencast_sync_playlists.php')[0]->task_id) {
136+
$task_id = $scheduler->registerTask(self::CRONJOBS_DIR . 'opencast_sync_playlists.php', true);
137+
}
89138

90-
// add the new cronjobs
91-
if ($task_id) {
92-
$scheduler->cancelByTask($task_id);
93-
$scheduler->schedulePeriodic($task_id, -10); // negative value means "every x minutes"
94-
\CronjobSchedule::findByTask_id($task_id)[0]->activate();
139+
// add the new cronjobs
140+
if ($task_id) {
141+
$scheduler->cancelByTask($task_id);
142+
$scheduler->schedulePeriodic($task_id, -10); // negative value means "every x minutes"
143+
\CronjobSchedule::findByTask_id($task_id)[0]->activate();
144+
}
145+
} catch (\Throwable $th) {
146+
throw new \Exception('Migration fehlgeschlagen: ' . $th->getMessage());
95147
}
96148
}
97149

@@ -179,4 +231,4 @@ public static function isConverted()
179231

180232
return $result['Null'] != 'YES';
181233
}
182-
}
234+
}

0 commit comments

Comments
 (0)