# HG changeset patch # User Sebastien Jodogne # Date 1526480600 -7200 # Node ID 34dc57f4a7d210a4e76784b57d5f762833494195 # Parent ef7ba1b21d58df6a44a38e71d4041cc6e401ae0e simplification of JobStepResult diff -r ef7ba1b21d58 -r 34dc57f4a7d2 Core/JobsEngine/IJob.h --- a/Core/JobsEngine/IJob.h Wed May 16 14:40:05 2018 +0200 +++ b/Core/JobsEngine/IJob.h Wed May 16 16:23:20 2018 +0200 @@ -50,7 +50,7 @@ // Method called once the job enters the jobs engine virtual void Start() = 0; - virtual JobStepResult* ExecuteStep() = 0; + virtual JobStepResult ExecuteStep() = 0; // Method called once the job is resubmitted after a failure virtual void SignalResubmit() = 0; diff -r ef7ba1b21d58 -r 34dc57f4a7d2 Core/JobsEngine/JobStepResult.cpp --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/Core/JobsEngine/JobStepResult.cpp Wed May 16 16:23:20 2018 +0200 @@ -0,0 +1,81 @@ +/** + * Orthanc - A Lightweight, RESTful DICOM Store + * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics + * Department, University Hospital of Liege, Belgium + * Copyright (C) 2017-2018 Osimis S.A., Belgium + * + * This program is free software: you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * In addition, as a special exception, the copyright holders of this + * program give permission to link the code of its release with the + * OpenSSL project's "OpenSSL" library (or with modified versions of it + * that use the same license as the "OpenSSL" library), and distribute + * the linked executables. You must obey the GNU General Public License + * in all respects for all of the code used other than "OpenSSL". If you + * modify file(s) with this exception, you may extend this exception to + * your version of the file(s), but you are not obligated to do so. If + * you do not wish to do so, delete this exception statement from your + * version. If you delete this exception statement from all source files + * in the program, then also delete it here. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + **/ + + +#include "../PrecompiledHeaders.h" +#include "JobStepResult.h" + +#include "../OrthancException.h" + +namespace Orthanc +{ + JobStepResult JobStepResult::Retry(unsigned int timeout) + { + JobStepResult result(JobStepCode_Retry); + result.timeout_ = timeout; + return result; + } + + + JobStepResult JobStepResult::Failure(const ErrorCode& error) + { + JobStepResult result(JobStepCode_Failure); + result.error_ = error; + return result; + } + + + unsigned int JobStepResult::GetRetryTimeout() const + { + if (code_ == JobStepCode_Retry) + { + return timeout_; + } + else + { + throw OrthancException(ErrorCode_BadSequenceOfCalls); + } + } + + + ErrorCode JobStepResult::GetFailureCode() const + { + if (code_ == JobStepCode_Failure) + { + return error_; + } + else + { + throw OrthancException(ErrorCode_BadSequenceOfCalls); + } + } +} diff -r ef7ba1b21d58 -r 34dc57f4a7d2 Core/JobsEngine/JobStepResult.h --- a/Core/JobsEngine/JobStepResult.h Wed May 16 14:40:05 2018 +0200 +++ b/Core/JobsEngine/JobStepResult.h Wed May 16 16:23:20 2018 +0200 @@ -40,21 +40,46 @@ class JobStepResult { private: - JobStepCode code_; + JobStepCode code_; + unsigned int timeout_; + ErrorCode error_; - public: explicit JobStepResult(JobStepCode code) : - code_(code) + code_(code), + timeout_(0), + error_(ErrorCode_Success) { } - virtual ~JobStepResult() + public: + explicit JobStepResult() : + code_(JobStepCode_Failure), + timeout_(0), + error_(ErrorCode_InternalError) { } + static JobStepResult Success() + { + return JobStepResult(JobStepCode_Success); + } + + static JobStepResult Continue() + { + return JobStepResult(JobStepCode_Continue); + } + + static JobStepResult Retry(unsigned int timeout); + + static JobStepResult Failure(const ErrorCode& error); + JobStepCode GetCode() const { return code_; } + + unsigned int GetRetryTimeout() const; + + ErrorCode GetFailureCode() const; }; } diff -r ef7ba1b21d58 -r 34dc57f4a7d2 Core/JobsEngine/JobStepRetry.h --- a/Core/JobsEngine/JobStepRetry.h Wed May 16 14:40:05 2018 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,57 +0,0 @@ -/** - * Orthanc - A Lightweight, RESTful DICOM Store - * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics - * Department, University Hospital of Liege, Belgium - * Copyright (C) 2017-2018 Osimis S.A., Belgium - * - * This program is free software: you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * In addition, as a special exception, the copyright holders of this - * program give permission to link the code of its release with the - * OpenSSL project's "OpenSSL" library (or with modified versions of it - * that use the same license as the "OpenSSL" library), and distribute - * the linked executables. You must obey the GNU General Public License - * in all respects for all of the code used other than "OpenSSL". If you - * modify file(s) with this exception, you may extend this exception to - * your version of the file(s), but you are not obligated to do so. If - * you do not wish to do so, delete this exception statement from your - * version. If you delete this exception statement from all source files - * in the program, then also delete it here. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - **/ - - -#pragma once - -#include "JobStepResult.h" - -namespace Orthanc -{ - class JobStepRetry : public JobStepResult - { - private: - unsigned int timeout_; // Retry after "timeout_" milliseconds - - public: - JobStepRetry(unsigned int timeout) : - JobStepResult(JobStepCode_Retry), - timeout_(timeout) - { - } - - unsigned int GetTimeout() const - { - return timeout_; - } - }; -} diff -r ef7ba1b21d58 -r 34dc57f4a7d2 Core/JobsEngine/JobsEngine.cpp --- a/Core/JobsEngine/JobsEngine.cpp Wed May 16 14:40:05 2018 +0200 +++ b/Core/JobsEngine/JobsEngine.cpp Wed May 16 16:23:20 2018 +0200 @@ -34,8 +34,6 @@ #include "../PrecompiledHeaders.h" #include "JobsEngine.h" -#include "JobStepRetry.h" - #include "../Logging.h" #include "../OrthancException.h" @@ -67,59 +65,47 @@ return false; } - std::auto_ptr result; - - { - try - { - result.reset(running.GetJob().ExecuteStep()); + JobStepResult result; - if (result->GetCode() == JobStepCode_Failure) - { - running.UpdateStatus(ErrorCode_InternalError); - } - else - { - running.UpdateStatus(ErrorCode_Success); - } - } - catch (OrthancException& e) - { - running.UpdateStatus(e.GetErrorCode()); - } - catch (boost::bad_lexical_cast&) - { - running.UpdateStatus(ErrorCode_BadFileFormat); - } - catch (...) - { - running.UpdateStatus(ErrorCode_InternalError); - } - - if (result.get() == NULL) - { - result.reset(new JobStepResult(JobStepCode_Failure)); - } + try + { + result = running.GetJob().ExecuteStep(); + } + catch (OrthancException& e) + { + result = JobStepResult::Failure(e.GetErrorCode()); + } + catch (boost::bad_lexical_cast&) + { + result = JobStepResult::Failure(ErrorCode_BadFileFormat); + } + catch (...) + { + result = JobStepResult::Failure(ErrorCode_InternalError); } - switch (result->GetCode()) + switch (result.GetCode()) { case JobStepCode_Success: + running.UpdateStatus(ErrorCode_Success); running.GetJob().ReleaseResources(); running.MarkSuccess(); return false; case JobStepCode_Failure: running.GetJob().ReleaseResources(); + running.UpdateStatus(result.GetFailureCode()); running.MarkFailure(); return false; case JobStepCode_Retry: running.GetJob().ReleaseResources(); - running.MarkRetry(dynamic_cast(*result).GetTimeout()); + running.UpdateStatus(ErrorCode_Success); + running.MarkRetry(result.GetRetryTimeout()); return false; case JobStepCode_Continue: + running.UpdateStatus(ErrorCode_Success); return true; default: diff -r ef7ba1b21d58 -r 34dc57f4a7d2 Core/JobsEngine/SetOfInstancesJob.cpp --- a/Core/JobsEngine/SetOfInstancesJob.cpp Wed May 16 14:40:05 2018 +0200 +++ b/Core/JobsEngine/SetOfInstancesJob.cpp Wed May 16 16:23:20 2018 +0200 @@ -113,7 +113,7 @@ } - JobStepResult* SetOfInstancesJob::ExecuteStep() + JobStepResult SetOfInstancesJob::ExecuteStep() { if (!started_) { @@ -125,13 +125,13 @@ { // No instance to handle, we're done position_ = 1; - return new JobStepResult(JobStepCode_Success); + return JobStepResult::Success(); } if (position_ >= instances_.size()) { // Already done - return new JobStepResult(JobStepCode_Failure); + throw OrthancException(ErrorCode_BadSequenceOfCalls); } const std::string currentInstance = instances_[position_]; @@ -144,7 +144,7 @@ if (!ok && !permissive_) { - throw OrthancException(ErrorCode_InternalError); + return JobStepResult::Failure(ErrorCode_InternalError); } } catch (OrthancException& e) @@ -169,11 +169,11 @@ if (position_ == instances_.size()) { // We're done - return new JobStepResult(JobStepCode_Success); + return JobStepResult::Success(); } else { - return new JobStepResult(JobStepCode_Continue); + return JobStepResult::Continue(); } } diff -r ef7ba1b21d58 -r 34dc57f4a7d2 Core/JobsEngine/SetOfInstancesJob.h --- a/Core/JobsEngine/SetOfInstancesJob.h Wed May 16 14:40:05 2018 +0200 +++ b/Core/JobsEngine/SetOfInstancesJob.h Wed May 16 16:23:20 2018 +0200 @@ -94,7 +94,7 @@ return failedInstances_; } - virtual JobStepResult* ExecuteStep(); + virtual JobStepResult ExecuteStep(); virtual void GetInternalContent(Json::Value& value); }; diff -r ef7ba1b21d58 -r 34dc57f4a7d2 Resources/CMake/OrthancFrameworkConfiguration.cmake --- a/Resources/CMake/OrthancFrameworkConfiguration.cmake Wed May 16 14:40:05 2018 +0200 +++ b/Resources/CMake/OrthancFrameworkConfiguration.cmake Wed May 16 16:23:20 2018 +0200 @@ -500,6 +500,7 @@ ${ORTHANC_ROOT}/Core/FileStorage/FilesystemStorage.cpp ${ORTHANC_ROOT}/Core/JobsEngine/JobInfo.cpp ${ORTHANC_ROOT}/Core/JobsEngine/JobStatus.cpp + ${ORTHANC_ROOT}/Core/JobsEngine/JobStepResult.cpp ${ORTHANC_ROOT}/Core/JobsEngine/JobsEngine.cpp ${ORTHANC_ROOT}/Core/JobsEngine/JobsRegistry.cpp ${ORTHANC_ROOT}/Core/JobsEngine/SetOfInstancesJob.cpp diff -r ef7ba1b21d58 -r 34dc57f4a7d2 UnitTestsSources/MultiThreadingTests.cpp --- a/UnitTestsSources/MultiThreadingTests.cpp Wed May 16 14:40:05 2018 +0200 +++ b/UnitTestsSources/MultiThreadingTests.cpp Wed May 16 16:23:20 2018 +0200 @@ -34,7 +34,6 @@ #include "PrecompiledHeadersUnitTests.h" #include "gtest/gtest.h" -#include "../Core/JobsEngine/JobStepRetry.h" #include "../Core/JobsEngine/JobsEngine.h" #include "../Core/MultiThreading/Locker.h" #include "../Core/OrthancException.h" @@ -244,20 +243,20 @@ class DummyJob : public Orthanc::IJob { private: - JobStepResult result_; + bool fails_; unsigned int count_; unsigned int steps_; public: DummyJob() : - result_(Orthanc::JobStepCode_Success), + fails_(false), count_(0), steps_(4) { } - explicit DummyJob(JobStepResult result) : - result_(result), + explicit DummyJob(bool fails) : + fails_(fails), count_(0), steps_(4) { @@ -271,18 +270,22 @@ { } - virtual JobStepResult* ExecuteStep() + virtual JobStepResult ExecuteStep() { boost::this_thread::sleep(boost::posix_time::milliseconds(10)); - if (count_ == steps_ - 1) + if (fails_) { - return new JobStepResult(result_); + return JobStepResult::Failure(ErrorCode_ParameterOutOfRange); + } + else if (count_ == steps_ - 1) + { + return JobStepResult::Success(); } else { count_++; - return new JobStepResult(JobStepCode_Continue); + return JobStepResult::Continue(); } } @@ -767,7 +770,8 @@ if (1) { - printf(">> %d\n", engine.GetRegistry().SubmitAndWait(new DummyJob(JobStepResult(Orthanc::JobStepCode_Failure)), rand() % 10)); + ASSERT_TRUE(engine.GetRegistry().SubmitAndWait(new DummyJob(), rand() % 10)); + ASSERT_FALSE(engine.GetRegistry().SubmitAndWait(new DummyJob(true), rand() % 10)); } boost::this_thread::sleep(boost::posix_time::milliseconds(100));