diff --git a/plugins/Jobs.cpp b/plugins/Jobs.cpp index 7ef28fede..5b07d4987 100644 --- a/plugins/Jobs.cpp +++ b/plugins/Jobs.cpp @@ -281,12 +281,12 @@ bool BedrockJobsCommand::peek(SQLite& db) { } // Verify unique, but only do so when creating a single job using CreateJob - if (SIEquals(requestVerb, "CreateJob") && SContains(job, "unique") && job["unique"] == "true") { + if (SContains(job, "unique") && job["unique"] == "true") { SQResult result; SINFO("Unique flag was passed, checking existing job with name " << job["name"] << ", mocked? " << (mockRequest ? "true" : "false")); string operation = mockRequest ? "IS NOT" : "IS"; - if (!db.read("SELECT jobID, data " + if (!db.read("SELECT jobID, data, parentJobID " "FROM jobs " "WHERE name=" + SQ(job["name"]) + " AND JSON_EXTRACT(data, '$.mockRequest') " + operation + " NULL;", @@ -295,12 +295,19 @@ bool BedrockJobsCommand::peek(SQLite& db) { } // If there's no job or the existing job doesn't match the data we've been passed, escalate to leader. - if (!result.empty() && ((job["data"].empty() && result[0][1] == "{}") || (!job["data"].empty() && result[0][1] == job["data"]))) { - // Return early, no need to pass to leader, there are no more jobs to create. - SINFO("Job already existed and unique flag was passed, reusing existing job " << result[0][0] << ", mocked? " - << (mockRequest ? "true" : "false")); - jsonContent["jobID"] = result[0][0]; - return true; + if (!result.empty()) { + // If the parent passed does not match the parent the job already had, then it must mean we did something + // wrong or made a bad CQ, so we throw so we can investigate. Updating the parent here would be + // confusing, as it could leave the original parent in a bad state (like for example paused forever) + if (result[0][2] != "0" && result[0][2] != job["parentJobID"]) { + STHROW("404 Trying to create a child that already exists, but it is tied to a different parent"); + } + if (SIEquals(requestVerb, "CreateJob") && ((job["data"].empty() && result[0][1] == "{}") || (!job["data"].empty() && result[0][1] == job["data"]))) { + // Return early, no need to pass to leader, there are no more jobs to create. + SINFO("Job already existed and unique flag was passed, reusing existing job " << result[0][0] << ", mocked? " << (mockRequest ? "true" : "false")); + jsonContent["jobID"] = result[0][0]; + return true; + } } } } diff --git a/test/tests/jobs/CreateJobsTest.cpp b/test/tests/jobs/CreateJobsTest.cpp index 39ef58c4e..5512a4b7e 100644 --- a/test/tests/jobs/CreateJobsTest.cpp +++ b/test/tests/jobs/CreateJobsTest.cpp @@ -11,6 +11,7 @@ struct CreateJobsTest : tpunit::TestFixture { TEST(CreateJobsTest::createWithInvalidJson), TEST(CreateJobsTest::createWithParentIDNotRunning), TEST(CreateJobsTest::createWithParentMocked), + TEST(CreateJobsTest::createUniqueChildWithWrongParent), AFTER(CreateJobsTest::tearDown), AFTER_CLASS(CreateJobsTest::tearDownClass)) { } @@ -195,4 +196,58 @@ struct CreateJobsTest : tpunit::TestFixture { ASSERT_EQUAL(result.rows.size(), 0); } + + void createUniqueChildWithWrongParent() + { + // Create 2 parents + SData command("CreateJob"); + command["name"] = "createWithParent1"; + string response = tester->executeWaitVerifyContent(command); + STable responseJSON = SParseJSONObject(response); + string parentID1 = responseJSON["jobID"]; + command.clear(); + command.methodLine = "CreateJob"; + command["name"] = "createWithParent2"; + response = tester->executeWaitVerifyContent(command); + responseJSON = SParseJSONObject(response); + string parentID2 = responseJSON["jobID"]; + + // Get the parents (to set it running) + command.clear(); + command.methodLine = "GetJob"; + command["name"] = "createWithParent1"; + tester->executeWaitVerifyContent(command); + command.clear(); + command.methodLine = "GetJob"; + command["name"] = "createWithParent2"; + tester->executeWaitVerifyContent(command); + + // Send the command to create the child in parent1 + command.clear(); + command.methodLine = "CreateJobs"; + STable job1Content; + STable data1; + data1["blabla"] = "blabla"; + job1Content["name"] = "createWithParent_child"; + job1Content["data"] = SComposeJSONObject(data1); + job1Content["parentJobID"] = parentID1; + vector jobs; + jobs.push_back(SComposeJSONObject(job1Content)); + command["jobs"] = SComposeJSONArray(jobs); + tester->executeWaitVerifyContent(command); + + // Try to create the same child with unique, but pass parent2 instead + command.clear(); + command.methodLine = "CreateJobs"; + data1["blabla"] = "blabla"; + job1Content["name"] = "createWithParent_child"; + job1Content["unique"] = "true"; + job1Content["data"] = SComposeJSONObject(data1); + job1Content["parentJobID"] = parentID2; + jobs.clear(); + jobs.push_back(SComposeJSONObject(job1Content)); + command["jobs"] = SComposeJSONArray(jobs); + tester->executeWaitVerifyContent(command, "404 Trying to create a child that already exists, but it is tied to a different parent"); + } + } __CreateJobsTest;