Merge pull request #133 from ConductionNL/development

Update main with job test run and job log fixes
ConductionNL · Dec 13, 2024 · beb7fcc · beb7fcc
2 parents d938ed9 + 772fe63
commit beb7fcc
Show file tree

Hide file tree

Showing 5 changed files with 113 additions and 62 deletions.
diff --git a/lib/Controller/JobsController.php b/lib/Controller/JobsController.php
@@ -236,13 +236,18 @@ public function run(int $id): JSONResponse
         }
 
         try {
-            $this->jobList->getById($job->getJobListId())->start($this->jobList);
-            $lastLog = $this->jobLogMapper->getLastCallLog();
-            if ($lastLog !== null) {
-                return new JSONResponse(data: $lastLog, statusCode: 200);
-            }
-
-            return new JSONResponse(data: ['error' => 'No job log could be found, job did not went succesfully or failed to log anything'], statusCode: 500);
+            $job = $this->jobList->getById($job->getJobListId());
+			if ($job !== null) {
+				$job->setArgument(['jobId' => $id, 'forceRun' => true]);
+				$job->start($this->jobList);
+
+				$lastLog = $this->jobLogMapper->getLastCallLog();
+				if ($lastLog !== null && ($lastLog->getJobId() === null || (int) $lastLog->getJobId() === $id)) {
+					return new JSONResponse(data: $lastLog, statusCode: 200);
+				}
+			}
+
+            return new JSONResponse(data: ['error' => 'No job log could be found, job did not go successfully or failed to log anything'], statusCode: 500);
         } catch (Exception $exception) {
             return new JSONResponse(data: ['error' => $exception->getMessage()], statusCode: 400);
         }

diff --git a/lib/Controller/SynchronizationsController.php b/lib/Controller/SynchronizationsController.php
@@ -2,6 +2,7 @@
 
 namespace OCA\OpenConnector\Controller;
 
+use GuzzleHttp\Exception\GuzzleException;
 use OCA\OpenConnector\Service\ObjectService;
 use OCA\OpenConnector\Service\SearchService;
 use OCA\OpenConnector\Service\SynchronizationService;
@@ -15,6 +16,8 @@
 use OCP\IRequest;
 use Exception;
 use OCP\AppFramework\Db\DoesNotExistException;
+use Psr\Container\ContainerExceptionInterface;
+use Psr\Container\NotFoundExceptionInterface;
 
 class SynchronizationsController extends Controller
 {
@@ -213,33 +216,36 @@ public function logs(int $id): JSONResponse
         }
     }
 
