Skip to content

CO-2916_SqlSource_May_Not_Detect_All_Changes #739

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
ioigoume wants to merge 1 commit into
base: hotfix-4.4.x
Choose a base branch
from
Draft

CO-2916_SqlSource_May_Not_Detect_All_Changes #739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 31 additions & 18 deletions app/AvailablePlugin/SqlSource/Model/SqlSourceBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,35 +790,48 @@ public function updateCache() {
// prevent the cache from being updated, in that case we'll just end up reprocessing
// some records, which should effectively be a no-op.

if(!empty($this->tableList)) {
// We just need any model to get access to query(), it doesn't have to be the
// one associated with the table we're updating (since we're not looking at the
// query results).
if(empty($this->tableList)) {
$this->log(__METHOD__ . "::Empty list of changed records.", LOG_DEBUG);
return true;
}

$QueryTable = $this->getRecordModel();
// We just need any model to get access to query(), it doesn't have to be the
// one associated with the table we're updating (since we're not looking at the
// query results).

foreach($this->tableList as $sourceTableName) {
$archiveTableName = $sourceTableName . "_archive";
$QueryTable = $this->getRecordModel();

// Rather than try to determine which individual columns across which records
// were updated, we delete any updated record from the archive table, then
// insert any rows that were added from or updated in the source table.
foreach($this->tableList as $sourceTableName) {
$archiveTableName = $sourceTableName . "_archive";

// Only the primary table uses id, so we use sorid as a general solution.

$deleteQuery = "DELETE FROM " . $archiveTableName . " WHERE sorid IN
// Rather than try to determine which individual columns across which records
// were updated, we delete any updated record from the archive table, then
// insert any rows that were added from or updated in the source table.

// Only the primary table uses id, so we use sorid as a general solution.

$deleteQuery = "DELETE FROM " . $archiveTableName . " WHERE sorid IN
(SELECT diff.sorid FROM
(SELECT * FROM " . $archiveTableName . " EXCEPT
SELECT * FROM " . $sourceTableName . ") AS diff)";

$QueryTable->query($deleteQuery);

$insertQuery = "INSERT INTO " . $archiveTableName . "
$deleteResult = $QueryTable->query($deleteQuery);
if ($deleteResult === false) {
$this->log(__METHOD__ . "::Error deleting records from the {$archiveTableName} table.", LOG_ERR);
return false;
}


$insertQuery = "INSERT INTO " . $archiveTableName . "
(SELECT * FROM " . $sourceTableName . " EXCEPT
SELECT * FROM " . $archiveTableName . ")";

$QueryTable->query($insertQuery);

$insertResult = $QueryTable->query($insertQuery);
if ($insertResult === false) {
$this->log(__METHOD__ . "::Error inserting records into the {$archiveTableName} table.", LOG_ERR);
return false;
}

}

return true;
Expand Down
47 changes: 35 additions & 12 deletions app/Model/OrgIdentitySource.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1456,7 +1456,7 @@ public function syncOrgIdentity($id, $sourceKey, $actorCoPersonId=null, $jobId=n
*/

public function syncOrgIdentitySource($orgIdentitySource, $force=false, $coJobId=null) {
$dbc = ConnectionManager::getDataSource('default');;
$dbc = ConnectionManager::getDataSource('default');

// We don't check here that the source is in Manual mode in case an admin
// wants to manually force a sync. (syncAll honors that setting.)
Expand Down Expand Up @@ -1560,15 +1560,21 @@ public function syncOrgIdentitySource($orgIdentitySource, $force=false, $coJobId
if(!$coJobId) {
$this->Co->CoJob->finish($jobId, $e->getMessage(), JobStatusEnum::Failed);
}
$this->log(__METHOD__ . "::throw::" . $e->getMessage(), LOG_DEBUG);
throw new RuntimeException($e->getMessage());
}
}

if($changelist !== false) {
$known = 0; // The number of records reported changed that we already know about

foreach($changelist as $srckey) {
if($this->Co->CoJob->canceled($jobId)) { return false; }
$this->log(__METHOD__ . "Changelist length: " . count($changelist), LOG_DEBUG);
foreach($changelist as $idx => $srckey) {
$this->log(__METHOD__ . "[$idx] Processing source key: " . $srckey, LOG_DEBUG);

if($this->Co->CoJob->canceled($jobId)) {
$this->log(__METHOD__ . "[$idx] $srckey, Job Canceled.", LOG_DEBUG);
return false;
}

try {
// syncOrgIdentity does NOT create a new org identity if none exists, it
Expand Down Expand Up @@ -1603,6 +1609,7 @@ public function syncOrgIdentitySource($orgIdentitySource, $force=false, $coJobId
$resCnt['unchanged'] = count($orgRecords) - $known;
}

$this->log(__METHOD__ . ":resCnt" . print_r($resCnt, true), LOG_DEBUG);
$this->Co->CoJob->CoJobHistoryRecord->record($jobId,
null,
_txt('jb.ois.sync.update.changed',
Expand All @@ -1613,8 +1620,11 @@ public function syncOrgIdentitySource($orgIdentitySource, $force=false, $coJobId
} else {
// Changelist not supported, perform per-record sync

foreach($orgRecords as $rec) {
if($this->Co->CoJob->canceled($jobId)) { return false; }
foreach($orgRecords as $idxx => $rec) {
if($this->Co->CoJob->canceled($jobId)) {
$this->log(__METHOD__ . "[$idxx] $rec, Job Canceled.", LOG_DEBUG);
return false;
}

try {
$r = $this->syncOrgIdentity($rec['OrgIdentitySourceRecord']['org_identity_source_id'],
Expand All @@ -1634,7 +1644,7 @@ public function syncOrgIdentitySource($orgIdentitySource, $force=false, $coJobId
// but if we do we rollback so that we do not continue onto
// the next OrgIdentity with an open transaction.
$dbc = $this->getDataSource();
for($i = $dbc->getTransactionNesting();$i >= 0;$i--) {
for($i = $dbc->getTransactionNesting(); $i >= 0; $i--) {
$dbc->rollback();
}
}
Expand Down Expand Up @@ -1670,6 +1680,8 @@ public function syncOrgIdentitySource($orgIdentitySource, $force=false, $coJobId
catch(Exception $e) {
$eclass = get_class($e);
$err = $e->getMessage();

$this->log(__METHOD__ . "Exception A Full: " . $e->getMessage() , LOG_DEBUG);

if($eclass == 'DomainException') {
// We're misconfigured, the backend does not support inventory().
Expand Down Expand Up @@ -1697,8 +1709,11 @@ public function syncOrgIdentitySource($orgIdentitySource, $force=false, $coJobId
null,
JobStatusEnum::Notice);

foreach($newKeys as $newKey) {
if($this->Co->CoJob->canceled($jobId)) { return false; }
foreach($newKeys as $idy => $newKey) {
if($this->Co->CoJob->canceled($jobId)) {
$this->log(__METHOD__ . "[$idy] $newKey, Job Canceled.", LOG_DEBUG);
return false;
}

// This is basically the same logic as used in SyncModeEnum::Query, below
try {
Expand All @@ -1723,8 +1738,11 @@ public function syncOrgIdentitySource($orgIdentitySource, $force=false, $coJobId
// There's already an associated identity. We could log a message,
// but that seems like it'll get noisy. We don't increment a counter
// either since we should have counted this in 'synced' already.
$this->log(__METHOD__ . "OverflowException Full: " . $e->getMessage() , LOG_DEBUG);
}
catch(Exception $e) {
$this->log(__METHOD__ . "Exception B Full: " . $e->getMessage() , LOG_DEBUG);

// Create a job history record to record the error.
// Because of the exception we just caught there may be one or
// more open transactions so roll them back so that we can
Expand Down Expand Up @@ -1852,8 +1870,12 @@ public function syncOrgIdentitySource($orgIdentitySource, $force=false, $coJobId
null,
JobStatusEnum::Notice);

foreach($emailList as $ea) {
if($this->Co->CoJob->canceled($jobId)) { return false; }
foreach($emailList as $ide => $ea) {
if($this->Co->CoJob->canceled($jobId))
{
$this->log(__METHOD__ . "[$ide] $ea: Job Canceled.", LOG_DEBUG);
return false;
}

// Since this is search and not retrieve, it's technically possible to get
// more than one result back from a source, if (eg) there are multiple records
Expand Down Expand Up @@ -1974,7 +1996,8 @@ public function syncOrgIdentitySource($orgIdentitySource, $force=false, $coJobId
if(!$coJobId) {
$this->Co->CoJob->finish($jobId, json_encode($resCnt));
}


$this->log(__METHOD__ . ":finished the sync.", LOG_DEBUG);
return true;
}

Expand Down
3 changes: 3 additions & 0 deletions lib/Cake/Model/Datasource/DboSource.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,11 +510,14 @@ protected function _execute($sql, $params = array(), $prepareOptions = array())
return true;
}
}
CakeLog::write(LOG_DEBUG, "query: \n" . $query->queryString);
return $query;
} catch (PDOException $e) {
if (isset($query->queryString)) {
CakeLog::write(LOG_ERROR, "query: \n" . $query->queryString);
$e->queryString = $query->queryString;
} else {
CakeLog::write(LOG_ERROR, "sql: \n" . $sql);
$e->queryString = $sql;
}
throw $e;
Expand Down