mirror of
https://github.com/opensourcepos/opensourcepos.git
synced 2026-09-21 02:07:23 -04:00
fix(jobs): validate all throttles before saving any
Bug: saveThrottles validated and saved throttles in same loop. Invalid entry mid-loop stopped processing but left already-saved throttles committed, and omitted throttles could still get deleted via notToDelete tracking — partial/inconsistent state on failure. - app/Controllers/Jobs.php: split into two passes — validate all throttleData entries first; on any invalid entry, roll back transaction and return failure immediately before touching DB. Only save/delete once full payload validated. Drop now-redundant $success flag folded into invalid-entry early return. - tests/Controllers/JobsControllerTest.php: add test asserting existing throttles stay unchanged in DB when payload contains invalid throttle data. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
This commit is contained in:
1 parent
9f70efb592
commit
171dd056b3
2 files changed
+28
-7
No files matched your search
@@ -98,14 +98,15 @@ class Jobs extends Secure_Controller
|
||||
}
|
||||
}
|
||||
|
||||
$success = true;
|
||||
foreach ($arraySave as $throttleData) {
|
||||
if (!ctype_digit((string)$throttleData['max_count']) || !in_array($throttleData['period'], $allowedPeriods, true)) {
|
||||
$this->db->transRollback();
|
||||
|
||||
return $this->response->setJSON(['success' => false, 'message' => lang('Jobs.saved_unsuccessfully')]);
|
||||
}
|
||||
}
|
||||
|
||||
foreach ($arraySave as $throttleId => $throttleData) {
|
||||
if (!ctype_digit((string)$throttleData['max_count']) || !in_array($throttleData['period'], $allowedPeriods, true)) {
|
||||
$success = false;
|
||||
continue;
|
||||
}
|
||||
|
||||
$savedThrottleId = $this->jobThrottle->saveValue($throttleData, $throttleId);
|
||||
$notToDelete[] = (string)$savedThrottleId;
|
||||
}
|
||||
@@ -121,7 +122,7 @@ class Jobs extends Secure_Controller
|
||||
|
||||
$this->db->transComplete();
|
||||
|
||||
$success = $success && $this->db->transStatus();
|
||||
$success = $this->db->transStatus();
|
||||
|
||||
return $this->response->setJSON(['success' => $success, 'message' => lang('Jobs.saved_' . ($success ? '' : 'un') . 'successfully')]);
|
||||
}
|
||||
|
||||
@@ -135,6 +135,26 @@ class JobsControllerTest extends CIUnitTestCase
|
||||
$this->assertFalse($result['success']);
|
||||
}
|
||||
|
||||
public function testPostSaveThrottlesLeavesExistingThrottlesUnchangedWhenPayloadInvalid(): void
|
||||
{
|
||||
$this->loginAsAdmin();
|
||||
$this->jobThrottle->saveValue(['max_count' => 5, 'period' => 'minute'], 1);
|
||||
$this->jobThrottle->saveValue(['max_count' => 20, 'period' => 'day'], 2);
|
||||
|
||||
// Throttle 2 is invalid and throttle 1 is omitted; neither should be touched.
|
||||
$response = $this->post('/jobs/saveThrottles', [
|
||||
'throttle_count_2' => 15,
|
||||
'throttle_period_2' => 'fortnight',
|
||||
]);
|
||||
|
||||
$response->assertStatus(200);
|
||||
$result = json_decode($response->getJSON(), true);
|
||||
$this->assertFalse($result['success']);
|
||||
|
||||
$this->seeInDatabase('job_throttles', ['throttle_id' => 1, 'max_count' => 5, 'period' => 'minute', 'deleted' => 0]);
|
||||
$this->seeInDatabase('job_throttles', ['throttle_id' => 2, 'max_count' => 20, 'period' => 'day', 'deleted' => 0]);
|
||||
}
|
||||
|
||||
public function testGetThrottlesRendersPartial(): void
|
||||
{
|
||||
$this->loginAsAdmin();
|
||||
|
||||
Reference in new issue
Block a user