feat: key Frames by (EventId, FrameId) and drop the surrogate Id refs #5164

Every query on Frames filters on EventId and orders or ranges by FrameId,
and nothing needs a frame by Id. The Id primary key was a clustered index
nothing read, and EventId_FrameId_idx was a secondary index every query
used. On a copy of a local table that secondary index was half the
table's size. With (EventId, FrameId) as the primary key the index is
gone, and an event's rows are stored together, so deleting an event is a
range delete.

zm_update-1.39.36.sql:
- converts AI_Detections.FrameId from Frames.Id to the per-event frame
  number, drops its foreign key to Frames and indexes (EventId, FrameId).
  A composite foreign key cannot replace it: ON DELETE SET NULL would
  have to null the NOT NULL EventId, and Frames rows are written in
  batches, so a detection can be recorded before its frame row.
- removes duplicate (EventId, FrameId) rows, keeping the earliest.
- rebuilds Frames with the new primary key.
- removes ON UPDATE CURRENT_TIMESTAMP from Frames.TimeStamp. Any UPDATE
  of a frame row was overwriting its capture time.
Each step checks the current schema first, so the migration can be
re-run.

REST API: view, edit and delete take /frames/<action>/<EventId>/<FrameId>.json.
The old single-Id URLs return 404. CakePHP 2 has no composite keys, so
the model's primaryKey is EventId. That keeps Event's dependent cascade
delete limited to the event's own frames. The controller writes with
explicit (EventId, FrameId) conditions instead of save(), which would
match rows on EventId alone. Edit no longer changes EventId or FrameId.

view=image with fid but no eid used to look up Frames.Id. It now returns
404.

The Perl Frame class is identified by (EventId, FrameId).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Isaac ConnorandClaude Opus 5.5 committed 2026-10-01 11:48:26 -04:00
1 parent b79239940f
commit c4ef3fdd62
11 files changed
+198 -89

No files matched your search

