Skip to content

PLAT-25826: Prevent Duplicate Entry Export - #13960

Open
MaimanaGhnem wants to merge 3 commits into
West-23.4.0from
Venus-22.20.0-PLAT-25826
Open

PLAT-25826: Prevent Duplicate Entry Export#13960
MaimanaGhnem wants to merge 3 commits into
West-23.4.0from
Venus-22.20.0-PLAT-25826

Conversation

@MaimanaGhnem

Copy link
Copy Markdown
Contributor

Pull Request Checklist

Please complete the following before submitting:

General notes -

  • I have tested the changes locally.
  • I have written unit tests where applicable.
  • I have updated documentation where needed.
  • I have added comments to complex code.
  • This PR follows the coding style guidelines.
  • I have updated release notes with new feature

New Kaltura Types

  • I have created new clients
  • I have notified related apps - KMCNG / KMS / EP .... about new clients

New Kaltura Services / Actions

  • I have added a deployment script

Questions

  1. What is the purpose of this PR?

    • Enter your answer here
  2. Does this change affect production code or infrastructure?

    • Yes
    • No
  3. If yes, what is the rollback plan?

    • Enter your answer here

@github-actions

Copy link
Copy Markdown

@github-copilot suggest

@shahbaa123
shahbaa123 changed the base branch from Venus-22.20.0 to West-23.4.0 July 22, 2026 12:03
@shahbaa123

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved by merging the latest Venus-22.20.0 into this branch in commit db31b61037, which clears the PR merge conflicts.

Copilot AI requested a review from shahbaa123 July 22, 2026 12:07
@github-actions

Copy link
Copy Markdown

@github-copilot suggest

@shahbaa123

Copy link
Copy Markdown
Collaborator

🤖 AI PR Review

🚦 Verdict: 🔴 CRITICAL

The PR contains a merge-conflict resolution error that deletes a live error constant used by ShortLinkService, which will cause PHP fatal errors on short link operations, plus a type-mismatch bug in the DC filter that would silently make the duplicate-check logic ineffective.


📌 Context Summary & Code Archaeology

Objective (PLAT-25826): Prevent duplicate entry/exportCSV API requests for the same partner. When a large export takes time, clients resend the request assuming it was lost. The fix adds a pre-flight check in kJobsManager::addExportCsvJob that queries for an active (non-closed) export job for the partner before creating a new one, throwing a kCoreException that is caught and surfaced as a KalturaAPIException to the API caller.

Track classification: Track B — Surgical Fix. Files under alpha/apps/kaltura/lib/batch2/ are designated legacy paths. All Track B constraints were checked.

Sibling search — hasActiveEntryExportJob: New method, no pre-existing sibling implementations found. The three other callers of addExportCsvJob (ESearchHistoryService, EntryVendorTaskService, ESearchService) do not pass ExportObjectType::ENTRY, so the new guard does not affect them.

Origin commit for addExportCsvJob: Introduced cleanly as a new method — no inherited debt or deliberate design constraints that conflict with adding this guard.


📋 Actionable Feedback

Finding 1 — BLOCKER
File: api_v3/lib/KalturaErrors.php

Two constants — INVALID_SHORT_LINK_DOMAIN and INVALID_LINK_PREFIX — are deleted from this file in the diff. This is a merge conflict resolution error: the new ENTRY_EXPORT_CSV_IN_PROGRESS constant was added but the two short-link constants directly above it in the file were silently dropped in the process.

INVALID_SHORT_LINK_DOMAIN is actively referenced at plugins/short_link/services/ShortLinkService.php:224:

throw new KalturaAPIException(KalturaErrors::INVALID_SHORT_LINK_DOMAIN, $id);

After this PR merges, any request that hits the short-link domain validation path will throw a PHP fatal error (Undefined class constant) rather than the expected API exception. This is a live regression on an unrelated service.

Fix: Restore both deleted constants. The correct state after the merge should be:

const INVALID_SHORT_LINK_DOMAIN = "INVALID_SHORT_LINK_DOMAIN;ID;Domain of short link [@ID@] is not allowed";
const INVALID_LINK_PREFIX = "INVALID_LINK_PREFIX;;The provided url must be https";

/*
 * Export CSV service
 */
const ENTRY_EXPORT_CSV_IN_PROGRESS = "ENTRY_EXPORT_CSV_IN_PROGRESS;;An entry export job is already in progress. Please wait for it to complete before starting a new export.";

Finding 2 — MAJOR
File: alpha/apps/kaltura/lib/batch2/kJobsManager.phphasActiveEntryExportJob, line adding DC filter

kDataCenterMgr::getCurrentDc() returns an array (the full DC config map, e.g. ['id' => 0, 'name' => '...']), not an integer. BatchJobPeer::DC is an integer column. Passing an array as a Criteria value with the default Criteria::EQUAL operator will produce an Array to string conversion notice and invalid SQL in PHP 8, causing the method to either error out or silently return no results (meaning the duplicate check is bypassed entirely).

The correct call — consistent with all other DC assignments in this same file (lines 1566 and 1678) — is kDataCenterMgr::getCurrentDcId().

$c->add(BatchJobPeer::DC, kDataCenterMgr::getCurrentDcId());

Finding 3 — MAJOR
File: alpha/apps/kaltura/lib/batch2/kJobsManager.phphasActiveEntryExportJob, DC filter design

Even after correcting Finding 2, the DC filter introduces a scope problem. CSV export jobs are partner-scoped operations (the result is emailed to the user regardless of which DC processes it). Filtering by the current DC means that if a partner's first export job was submitted via DC 0 and the duplicate request arrives at DC 1, hasActiveEntryExportJob will find no job for DC 1 and allow a second job to be created — defeating the purpose of the feature entirely.

Furthermore, addExportCsvJob does not call $batchJob->setDc(...) before dispatching to addJob(). If the DC column is null or defaults to a system value on job creation, the DC filter in the query may never match any jobs.

The duplicate check should be per-partner across all DCs. Remove the DC criterion:

public static function hasActiveEntryExportJob($partnerId)
{
    $c = new Criteria();
    $c->add(BatchJobPeer::PARTNER_ID, $partnerId);
    $c->add(BatchJobPeer::JOB_TYPE, BatchJobType::EXPORT_CSV);
    $c->add(BatchJobPeer::JOB_SUB_TYPE, ExportObjectType::ENTRY);
    $c->add(BatchJobPeer::STATUS, BatchJobPeer::getUnClosedStatusList(), Criteria::IN);

    $existingJob = BatchJobPeer::doSelectOne($c);
    // ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants