| Conditions | 1 |
| Paths | 1 |
| Total Lines | 109 |
| Code Lines | 62 |
| Lines | 0 |
| Ratio | 0 % |
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
| 1 | <?php |
||
| 257 | public function testJobHealthCheck() |
||
| 258 | { |
||
| 259 | // Create a job and add it to the queue |
||
| 260 | $svc = $this->getService(); |
||
| 261 | $job = new TestQueuedJob(QueuedJob::IMMEDIATE); |
||
| 262 | $job->firstJob = true; |
||
| 263 | $id = $svc->queueJob($job); |
||
| 264 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
| 265 | |||
| 266 | // Verify initial state is new and LastProcessedCount is not marked yet |
||
| 267 | $this->assertEquals(QueuedJob::STATUS_NEW, $descriptor->JobStatus); |
||
| 268 | $this->assertEquals(0, $descriptor->StepsProcessed); |
||
| 269 | $this->assertEquals(-1, $descriptor->LastProcessedCount); |
||
| 270 | $this->assertEquals(0, $descriptor->ResumeCounts); |
||
| 271 | |||
| 272 | // Loop 1 - Pick up new job and attempt to run it |
||
| 273 | // Job health should not attempt to cleanup unstarted jobs |
||
| 274 | $svc->checkJobHealth(); |
||
| 275 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
| 276 | |||
| 277 | // Ensure that this is the next job ready to go |
||
| 278 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
| 279 | $this->assertEquals($nextJob->ID, $descriptor->ID); |
||
| 280 | $this->assertEquals(QueuedJob::STATUS_NEW, $descriptor->JobStatus); |
||
| 281 | $this->assertEquals(0, $descriptor->StepsProcessed); |
||
| 282 | $this->assertEquals(-1, $descriptor->LastProcessedCount); |
||
| 283 | $this->assertEquals(0, $descriptor->ResumeCounts); |
||
| 284 | |||
| 285 | // Run 1 - Start the job (no work is done) |
||
| 286 | $descriptor->JobStatus = QueuedJob::STATUS_INIT; |
||
| 287 | $descriptor->write(); |
||
| 288 | |||
| 289 | // Assume that something bad happens at this point, the process dies during execution, and |
||
| 290 | // the task is re-initiated somewhere down the track |
||
| 291 | |||
| 292 | // Loop 2 - Detect broken job, and mark it for future checking. |
||
| 293 | $svc->checkJobHealth(); |
||
| 294 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
| 295 | |||
| 296 | // Note that we don't immediately try to restart it until StepsProcessed = LastProcessedCount |
||
| 297 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
| 298 | $this->assertFalse($nextJob); // Don't run it this round please! |
||
| 299 | $this->assertEquals(QueuedJob::STATUS_INIT, $descriptor->JobStatus); |
||
| 300 | $this->assertEquals(0, $descriptor->StepsProcessed); |
||
| 301 | $this->assertEquals(0, $descriptor->LastProcessedCount); |
||
| 302 | $this->assertEquals(0, $descriptor->ResumeCounts); |
||
| 303 | |||
| 304 | // Loop 3 - We've previously marked this job as broken, so restart it this round |
||
| 305 | // If no more work has been done on the job at this point, assume that we are able to |
||
| 306 | // restart it |
||
| 307 | $svc->checkJobHealth(); |
||
| 308 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
| 309 | |||
| 310 | // This job is resumed and exeuction is attempted this round |
||
| 311 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
| 312 | $this->assertEquals($nextJob->ID, $descriptor->ID); |
||
| 313 | $this->assertEquals(QueuedJob::STATUS_WAIT, $descriptor->JobStatus); |
||
| 314 | $this->assertEquals(0, $descriptor->StepsProcessed); |
||
| 315 | $this->assertEquals(0, $descriptor->LastProcessedCount); |
||
| 316 | $this->assertEquals(1, $descriptor->ResumeCounts); |
||
| 317 | |||
| 318 | // Run 2 - First restart (work is done) |
||
| 319 | $descriptor->JobStatus = QueuedJob::STATUS_RUN; |
||
| 320 | $descriptor->StepsProcessed++; // Essentially delays the next restart by 1 loop |
||
| 321 | $descriptor->write(); |
||
| 322 | |||
| 323 | // Once again, at this point, assume the job fails and crashes |
||
| 324 | |||
| 325 | // Loop 4 - Assuming a job has LastProcessedCount < StepsProcessed we are in the same |
||
| 326 | // situation as step 2. |
||
| 327 | // Because the last time the loop ran, StepsProcessed was incremented, |
||
| 328 | // this indicates that it's likely that another task could be working on this job, so |
||
| 329 | // don't run this. |
||
| 330 | $svc->checkJobHealth(); |
||
| 331 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
| 332 | |||
| 333 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
| 334 | $this->assertFalse($nextJob); // Don't run jobs we aren't sure should be restarted |
||
| 335 | $this->assertEquals(QueuedJob::STATUS_RUN, $descriptor->JobStatus); |
||
| 336 | $this->assertEquals(1, $descriptor->StepsProcessed); |
||
| 337 | $this->assertEquals(1, $descriptor->LastProcessedCount); |
||
| 338 | $this->assertEquals(1, $descriptor->ResumeCounts); |
||
| 339 | |||
| 340 | // Loop 5 - Job is again found to not have been restarted since last iteration, so perform second |
||
| 341 | // restart. The job should be attempted to run this loop |
||
| 342 | $svc->checkJobHealth(); |
||
| 343 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
| 344 | |||
| 345 | // This job is resumed and exeuction is attempted this round |
||
| 346 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
| 347 | $this->assertEquals($nextJob->ID, $descriptor->ID); |
||
| 348 | $this->assertEquals(QueuedJob::STATUS_WAIT, $descriptor->JobStatus); |
||
| 349 | $this->assertEquals(1, $descriptor->StepsProcessed); |
||
| 350 | $this->assertEquals(1, $descriptor->LastProcessedCount); |
||
| 351 | $this->assertEquals(2, $descriptor->ResumeCounts); |
||
| 352 | |||
| 353 | // Run 3 - Second and last restart (no work is done) |
||
| 354 | $descriptor->JobStatus = QueuedJob::STATUS_RUN; |
||
| 355 | $descriptor->write(); |
||
| 356 | |||
| 357 | // Loop 6 - As no progress has been made since loop 3, we can mark this as dead |
||
| 358 | $svc->checkJobHealth(); |
||
| 359 | $nextJob = $svc->getNextPendingJob(QueuedJob::IMMEDIATE); |
||
| 360 | |||
| 361 | // Since no StepsProcessed has been done, don't wait another loop to mark this as dead |
||
| 362 | $descriptor = QueuedJobDescriptor::get()->byID($id); |
||
| 363 | $this->assertEquals(QueuedJob::STATUS_PAUSED, $descriptor->JobStatus); |
||
| 364 | $this->assertEmpty($nextJob); |
||
| 365 | } |
||
| 366 | } |
||
| 423 |
You can fix this by adding a namespace to your class:
When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.