-    /**
-     * Tests a synchronization
-     *
-     * This method tests a synchronization without persisting anything to the database.
-     *
-     * @NoAdminRequired
-     * @NoCSRFRequired
-     *
-     * @param int $id The ID of the synchronization
-     *
-     * @return JSONResponse A JSON response containing the test results
-     *
-     * @example
-     * Request:
-     * empty POST
-     *
-     * Response:
-     * {
-     *     "resultObject": {
-     *         "fullName": "John Doe",
-     *         "userAge": 30,
-     *         "contactEmail": "[email protected]"
-     *     },
-     *     "isValid": true,
-     *     "validationErrors": []
-     * }
-     */
+	/**
+	 * Tests a synchronization
+	 *
+	 * This method tests a synchronization without persisting anything to the database.
+	 *
+	 * @NoAdminRequired
+	 * @NoCSRFRequired
+	 *
+	 * @param int $id The ID of the synchronization
+	 *
+	 * @return JSONResponse A JSON response containing the test results
+	 * @throws GuzzleException
+	 * @throws ContainerExceptionInterface
+	 * @throws NotFoundExceptionInterface
+	 *
+	 * @example
+	 * Request:
+	 * empty POST
+	 *
+	 * Response:
+	 * {
+	 *     "resultObject": {
+	 *         "fullName": "John Doe",
+	 *         "userAge": 30,
+	 *         "contactEmail": "[email protected]"
+	 *     },
+	 *     "isValid": true,
+	 *     "validationErrors": []
+	 * }
+	 */
     public function test(int $id): JSONResponse
     {
         try {
@@ -263,4 +269,4 @@ public function test(int $id): JSONResponse
 
         return new JSONResponse($resultFromTest, 200);
     }
-}
+}
diff --git a/lib/Cron/ActionTask.php b/lib/Cron/ActionTask.php
@@ -63,28 +63,49 @@ public function __construct(
     public function run($argument)
     {
         // if we do not have a job id then everything is wrong
-        if (isset($arguments['jobId']) === true && is_int($argument['jobId']) === true) {
-            return;
+        if (isset($argument['jobId']) === false || is_int($argument['jobId']) === false) {
+			return $this->jobLogMapper->createFromArray([
+				'jobId'         => 'null',
+				'level'			=> 'ERROR',
+				'message'		=> "Couldn't find a jobId in the action argument"
+			]);
         }
 
-        // Let's get the job, the user might have deleted it in the mean time
+        // Let's get the job, the user might have deleted it in the meantime
         try {
             $job = $this->jobMapper->find($argument['jobId']);
         } catch (Exception $e) {
-            return;
+            return $this->jobLogMapper->createFromArray([
+				'jobId'         => $argument['jobId'],
+				'level'			=> 'ERROR',
+				'message'		=> "Couldn't find a Job with this jobId, message: ".$e->getMessage()
+			]);
         }
 
+		$forceRun = false;
+		$stackTrace = [];
+		if (isset($argument['forceRun']) === true && $argument['forceRun'] === true) {
+			$forceRun = true;
+			$stackTrace[] = 'Doing a force run for this job, ignoring "enabled" & "nextRun" check...';
+		}
+
         // If the job is not enabled, we don't need to do anything
-        if ($job->getIsEnabled() === false) {
-            return;
+        if ($forceRun === false && $job->getIsEnabled() === false) {
+			return $this->jobLogMapper->createForJob($job, [
+				'level'			=> 'WARNING',
+				'message'		=> 'This job is disabled'
+			]);
         }
 
-        // if the next run is in the the future, we don't need to do anything
-        if ($job->getNextRun() !== null && $job->getNextRun() > new DateTime()) {
-            return;
+        // if the next run is in the future, we don't need to do anything
+        if ($forceRun === false && $job->getNextRun() !== null && $job->getNextRun() > new DateTime()) {
+			return $this->jobLogMapper->createForJob($job, [
+				'level'			=> 'WARNING',
+				'message'		=> 'Next Run is still in the future for this job'
+			]);
         }
 
-		if(empty($job->getUserId()) === false && $this->userSession->getUser() === null) {
+		if (empty($job->getUserId()) === false && $this->userSession->getUser() === null) {
 			$user = $this->userManager->get($job->getUserId());
 			$this->userSession->setUser($user);
 		}
@@ -102,26 +123,23 @@ public function run($argument)
         $executionTime = ( $time_end - $time_start ) * 1000;
 
         // deal with single run
-        if ($job->isSingleRun() === true) {
+        if ($forceRun === false && $job->isSingleRun() === true) {
             $job->setIsEnabled(false);
         }
 
-
         // Update the job
-        $job->setLastRun(new DateTime());
-		$nextRun = new DateTime('now + '.$job->getInterval().' seconds');
-		$nextRun->setTime(hour: $nextRun->format('H'), minute: $nextRun->format('i'));
-        $job->setNextRun($nextRun);
-        $this->jobMapper->update($job);
+		if ($forceRun === false) {
+			$job->setLastRun(new DateTime());
+			$nextRun = new DateTime('now + '.$job->getInterval().' seconds');
+			$nextRun->setTime(hour: $nextRun->format('H'), minute: $nextRun->format('i'));
+			$job->setNextRun($nextRun);
+			$this->jobMapper->update($job);
+		}
 
         // Log the job
-        $jobLog = $this->jobLogMapper->createFromArray([
-            'jobId'         => $job->getId(),
-            'jobClass'      => $job->getJobClass(),
-            'jobListId'     => $job->getJobListId(),
-            'arguments'     => $job->getArguments(),
-            'lastRun'       => $job->getLastRun(),
-            'nextRun'       => $job->getNextRun(),
+        $jobLog = $this->jobLogMapper->createForJob($job, [
+			'level'			=> 'INFO',
+			'message'		=> 'Succes',
             'executionTime' => $executionTime
         ]);
 
@@ -134,10 +152,12 @@ public function run($argument)
                 $jobLog->setMessage($result['message']);
             }
             if (isset($result['stackTrace']) === true) {
-                $jobLog->setStackTrace($result['stackTrace']);
+				$stackTrace = array_merge($stackTrace, $result['stackTrace']);
             }
         }
 
+		$jobLog->setStackTrace($stackTrace);
+
 		$this->jobLogMapper->update(entity: $jobLog);
 
         // Let's report back about what we have just done

diff --git a/lib/Db/JobLogMapper.php b/lib/Db/JobLogMapper.php
@@ -56,8 +56,28 @@ public function findAll(?int $limit = null, ?int $offset = null, ?array $filters
         return $this->findEntities($qb);
     }
 
+	public function createForJob(Job $job, array $object): JobLog
+	{
+		$jobObject = [
+			'jobId'         => $job->getId(),
+			'jobClass'      => $job->getJobClass(),
+			'jobListId'     => $job->getJobListId(),
+			'arguments'     => $job->getArguments(),
+			'lastRun'       => $job->getLastRun(),
+			'nextRun'       => $job->getNextRun(),
+		];
+
+		$object = array_merge($jobObject, $object);
+
+		return $this->createFromArray($object);
+	}
+
     public function createFromArray(array $object): JobLog
     {
+		if (isset($object['executionTime']) === false) {
+			$object['executionTime'] = 0;
+		}
+
         $obj = new JobLog();
 		$obj->hydrate($object);
 		// Set uuid

diff --git a/lib/Service/JobService.php b/lib/Service/JobService.php
@@ -51,10 +51,10 @@ public function scheduleJob(Job $job): Job
         $arguments['jobId'] = $job->getId();
 
         if (!$job->getScheduleAfter()) {
-            $iJob = $this->jobList->add($this->actionTask::class, $arguments);
+            $this->jobList->add($this->actionTask::class, $arguments);
         } else {
             $runAfter = $job->getScheduleAfter()->getTimestamp();
-            $iJob = $this->jobList->scheduleAfter($this->actionTask::class, $runAfter, $arguments);
+            $this->jobList->scheduleAfter($this->actionTask::class, $runAfter, $arguments);
         }
 
         // Set the job list id