Skip to content

creator as non-classic data object #161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
abaevbog wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

creator as non-classic data object #161

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9a15ff7
creator as non-classic data object
abaevbog Jul 21, 2023
0561025
small libraryID fix
abaevbog Jul 21, 2023
e03e1dc
bulk-insert new creators + minor cleanup
abaevbog Jul 24, 2023
170704d
store all data in itemCreators table
abaevbog Jul 24, 2023
dd18ed3
php db migration script + use old files in autoload
abaevbog Jul 25, 2023
422ac13
removed typo from header
abaevbog Jul 25, 2023
baf5c93
removing unused code, prevent updating unchanged creators
abaevbog Jul 27, 2023
8a25c08
removing tags as classic data objects - in progress
abaevbog Jul 28, 2023
ed68a2e
tag tests passing, move old model files
abaevbog Jul 28, 2023
ea9030c
itemTags primary key including itemID
abaevbog Jul 28, 2023
327f68e
final cleanup, tests passing
abaevbog Jul 31, 2023
2956830
moved checking of previously existing items outside of the loop
abaevbog Jul 31, 2023
938f1fe
no creatorID, added key to itemTags, tag insert performace improvemen…
abaevbog Aug 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions misc/db-updates/2023-07-17/classic_data_objects.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
alter table itemCreators drop constraint itemCreators_ibfk_2;
DROP table creators;

DROP trigger if exists fki_itemCreators_libraryID;
DROP trigger if exists fku_itemCreators_libraryID;