+4 -4
View File
@@ -62,7 +62,9 @@ CREATE TABLE IF NOT EXISTS `AI_Detection_Settings` (
CREATE TABLE IF NOT EXISTS `AI_Detections` (
`Id` BIGINT unsigned NOT NULL auto_increment,
`EventId` BIGINT unsigned NOT NULL,
`FrameId` BIGINT unsigned,
/* Frames.FrameId within EventId. Not a foreign key: Frames rows are
written in batches, so a detection can be recorded before its frame. */
`FrameId` int(10) unsigned,
`ObjectClassId` int(10) unsigned NOT NULL,
`Confidence` decimal(5,4) NOT NULL,
`BoundingBoxX` int(10) unsigned,
@@ -71,10 +73,8 @@ CREATE TABLE IF NOT EXISTS `AI_Detections` (
`BoundingBoxHeight` int(10) unsigned,
`DetectedAt` TIMESTAMP(3) DEFAULT CURRENT_TIMESTAMP(3),
PRIMARY KEY (`Id`),
KEY `AI_Detections_EventId_idx` (`EventId`),
KEY `AI_Detections_FrameId_idx` (`FrameId`),
KEY `AI_Detections_EventId_FrameId_idx` (`EventId`,`FrameId`),
KEY `AI_Detections_ObjectClassId_idx` (`ObjectClassId`),
FOREIGN KEY (`EventId`) REFERENCES `Events` (`Id`) ON DELETE CASCADE,
FOREIGN KEY (`FrameId`) REFERENCES `Frames` (`Id`) ON DELETE SET NULL,
FOREIGN KEY (`ObjectClassId`) REFERENCES `AI_Object_Classes` (`Id`) ON DELETE CASCADE
) ENGINE=InnoDB;
+5 -6
View File
@@ -443,12 +443,11 @@ CREATE TABLE `Filters` (
DROP TABLE IF EXISTS `Frames`;
CREATE TABLE `Frames` (
`Id` BIGINT UNSIGNED NOT NULL AUTO_INCREMENT,
`EventId` BIGINT UNSIGNED NOT NULL default '0',
/* FOREIGN KEY (`EventId`) REFERENCES `Events` (`Id`) ON DELETE CASCADE,*/
`FrameId` int(10) unsigned NOT NULL default '0',
`Type` enum('Normal','Bulk','Alarm') NOT NULL default 'Normal',
`TimeStamp` timestamp NOT NULL default CURRENT_TIMESTAMP on update CURRENT_TIMESTAMP,
`TimeStamp` timestamp NOT NULL default CURRENT_TIMESTAMP,
`Delta` decimal(8,2) NOT NULL default '0.00',
`Score` smallint(5) unsigned NOT NULL default '0',
/* Peak audio level since the previous row, 0-100 on the same dBFS-derived
@@ -457,11 +456,11 @@ CREATE TABLE `Frames` (
chosen by looking at the graph. 0 means silence or no audio, which is how
the event view knows not to draw an audio line. */
`AudioLevel` tinyint(3) unsigned NOT NULL default '0',
PRIMARY KEY (`Id`),
/* Every query on Frames is anchored on EventId and orders or ranges by
FrameId within the event, so one composite index serves them all. Nothing
reads Frames by Type or TimeStamp without EventId. See zm_update-1.39.27. */
INDEX `EventId_FrameId_idx` (`EventId`,`FrameId`)
FrameId within the event, so the clustered key serves them all and keeps
an event's rows together. Nothing reads Frames by Type or TimeStamp
without EventId. See zm_update-1.39.27 and zm_update-1.39.36. */
PRIMARY KEY (`EventId`,`FrameId`)
) ENGINE=@ZM_MYSQL_ENGINE@;
--
+73
View File
@@ -0,0 +1,73 @@
--
-- This updates a 1.39.35 database to 1.39.36
--
-- Make (EventId, FrameId) the primary key of Frames and drop the surrogate Id.
--
-- Frames is the largest table in the schema. Every query against it is
-- anchored on EventId and orders or ranges by FrameId (see zm_update-1.39.27),
-- and nothing looks a frame up by Id. Keeping Id meant a clustered index that
-- nothing reads plus a secondary EventId_FrameId_idx that every read uses,
-- each secondary entry also carrying a copy of Id. On a copy of a real table
-- the secondary index was half the table's size. With (EventId, FrameId) as
-- the clustered key that index goes away, and an event's rows sit together on
-- disk, so deleting an event is a range delete rather than scattered writes.
--
-- FrameId is the per-event frame counter, so (EventId, FrameId) is unique for
-- anything zmc writes. Duplicates are removed first anyway, keeping the
-- earliest row, because a single duplicate would make ADD PRIMARY KEY fail.
--
-- TimeStamp also loses ON UPDATE CURRENT_TIMESTAMP. It is the time the frame
-- was captured, and any UPDATE of a frame row was silently replacing it with
-- the time of the update.
--
-- AI_Detections.FrameId was a foreign key to Frames.Id. It becomes the
-- per-event frame number, the same thing Frames.FrameId, Stats.FrameId and
-- Event_Data.FrameId hold, converted from the old Id before Id is dropped.
-- It cannot be a composite foreign key to Frames: ON DELETE SET NULL would
-- have to null the NOT NULL EventId, and Frames rows are written in batches,
-- so a detection can be recorded before its frame row exists.
--
-- Note for large installs: this rebuilds the table and will take a while on a
-- Frames table with hundreds of millions of rows.
--
set @exist := (select count(*) from information_schema.columns where table_schema = database() and table_name = 'Frames' and column_name = 'Id');
set @fk := (select constraint_name from information_schema.referential_constraints where constraint_schema = database() and table_name = 'AI_Detections' and referenced_table_name = 'Frames' limit 1);
set @sqlstmt := if( @fk is not null, concat('ALTER TABLE `AI_Detections` DROP FOREIGN KEY `', @fk, '`'), "SELECT 'AI_Detections has no foreign key to Frames.'");
PREPARE stmt FROM @sqlstmt;
EXECUTE stmt;
set @sqlstmt := if( @exist > 0, 'UPDATE `AI_Detections` D JOIN `Frames` F ON F.`Id` = D.`FrameId` SET D.`FrameId` = F.`FrameId`', "SELECT 'AI_Detections.FrameId already converted.'");
PREPARE stmt FROM @sqlstmt;
EXECUTE stmt;
set @idx := (select count(*) from information_schema.statistics where table_schema = database() and table_name = 'AI_Detections' and index_name = 'AI_Detections_EventId_FrameId_idx');
set @sqlstmt := if( @idx = 0, 'ALTER TABLE `AI_Detections` MODIFY `FrameId` int(10) unsigned, ADD INDEX `AI_Detections_EventId_FrameId_idx` (`EventId`,`FrameId`)', "SELECT 'AI_Detections_EventId_FrameId_idx already exists.'");
PREPARE stmt FROM @sqlstmt;
EXECUTE stmt;
set @idx := (select count(*) from information_schema.statistics where table_schema = database() and table_name = 'AI_Detections' and index_name = 'AI_Detections_FrameId_idx');
set @sqlstmt := if( @idx > 0, 'DROP INDEX `AI_Detections_FrameId_idx` ON `AI_Detections`', "SELECT 'AI_Detections_FrameId_idx already removed.'");
PREPARE stmt FROM @sqlstmt;
EXECUTE stmt;
set @idx := (select count(*) from information_schema.statistics where table_schema = database() and table_name = 'AI_Detections' and index_name = 'AI_Detections_EventId_idx');
set @sqlstmt := if( @idx > 0, 'DROP INDEX `AI_Detections_EventId_idx` ON `AI_Detections`', "SELECT 'AI_Detections_EventId_idx already removed.'");
PREPARE stmt FROM @sqlstmt;
EXECUTE stmt;
SELECT IF(@exist > 0, 'Removing duplicate (EventId, FrameId) rows from Frames.', 'Frames.Id already removed.');
set @sqlstmt := if( @exist > 0, 'DELETE F1 FROM `Frames` F1 JOIN `Frames` F2 ON F1.`EventId` = F2.`EventId` AND F1.`FrameId` = F2.`FrameId` AND F1.`Id` > F2.`Id`', "SELECT 1");
PREPARE stmt FROM @sqlstmt;
EXECUTE stmt;
SELECT IF(@exist > 0, 'Rebuilding Frames with (EventId, FrameId) as primary key. On a large Frames table this will take some time.', '');
set @sqlstmt := if( @exist > 0, 'ALTER TABLE `Frames`
DROP PRIMARY KEY,
DROP COLUMN `Id`,
ADD PRIMARY KEY (`EventId`, `FrameId`),
DROP INDEX `EventId_FrameId_idx`,
MODIFY `TimeStamp` timestamp NOT NULL default CURRENT_TIMESTAMP', "SELECT 1");
PREPARE stmt FROM @sqlstmt;
EXECUTE stmt;
+2 -2
View File
@@ -929,14 +929,14 @@ sub recover_timestamps {
my $file = $path.'/'.$jpg;
( $file ) = $file =~ /^(.*)$/;
my $timestamp = (stat($file))[9];
my $Frame = new ZoneMinder::Frame();
$Frame->save({
my $Frame = new ZoneMinder::Frame(\@ZoneMinder::Frame::identified_by, {
EventId=>$$Event{Id}, FrameId=>$id,
TimeStamp=>Date::Format::time2str('%Y-%m-%d %H:%M:%S',$timestamp),
Delta => $timestamp - $first_timestamp,
Type=>'Normal',
Score=>0,
});
$Frame->save();
} # end if Frame not found
} # end foreach capture jpg
$ZoneMinder::Database::dbh->commit();
+2 -3
View File
@@ -33,12 +33,11 @@ require ZoneMinder::Object;
use parent qw(ZoneMinder::Object);
use vars qw/ $table $primary_key %fields /;
use vars qw/ $table @identified_by %fields /;
$table = 'Frames';
$primary_key = 'Id';
@identified_by = ('EventId', 'FrameId');
%fields = (
Id => 'Id',
EventId => 'EventId',
FrameId => 'FrameId',
Type => 'Type',
+38
View File
@@ -97,3 +97,41 @@ TEST_CASE("Frames carries the audio level the event graph plots") {
REQUIRE(block.find("'AudioLevel' => true") != std::string::npos);
}
}
TEST_CASE("Frames is keyed by (EventId, FrameId)") {
const auto repo_root = std::filesystem::path(ZM_SOURCE_DIR);
const auto schema = ReadFile(repo_root / "db" / "zm_create.sql.in");
const auto frames_at = schema.find("CREATE TABLE `Frames`");
REQUIRE(frames_at != std::string::npos);
const auto frames = schema.substr(frames_at, schema.find("ENGINE", frames_at) - frames_at);
SECTION("fresh schema has the composite primary key and no surrogate Id") {
REQUIRE(frames.find("PRIMARY KEY (`EventId`,`FrameId`)") != std::string::npos);
REQUIRE(frames.find("`Id` BIGINT") == std::string::npos);
REQUIRE(frames.find("AUTO_INCREMENT") == std::string::npos);
REQUIRE(frames.find("EventId_FrameId_idx") == std::string::npos);
}
SECTION("TimeStamp is not rewritten by updates") {
REQUIRE(frames.find("`TimeStamp` timestamp NOT NULL default CURRENT_TIMESTAMP,") != std::string::npos);
REQUIRE(frames.find("on update") == std::string::npos);
}
SECTION("nothing references Frames.Id") {
// A foreign key to Frames.Id makes the migration's DROP COLUMN fail.
const auto ai_models = ReadFile(repo_root / "db" / "AI_Models.sql");
REQUIRE(ai_models.find("REFERENCES `Frames`") == std::string::npos);
REQUIRE(schema.find("REFERENCES `Frames`") == std::string::npos);
}
SECTION("upgrade migration converts AI_Detections before dropping Id, and is re-runnable") {
const auto migration = ReadFile(repo_root / "db" / "zm_update-1.39.36.sql");
const auto convert = migration.find("SET D.`FrameId` = F.`FrameId`");
const auto drop = migration.find("DROP COLUMN `Id`");
REQUIRE(convert != std::string::npos);
REQUIRE(drop != std::string::npos);
REQUIRE(convert < drop);
REQUIRE(migration.find("ADD PRIMARY KEY (`EventId`, `FrameId`)") != std::string::npos);
REQUIRE(migration.find("column_name = 'Id'") != std::string::npos);
}
}
+1 -1
View File
@@ -1 +1 @@
1.39.35
1.39.36
+62 -49
View File
@@ -27,17 +27,22 @@ class FramesController extends AppController {
}
}
# Frames are addressed by their own Id, so the parent Event's per-monitor ACL
# has to be resolved explicitly. Without this a user denied a monitor can
# reach that monitor's frames by guessing frame Ids.
private function eventForFrame($id) {
# A single frame is addressed by (EventId, FrameId), its primary key.
private function findFrame($eventId, $frameId) {
$this->Frame->recursive = -1;
$frame = $this->Frame->find('first', array(
'conditions' => array('Frame.' . $this->Frame->primaryKey => $id)
'conditions' => array('Frame.EventId' => $eventId, 'Frame.FrameId' => $frameId)
));
if (!$frame) {
throw new NotFoundException(__('Invalid frame'));
}
return $frame;
}
# Frames carry no MonitorId, so the parent Event's per-monitor ACL has to be
# resolved explicitly.
private function eventForFrame($eventId, $frameId) {
$frame = $this->findFrame($eventId, $frameId);
$this->loadModel('Event');
$this->Event->recursive = -1;
$event = $this->Event->find('first', array(
@@ -49,18 +54,16 @@ class FramesController extends AppController {
return new ZM\Event($event['Event']);
}
# A frame being added or re-pointed names its event in the request data. Require
# edit on that event too, or a user could attach frames to a denied monitor's event.
private function requireRequestEventEdit($required) {
$data = $this->request->data;
if (isset($data['Frame']) and is_array($data['Frame'])) $data = $data['Frame'];
if (!isset($data['EventId'])) {
if ($required) throw new BadRequestException(__('EventId is required'));
return;
# A frame being added names its event in the request data. Require edit on
# that event too, or a user could attach frames to a denied monitor's event.
private function requireRequestEventEdit() {
$eventId = $this->requestField('Frame', 'EventId');
if ($eventId === null) {
throw new BadRequestException(__('EventId is required'));
}
$this->loadModel('Event');
$this->Event->recursive = -1;
$event = $this->Event->find('first', array('conditions' => array('Event.Id' => $data['EventId'])));
$event = $this->Event->find('first', array('conditions' => array('Event.Id' => $eventId)));
if (!$event) {
throw new NotFoundException(__('Invalid event'));
}
@@ -72,16 +75,29 @@ class FramesController extends AppController {
# Frame mutation is an Event mutation, so require Events=Edit as well as the
# per-monitor ACL. beforeFilter() only guarantees Events != None.
private function requireFrameEdit($id) {
private function requireFrameEdit($eventId, $frameId) {
global $user;
if ($user and ($user->Events() != 'Edit')) {
throw new UnauthorizedException(__('Insufficient Privileges'));
}
if (!$this->eventForFrame($id)->canEdit()) {
if (!$this->eventForFrame($eventId, $frameId)->canEdit()) {
throw new UnauthorizedException(__('Insufficient Privileges'));
}
}
# The request's Frame fields that are real columns. Model save() cannot be
# used to write them: it would match rows on the single-column primaryKey.
private function requestColumns($exclude = array()) {
$data = $this->request->data;
if (isset($data['Frame']) and is_array($data['Frame'])) $data = $data['Frame'];
$columns = array();
foreach (array_keys($this->Frame->schema()) as $field) {
if (in_array($field, $exclude) or !array_key_exists($field, $data)) continue;
$columns[$field] = $data[$field];
}
return $columns;
}
/**
* index method
* @return void
@@ -125,21 +141,16 @@ class FramesController extends AppController {
* view method
*
* @throws NotFoundException
* @param string $id
* @param string $eventId
* @param string $frameId
* @return void
*/
public function view($id = null) {
$this->Frame->recursive = -1;
if (!$this->Frame->exists($id)) {
throw new NotFoundException(__('Invalid frame'));
}
if (!$this->eventForFrame($id)->canView()) {
public function view($eventId = null, $frameId = null) {
if (!$this->eventForFrame($eventId, $frameId)->canView()) {
throw new UnauthorizedException(__('Insufficient Privileges'));
}
$options = array('conditions' => array('Frame.' . $this->Frame->primaryKey => $id));
$frame = $this->Frame->find('first', $options);
$this->set(array(
'frame' => $frame,
'frame' => $this->findFrame($eventId, $frameId),
'_serialize' => array('frame')
));
}
@@ -155,10 +166,11 @@ class FramesController extends AppController {
if ($user and ($user->Events() != 'Edit')) {
throw new UnauthorizedException(__('Insufficient Privileges'));
}
$this->requireRequestEventEdit(true);
$this->pinRequestId($this->Frame, null);
$this->Frame->create();
if ($this->Frame->save($this->request->data)) {
$this->requireRequestEventEdit();
$columns = $this->requestColumns();
$this->Frame->set($columns);
if ($this->Frame->validates() and
$this->Frame->getDataSource()->create($this->Frame, array_keys($columns), array_values($columns))) {
return $this->flash(__('The frame has been saved.'), array('action' => 'index'));
}
}
@@ -169,24 +181,28 @@ class FramesController extends AppController {
/**
* edit method
*
* EventId and FrameId are the key and cannot be changed.
*
* @throws NotFoundException
* @param string $id
* @param string $eventId
* @param string $frameId
* @return void
*/
public function edit($id = null) {
if (!$this->Frame->exists($id)) {
throw new NotFoundException(__('Invalid frame'));
}
$this->requireFrameEdit($id);
public function edit($eventId = null, $frameId = null) {
$this->requireFrameEdit($eventId, $frameId);
if ($this->request->is(array('post', 'put'))) {
$this->pinRequestId($this->Frame, $id);
$this->requireRequestEventEdit(false);
if ($this->Frame->save($this->request->data)) {
# updateAll() takes SQL expressions, so the values must be quoted here.
$db = $this->Frame->getDataSource();
$columns = array();
foreach ($this->requestColumns(array('EventId', 'FrameId')) as $field => $value) {
$columns[$field] = $db->value($value, $this->Frame->getColumnType($field));
}
if (count($columns) and $this->Frame->updateAll($columns,
array('Frame.EventId' => $eventId, 'Frame.FrameId' => $frameId))) {
return $this->flash(__('The frame has been saved.'), array('action' => 'index'));
}
} else {
$options = array('conditions' => array('Frame.' . $this->Frame->primaryKey => $id));
$this->request->data = $this->Frame->find('first', $options);
$this->request->data = $this->findFrame($eventId, $frameId);
}
$events = $this->Frame->Event->find('list');
$this->set(compact('events'));
@@ -196,17 +212,14 @@ class FramesController extends AppController {
* delete method
*
* @throws NotFoundException
* @param string $id
* @param string $eventId
* @param string $frameId
* @return void
*/
public function delete($id = null) {
$this->Frame->id = $id;
if (!$this->Frame->exists()) {
throw new NotFoundException(__('Invalid frame'));
}
public function delete($eventId = null, $frameId = null) {
$this->request->allowMethod('post', 'delete');
$this->requireFrameEdit($id);
if ($this->Frame->delete()) {
$this->requireFrameEdit($eventId, $frameId);
if ($this->Frame->deleteAll(array('Frame.EventId' => $eventId, 'Frame.FrameId' => $frameId), false)) {
return $this->flash(__('The frame has been deleted.'), array('action' => 'index'));
} else {
return $this->flash(__('The frame could not be deleted. Please, try again.'), array('action' => 'index'));
+7 -1
View File
@@ -17,9 +17,15 @@ class Frame extends AppModel {
/**
* Primary key field
*
* The real key is (EventId, FrameId), which CakePHP 2 cannot express. EventId
* is its leftmost column, and is what Event's dependent hasMany cascade
* resolves Frames by, so deleting an event deletes exactly its frames.
* FramesController addresses single frames by both columns and never relies
* on save(), exists() or delete() by this key.
*
* @var string
*/
public $primaryKey = 'Id';
public $primaryKey = 'EventId';
/**
* Validation rules
-1
View File
@@ -7,7 +7,6 @@ require_once('Object.php');
class Frame extends ZM_Object {
protected static $table = 'Frames';
protected $defaults = array(
'Id' => null,
'EventId' => 0,
'FrameId' => 0,
'Type' => 'Normal',
+4 -22
View File
@@ -498,28 +498,10 @@ if ( empty($_REQUEST['path']) ) {
} # if special frame (snapshot, alarm etc) or identified by id
} else {
# If we are only specifying fid, then the fid must be the primary key into the frames table. But when the event is specified, then it is the frame #
$Frame = ZM\Frame::find_one(array('Id'=>$_REQUEST['fid']));
if ( !$Frame ) {
header('HTTP/1.0 404 Not Found');
ZM\Error('Frame ' . $_REQUEST['fid'] . ' Not Found');
return;
}
$Event = ZM\Event::find_one(array('Id'=>$Frame->EventId()));
if ( !$Event ) {
header('HTTP/1.0 404 Not Found');
ZM\Error('Event ' . $Frame->EventId() . ' Not Found');
return;
}
// Per-event ACL: see GHSA-vj5r-pc2v-gfwv. The frame id is user-supplied so the
// event/monitor it resolves to may be one the user is denied from viewing.
if (!$Event->canView()) {
header('HTTP/1.0 404 Not Found');
ZM\Warning('Event '.$Frame->EventId().' access denied via frame '.$_REQUEST['fid']);
return;
}
$path = $Event->Path().'/'.sprintf('%0'.ZM_EVENT_IMAGE_DIGITS.'d',$Frame->FrameId()).'-'.$show.'.jpg';
# A frame is only identified by its event and its number within the event.
header('HTTP/1.0 404 Not Found');
ZM\Error('No Event ID specified for frame '.validInt($_REQUEST['fid']));
return;
} # end if have eid
if ( !file_exists($path) ) {