Skip to content

Commit 6582cd3

Browse files
authored
✨ feat(icu): wire ICU validation into TM Analysis + fix FastAnalysis transaction lifecycle (#4517)
* ✨ feat(icu): integrate optional ICU MessageFormat validation - add support for detecting and validating ICU MessageFormat patterns in segments - wire ICU detection in TMAnalysisWorker and PostProcess - update FastAnalysis to handle ICU-enabled metadata Signed-off-by: domenico <[email protected]> * ✅ test(icu): add ICU validation tests for PostProcess and ICUSourceSegmentDetector Cover the PostProcess → QA ICU wiring (PR #4517) and the shared detection gate used by both QAProcessor and TMAnalysisWorker: - 3 PostProcess tests: broken target detected, skipped without comparator, valid pass - 4 ICUSourceSegmentDetector tests: enabled+valid, disabled, plain text, simple argument * 🔧 chore(phpstan): update baseline for FastAnalysis refactoring and baseline APIDoc.php * 🔧 chore(phpstan): exclude APIDoc.php from analysis paths - add `excludePaths` to phpstan.neon configuration to skip lib/View/APIDoc.php Signed-off-by: domenico <[email protected]> * 🔧 fix(phpstan): remove baseline entries for untracked APIDoc.php * 🔧 refactor(icu): add null guard in FastAnalysis, rename icuPluralsValidator to icuComparator * ♻️ refactor(analysis): add Database::transaction() and fix FastAnalysis TX lifecycle Add Database::transaction(callable) for safe atomic execution with automatic rollback on failure. Refactor all 4 transaction sites in FastAnalysis.php: - Fix rollBack() called with no active transaction (crash on error) - Fix null dereference + dangling TX in _updateProject() - Fix mixed API usage in _getLockProjectForVolumeAnalysis() - Remove dead unset() calls for undefined variables - Remove 5 stale PHPStan baseline entries --------- Signed-off-by: domenico <[email protected]>
1 parent fe0cf80 commit 6582cd3

11 files changed

Lines changed: 317 additions & 158 deletions

File tree

internal_scripts

lib/Model/DataAccess/Database.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Exception;
66
use PDO;
77
use PDOException;
8+
use Throwable;
89

910
/**
1011
* Class which implements a database using PDO
@@ -181,6 +182,26 @@ public function rollback(): void
181182
$this->getConnection()->rollBack();
182183
}
183184

185+
/**
186+
* @Override
187+
* {@inheritdoc}
188+
*/
189+
public function transaction(callable $callback): mixed
190+
{
191+
$this->begin();
192+
try {
193+
$result = $callback();
194+
$this->commit();
195+
196+
return $result;
197+
} catch (Throwable $e) {
198+
if ($this->getConnection()->inTransaction()) {
199+
$this->rollback();
200+
}
201+
throw $e;
202+
}
203+
}
204+
184205
/**
185206
* @Warning This method does not support all the SQL syntax features. Only AND key/value pair is supported, OR in WHERE condition is not supported, nesting "AND ( .. OR .. ) AND ( .. )" is not supported
186207
* @Override

lib/Model/DataAccess/IDatabase.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Model\DataAccess;
44

55
use PDO;
6+
use Throwable;
67

78
interface IDatabase
89
{
@@ -56,6 +57,22 @@ public function commit(): void;
5657
*/
5758
public function rollback(): void;
5859

60+
/**
61+
* Execute a callback within a database transaction.
62+
*
63+
* Begins a transaction, executes the callback, and commits.
64+
* On any exception, rolls back (if still in transaction) and re-throws.
65+
*
66+
* @template T
67+
*
68+
* @param callable(): T $callback
69+
*
70+
* @return T The value returned by the callback
71+
*
72+
* @throws Throwable Re-throws the original exception after rollback
73+
*/
74+
public function transaction( callable $callback ): mixed;
75+
5976
/**
6077
* Execute a update query with an array as argument
6178
*

lib/Utils/AsyncTasks/Workers/Analysis/FastAnalysis.php

Lines changed: 74 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
use Model\Jobs\JobsMetadataMarshaller;
1414
use Model\Jobs\MetadataDao;
1515
use Model\MTQE\Templates\DTO\MTQEWorkflowParams;
16-
use Model\Projects\MetadataDao as ProjectsMetadataDao;
1716
use Model\Projects\ProjectDao;
1817
use Model\Projects\ProjectsMetadataMarshaller;
1918
use Model\Projects\ProjectStruct;
@@ -24,6 +23,7 @@
2423
use PDOException;
2524
use ReflectionException;
2625
use Stomp\Transport\Message;
26+
use Throwable;
2727
use UnexpectedValueException;
2828
use Utils\ActiveMQ\AMQHandler;
2929
use Utils\AsyncTasks\Workers\Traits\ProjectWordCount;
@@ -240,22 +240,38 @@ public function main(array $args = null): void
240240
// INSERT DATA
241241
$this->logger->debug("Inserting segments...");
242242

243-
// define variable for the sake of the code, even if empty
244243
$projectStruct = new ProjectStruct();
245244

246245
try {
247246
/**
248247
* Ensure we have fresh data from the master node
249248
*/
250-
Database::obtain()->getConnection()->beginTransaction();
251-
$projectStruct = ProjectDao::findById($pid);
252-
$projectFeaturesString = $projectStruct->getMetadataValue(ProjectsMetadataMarshaller::FEATURES_KEY->value);
253-
$mt_evaluation = $projectStruct->getMetadataValue(ProjectsMetadataMarshaller::MT_EVALUATION->value);
254-
$mt_qe_workflow_enabled = $projectStruct->getMetadataValue(ProjectsMetadataMarshaller::MT_QE_WORKFLOW_ENABLED->value);
255-
$mt_qe_workflow_parameters = $projectStruct->getMetadataValue(ProjectsMetadataMarshaller::MT_QE_WORKFLOW_PARAMETERS->value);
256-
$mt_quality_value_in_editor = $projectStruct->getMetadataValue(ProjectsMetadataMarshaller::MT_QUALITY_VALUE_IN_EDITOR->value);
257-
$subfiltering_handlers = (new ProjectsMetadataDao)->getProjectStaticSubfilteringCustomHandlers($projectStruct->id);
258-
Database::obtain()->getConnection()->commit();
249+
$metadataResult = Database::obtain()->transaction(function () use ($pid) {
250+
$projectStruct = ProjectDao::findById($pid);
251+
if ($projectStruct === null) {
252+
return null;
253+
}
254+
255+
return [
256+
'project' => $projectStruct,
257+
'metadata' => $projectStruct->getAllMetadataAsKeyValue(),
258+
];
259+
});
260+
261+
if ($metadataResult === null) {
262+
$this->logger->error("Unable to insert fast analysis: project not found for pid $pid");
263+
continue;
264+
}
265+
266+
$projectStruct = $metadataResult['project'];
267+
$allMetadata = $metadataResult['metadata'];
268+
$projectFeaturesString = $allMetadata[ProjectsMetadataMarshaller::FEATURES_KEY->value] ?? '';
269+
$mt_evaluation = $allMetadata[ProjectsMetadataMarshaller::MT_EVALUATION->value] ?? false;
270+
$mt_qe_workflow_enabled = $allMetadata[ProjectsMetadataMarshaller::MT_QE_WORKFLOW_ENABLED->value] ?? false;
271+
$mt_qe_workflow_parameters = $allMetadata[ProjectsMetadataMarshaller::MT_QE_WORKFLOW_PARAMETERS->value] ?? null;
272+
$mt_quality_value_in_editor = $allMetadata[ProjectsMetadataMarshaller::MT_QUALITY_VALUE_IN_EDITOR->value] ?? 85;
273+
$subfiltering_handlers = $allMetadata[ProjectsMetadataMarshaller::SUBFILTERING_HANDLERS->value] ?? [];
274+
$icu_enabled = $allMetadata[ProjectsMetadataMarshaller::ICU_ENABLED->value] ?? false;
259275

260276
$insertReportRes = $this->_insertFastAnalysis(
261277
$projectStruct,
@@ -267,11 +283,10 @@ public function main(array $args = null): void
267283
$mt_qe_workflow_enabled,
268284
$mt_qe_workflow_parameters,
269285
$mt_quality_value_in_editor,
270-
$subfiltering_handlers
286+
$subfiltering_handlers,
287+
$icu_enabled
271288
);
272-
} catch (Exception $e) {
273-
//Logging done and email sent
274-
//set to error
289+
} catch (Throwable $e) {
275290
$insertReportRes = -1;
276291
$this->logger->debug($e->getMessage() . " " . $e->getTraceAsString());
277292
}
@@ -399,16 +414,23 @@ public function cleanShutDown(): void
399414
*/
400415
protected function _updateProject($pid, $status): void
401416
{
402-
Database::obtain()->begin();
403-
$project = ProjectDao::findById($pid);
404-
if ($project->status_analysis != ProjectStatus::STATUS_DONE) { // avoid concurrency between fast and tm daemons ( they set DONE when complete )
405-
$this->logger->debug("*** Project $pid: Changing status...");
406-
ProjectDao::changeProjectStatus($pid, $status);
407-
$this->logger->debug("*** Project $pid: $status");
408-
} else {
409-
$this->logger->debug("*** Project $pid: TM Analysis already completed. Skip update...");
410-
}
411-
Database::obtain()->commit();
417+
Database::obtain()->transaction(function () use ($pid, $status) {
418+
$project = ProjectDao::findById($pid);
419+
if ($project === null) {
420+
$this->logger->debug("*** Project $pid: not found. Skip update.");
421+
422+
return;
423+
}
424+
425+
// avoid concurrency between fast and tm daemons ( they set DONE when complete )
426+
if ($project->status_analysis != ProjectStatus::STATUS_DONE) {
427+
$this->logger->debug("*** Project $pid: Changing status...");
428+
ProjectDao::changeProjectStatus($pid, $status);
429+
$this->logger->debug("*** Project $pid: $status");
430+
} else {
431+
$this->logger->debug("*** Project $pid: TM Analysis already completed. Skip update...");
432+
}
433+
});
412434
}
413435

414436
/**
@@ -452,6 +474,7 @@ protected function _executeInsert($tuple_list, $bind_values): void
452474
* @param MTQEWorkflowParams|null $mt_qe_workflow_parameters
453475
* @param int|null $mt_quality_value_in_editor
454476
* @param array|null $subfiltering_handlers
477+
* @param bool $icu_enabled
455478
* @return int
456479
* @throws Exception
457480
*/
@@ -465,7 +488,8 @@ protected function _insertFastAnalysis(
465488
?bool $mt_qe_workflow_enabled = false,
466489
?MTQEWorkflowParams $mt_qe_workflow_parameters = null,
467490
?int $mt_quality_value_in_editor = 85,
468-
?array $subfiltering_handlers = []
491+
?array $subfiltering_handlers = [],
492+
bool $icu_enabled = false
469493
): int {
470494
$pid = $projectStruct->id;
471495
$total_eq_wc = 0;
@@ -560,45 +584,35 @@ protected function _insertFastAnalysis(
560584
}
561585
}
562586

563-
unset($data);
564587
unset($tuple_list);
565-
unset($chunks_bind_values);
566-
unset($chunks_st);
567-
568-
//_TimeStampMsg( "Done." );
569588

570589
$data2 = ['fast_analysis_wc' => $total_eq_wc];
571590
$where = ["id" => $pid];
572591

573-
574-
$db = Database::obtain();
575-
$db->begin();
576-
577592
try {
578-
/*
579-
* IF NO TM ANALYSIS, update the jobs global word count
580-
*/
581-
if (!$perform_Tms_Analysis) {
582-
$_details = $this->getProjectSegmentsTranslationSummary($pid);
593+
$project_creation_success = Database::obtain()->transaction(function () use ($perform_Tms_Analysis, $pid, $data2, $where) {
594+
/*
595+
* IF NO TM ANALYSIS, update the jobs global word count
596+
*/
597+
if (!$perform_Tms_Analysis) {
598+
$_details = $this->getProjectSegmentsTranslationSummary($pid);
583599

584-
$this->logger->debug("--- trying to initialize job total word count.");
600+
$this->logger->debug("--- trying to initialize job total word count.");
585601

586-
/** @noinspection PhpUnusedLocalVariableInspection */
587-
$query_rollup = array_pop($_details); //Don't remove, needed to remove rollup row
602+
/** @noinspection PhpUnusedLocalVariableInspection */
603+
$query_rollup = array_pop($_details); //Don't remove, needed to remove rollup row
588604

589-
foreach ($_details as $job_info) {
590-
$counter = new CounterModel();
591-
$counter->initializeJobWordCount($job_info['id_job'], $job_info['password']);
605+
foreach ($_details as $job_info) {
606+
$counter = new CounterModel();
607+
$counter->initializeJobWordCount($job_info['id_job'], $job_info['password']);
608+
}
592609
}
593-
}
594-
/* IF NO TM ANALYSIS, upload the jobs global word count */
610+
/* IF NO TM ANALYSIS, upload the jobs global word count */
595611

596-
$project_creation_success = $db->update('projects', $data2, $where);
597-
598-
$db->commit();
612+
return Database::obtain()->update('projects', $data2, $where);
613+
});
599614
} catch (PDOException $e) {
600615
$this->logger->debug($e->getMessage());
601-
$db->rollback();
602616

603617
return $e->getCode() * -1;
604618
}
@@ -700,6 +714,7 @@ protected function _insertFastAnalysis(
700714
$queue_element['mt_quality_value_in_editor'] = $mt_quality_value_in_editor ?? false;
701715

702716
$queue_element[JobsMetadataMarshaller::SUBFILTERING_HANDLERS->value] = $subfiltering_handlers;
717+
$queue_element[ProjectsMetadataMarshaller::ICU_ENABLED->value] = $icu_enabled;
703718

704719
$element = new QueueElement();
705720
$element->params = new Params($queue_element);
@@ -894,14 +909,13 @@ protected function _getLockProjectForVolumeAnalysis(int $limit = 1): array
894909
ORDER BY id LIMIT " . $limit;
895910

896911
$db = Database::obtain();
897-
//Needed to address the query to the master database if exists
898-
Database::obtain()->begin();
899-
900-
$stmt = $db->getConnection()->prepare($query);
901-
$stmt->execute($bindParams);
902-
$results = $stmt->fetchAll(PDO::FETCH_ASSOC);
912+
// Needed to address the query to the master database if exists
913+
$results = $db->transaction(function () use ($db, $query, $bindParams) {
914+
$stmt = $db->getConnection()->prepare($query);
915+
$stmt->execute($bindParams);
903916

904-
$db->getConnection()->commit();
917+
return $stmt->fetchAll(PDO::FETCH_ASSOC);
918+
});
905919

906920
foreach ($results as $position => $project) {
907921
//acquire a lock
@@ -913,7 +927,7 @@ protected function _getLockProjectForVolumeAnalysis(int $limit = 1): array
913927

914928
try {
915929
$this->_updateProject($project['id'], ProjectStatus::STATUS_BUSY);
916-
} catch (PDOException) {
930+
} catch (Exception) {
917931
$this->queueHandler->getRedisClient()->del('_fPid:' . $project['id']);
918932
}
919933
}

0 commit comments

Comments
 (0)