CREATE TABLE `creators` (
`creatorID` bigint unsigned NOT NULL AUTO_INCREMENT,
`firstName` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci DEFAULT NULL,
`lastName` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci DEFAULT NULL,
`fieldMode` tinyint(1) unsigned DEFAULT NULL,
PRIMARY KEY (`creatorID`),
KEY `name` (`lastName`(7),`firstName`(6))
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

@dstillman dstillman Jul 24, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to be a PHP script like the others that does a proper migration of existing data. It should set the shard to down, wait 20 seconds, do the migration, and bring the shard up.

@abaevbog abaevbog Jul 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I only kept it as .sql until now for convenience.
I'll have it wrapped into the same algorithm as this update, except with state=down.

1 change: 1 addition & 0 deletions misc/master.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ CREATE TABLE `libraries` (
`lastUpdated` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00',
`version` int(10) unsigned NOT NULL DEFAULT '0',
`shardID` smallint(5) unsigned NOT NULL,
`hasData` TINYINT( 1 ) NOT NULL DEFAULT '0',

@dstillman dstillman Jul 24, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add this, but it shouldn't be in this PR.

@abaevbog abaevbog Jul 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I push just this one line to master?

@dstillman dstillman Jul 24, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

PRIMARY KEY (`libraryID`),
KEY `shardID` (`shardID`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
Expand Down
2 changes: 1 addition & 1 deletion misc/shard.sql
View file
Open in desktop

@dstillman dstillman Aug 1, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need all the schema changes here

Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ CREATE TABLE `syncDeleteLogIDs` (

CREATE TABLE `syncDeleteLogKeys` (
`libraryID` int(10) unsigned NOT NULL,
`objectType` enum('collection','creator','item','relation','search','setting','tag','tagName') NOT NULL,
`objectType` enum('collection','item','relation','search','setting','tag','tagName') NOT NULL,

@dstillman dstillman Aug 1, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tag should be removed

`key` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL,
`timestamp` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
`version` int(10) unsigned NOT NULL DEFAULT '1',
Expand Down
159 changes: 18 additions & 141 deletions model/Creator.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,40 +27,30 @@
class Zotero_Creator {
private $id;
private $libraryID;
private $key;
private $firstName = '';
private $lastName = '';
private $shortName = '';
private $fieldMode = 0;
private $birthYear;
private $dateAdded;
private $dateModified;

private $loaded = false;
private $creatorTypeID;
private $changed = array();

public function __construct() {
$numArgs = func_num_args();
if ($numArgs) {
throw new Exception("Constructor doesn't take any parameters");
}

$this->init();
}



private function init() {
$this->loaded = false;

public function __construct($id, $libraryID, $firstName, $lastName, $fieldMode, $creatorTypeID = null) {
$this->id = $id;
$this->libraryID = $libraryID;
$this->firstName = $firstName;
$this->lastName = $lastName;
$this->fieldMode = $fieldMode;
$this->creatorTypeID = $creatorTypeID;
$this->changed = array();
$props = array(
'libraryID',
'firstName',
'lastName',
'shortName',
'fieldMode',
'birthYear',
'dateAdded',
'dateModified'
'creatorTypeID'
);
foreach ($props as $prop) {
$this->changed[$prop] = false;
Expand All @@ -69,10 +59,7 @@ private function init() {


public function __get($field) {
if (($this->id || $this->key) && !$this->loaded) {
$this->load(true);
}


if (!property_exists('Zotero_Creator', $field)) {
throw new Exception("Zotero_Creator property '$field' doesn't exist");
}
Expand All @@ -85,10 +72,6 @@ public function __set($field, $value) {
switch ($field) {
case 'id':
case 'libraryID':
case 'key':
if ($this->loaded) {
throw new Exception("Cannot set $field after creator is already loaded");
}
$this->checkValue($field, $value);
$this->$field = $value;
return;
Expand All @@ -99,15 +82,6 @@ public function __set($field, $value) {
break;
}

if ($this->id || $this->key) {
if (!$this->loaded) {
$this->load(true);
}
}
else {
$this->loaded = true;
}

$this->checkValue($field, $value);

if ($this->$field !== $value) {
Expand All @@ -117,20 +91,6 @@ public function __set($field, $value) {
}


/**
* Check if creator exists in the database
*
* @return bool TRUE if the item exists, FALSE if not
*/
public function exists() {
if (!$this->id) {
trigger_error('$this->id not set');
}

$sql = "SELECT COUNT(*) FROM creators WHERE creatorID=?";
return !!Zotero_DB::valueQuery($sql, $this->id, Zotero_Shards::getByLibraryID($this->libraryID));
}


public function hasChanged() {
return in_array(true, array_values($this->changed));
Expand All @@ -153,7 +113,7 @@ public function save($userID=false) {
throw new Exception('First name must be empty in single-field mode');
}

if (!$this->hasChanged()) {
if (!$this->hasChanged() && isset($this->id)) {
Z_Core::debug("Creator $this->id has not changed");
return false;
}
Expand All @@ -166,24 +126,13 @@ public function save($userID=false) {

Z_Core::debug("Saving creator $this->id");

$key = $this->key ? $this->key : Zotero_ID::getKey();

$timestamp = Zotero_DB::getTransactionTimestamp();

$dateAdded = $this->dateAdded ? $this->dateAdded : $timestamp;
$dateModified = !empty($this->changed['dateModified']) ? $this->dateModified : $timestamp;

$fields = "firstName=?, lastName=?, fieldMode=?,
libraryID=?, `key`=?, dateAdded=?, dateModified=?, serverDateModified=?";
$fields = "firstName=?, lastName=?, fieldMode=?";
$params = array(
$this->firstName,
$this->lastName,
$this->fieldMode,
$this->libraryID,
$key,
$dateAdded,
$dateModified,
$timestamp
$this->fieldMode
);
$shardID = Zotero_Shards::getByLibraryID($this->libraryID);

Expand All @@ -193,9 +142,6 @@ public function save($userID=false) {
$stmt = Zotero_DB::getStatement($sql, true, $shardID);
Zotero_DB::queryFromStatement($stmt, array_merge(array($creatorID), $params));

// Remove from delete log if it's there
$sql = "DELETE FROM syncDeleteLogKeys WHERE libraryID=? AND objectType='creator' AND `key`=?";
Zotero_DB::query($sql, array($this->libraryID, $key), $shardID);
}
else {
$sql = "UPDATE creators SET $fields WHERE creatorID=?";
Expand Down Expand Up @@ -234,18 +180,6 @@ public function save($userID=false) {

Zotero_DB::commit();

Zotero_Creators::cachePrimaryData(
array(
'id' => $creatorID,
'libraryID' => $this->libraryID,
'key' => $key,
'dateAdded' => $dateAdded,
'dateModified' => $dateModified,
'firstName' => $this->firstName,
'lastName' => $this->lastName,
'fieldMode' => $this->fieldMode
)
);
}
catch (Exception $e) {
Zotero_DB::rollback();
Expand All @@ -256,19 +190,13 @@ public function save($userID=false) {
if (!$this->id) {
$this->id = $creatorID;
}
if (!$this->key) {
$this->key = $key;
}

$this->init();


if ($isNew) {
Zotero_Creators::cache($this);
}

// TODO: invalidate memcache?

return $this->id;
}


Expand All @@ -294,53 +222,14 @@ public function getLinkedItems() {
}


public function equals($creator) {
if (!$this->loaded) {
$this->load();
}

public function equals($creator) {
return
($creator->firstName === $this->firstName) &&
($creator->lastName === $this->lastName) &&
($creator->fieldMode == $this->fieldMode);
}


private function load() {
if (!$this->libraryID) {
throw new Exception("Library ID not set");
}

if (!$this->id && !$this->key) {
throw new Exception("ID or key not set");
}

if ($this->id) {
//Z_Core::debug("Loading data for creator $this->libraryID/$this->id");
$row = Zotero_Creators::getPrimaryDataByID($this->libraryID, $this->id);
}
else {
//Z_Core::debug("Loading data for creator $this->libraryID/$this->key");
$row = Zotero_Creators::getPrimaryDataByKey($this->libraryID, $this->key);
}

$this->loaded = true;
$this->changed = array();

if (!$row) {
return;
}

if ($row['libraryID'] != $this->libraryID) {
throw new Exception("libraryID {$row['libraryID']} != $this->libraryID");
}

foreach ($row as $key=>$val) {
$this->$key = $val;
}
}


private function checkValue($field, $value) {
if (!property_exists($this, $field)) {
throw new Exception("Invalid property '$field'");
Expand All @@ -350,6 +239,7 @@ private function checkValue($field, $value) {
switch ($field) {
case 'id':
case 'libraryID':
case 'creatorTypeID':
if (!Zotero_Utilities::isPosInt($value)) {
$this->invalidValueError($field, $value);
}
Expand All @@ -360,19 +250,6 @@ private function checkValue($field, $value) {
$this->invalidValueError($field, $value);
}
break;

case 'key':
if (!preg_match('/^[23456789ABCDEFGHIJKMNPQRSTUVWXTZ]{8}$/', $value)) {
$this->invalidValueError($field, $value);
}
break;

case 'dateAdded':
case 'dateModified':
if ($value !== '' && !preg_match("/^[0-9]{4}\-[0-9]{2}\-[0-9]{2} ([0-1][0-9]|[2][0-3]):([0-5][0-9]):([0-5][0-9])$/", $value)) {
$this->invalidValueError($field, $value);
}
break;
}
}

Expand Down
Loading