diff --git a/selfprivacy_api/jobs/__init__.py b/selfprivacy_api/jobs/__init__.py index 9e4d961..1547b84 100644 --- a/selfprivacy_api/jobs/__init__.py +++ b/selfprivacy_api/jobs/__init__.py @@ -67,10 +67,9 @@ class Jobs: """ Reset the jobs list. """ - r = RedisPool().get_connection() jobs = Jobs.get_jobs() for job in jobs: - r.delete(redis_key_from_uuid(job.uid)) + Jobs.remove(job) @staticmethod def add( @@ -99,7 +98,7 @@ class Jobs: result=None, ) r = RedisPool().get_connection() - store_job_as_hash(r, redis_key_from_uuid(job.uid), job) + _store_job_as_hash(r, _redis_key_from_uuid(job.uid), job) return job @staticmethod @@ -115,8 +114,10 @@ class Jobs: Remove a job from the jobs list. """ r = RedisPool().get_connection() - key = redis_key_from_uuid(job_uuid) - r.delete(key) + key = _redis_key_from_uuid(job_uuid) + if (r.exists(key)): + r.delete(key) + return True return False @staticmethod @@ -149,9 +150,9 @@ class Jobs: job.finished_at = datetime.datetime.now() r = RedisPool().get_connection() - key = redis_key_from_uuid(job.uid) + key = _redis_key_from_uuid(job.uid) if r.exists(key): - store_job_as_hash(r, key, job) + _store_job_as_hash(r, key, job) if status in (JobStatus.FINISHED, JobStatus.ERROR): r.expire(key, JOB_EXPIRATION_SECONDS) @@ -163,9 +164,9 @@ class Jobs: Get a job from the jobs list. """ r = RedisPool().get_connection() - key = redis_key_from_uuid(uid) + key = _redis_key_from_uuid(uid) if r.exists(key): - return job_from_hash(r, key) + return _job_from_hash(r, key) return None @staticmethod @@ -175,7 +176,7 @@ class Jobs: """ r = RedisPool().get_connection() jobs = r.keys("jobs:*") - return [job_from_hash(r, job_key) for job_key in jobs] + return [_job_from_hash(r, job_key) for job_key in jobs] @staticmethod def is_busy() -> bool: @@ -183,16 +184,16 @@ class Jobs: Check if there is a job running. """ for job in Jobs.get_jobs(): - if job["status"] == JobStatus.RUNNING.value: + if job.status == JobStatus.RUNNING: return True return False -def redis_key_from_uuid(uuid): +def _redis_key_from_uuid(uuid): return "jobs:" + str(uuid) -def store_job_as_hash(r, redis_key, model): +def _store_job_as_hash(r, redis_key, model): for key, value in model.dict().items(): if isinstance(value, uuid.UUID): value = str(value) @@ -203,7 +204,7 @@ def store_job_as_hash(r, redis_key, model): r.hset(redis_key, key, str(value)) -def job_from_hash(r, redis_key): +def _job_from_hash(r, redis_key): if r.exists(redis_key): job_dict = r.hgetall(redis_key) for date in [ diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 371dca4..56e4aa3 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -6,30 +6,65 @@ from selfprivacy_api.jobs import Jobs, JobStatus import selfprivacy_api.jobs as jobsmodule -def test_jobs(authorized_client, jobs_file, shared_datadir): - jobs = Jobs() - jobs.reset() - assert jobs.get_jobs() == [] +def test_add_reset(jobs_with_one_job): + jobs_with_one_job.reset() + assert jobs_with_one_job.get_jobs() == [] - test_job = jobs.add( - type_id="test", - name="Test job", - description="This is a test job.", - status=JobStatus.CREATED, - status_text="Status text", - progress=0, - ) + +def test_minimal_update(jobs_with_one_job): + jobs = jobs_with_one_job + test_job = jobs_with_one_job.get_jobs()[0] + + jobs.update(job=test_job, status=JobStatus.ERROR) assert jobs.get_jobs() == [test_job] + +def test_remove_by_uid(jobs_with_one_job): + test_job = jobs_with_one_job.get_jobs()[0] + uid_str = str(test_job.uid) + + assert jobs_with_one_job.remove_by_uid(uid_str) + assert jobs_with_one_job.get_jobs() == [] + assert not jobs_with_one_job.remove_by_uid(uid_str) + + +def test_remove_update_nonexistent(jobs_with_one_job): + test_job = jobs_with_one_job.get_jobs()[0] + + jobs_with_one_job.remove(test_job) + assert jobs_with_one_job.get_jobs() == [] + + result = jobs_with_one_job.update(job=test_job, status=JobStatus.ERROR) + assert result == test_job # even though we might consider changing this behavior + + +def test_remove_get_nonexistent(jobs_with_one_job): + test_job = jobs_with_one_job.get_jobs()[0] + uid_str = str(test_job.uid) + assert jobs_with_one_job.get_job(uid_str) == test_job + + jobs_with_one_job.remove(test_job) + + assert jobs_with_one_job.get_job(uid_str) is None + + +def test_jobs(jobs_with_one_job): + jobs = jobs_with_one_job + test_job = jobs_with_one_job.get_jobs()[0] + assert not jobs.is_busy() + jobs.update( job=test_job, + name="Write Tests", + description="An oddly satisfying experience", status=JobStatus.RUNNING, status_text="Status text", progress=50, ) assert jobs.get_jobs() == [test_job] + assert jobs.is_busy() backup = jobsmodule.JOB_EXPIRATION_SECONDS jobsmodule.JOB_EXPIRATION_SECONDS = 0 @@ -46,18 +81,23 @@ def test_jobs(authorized_client, jobs_file, shared_datadir): @pytest.fixture -def mock_subprocess_run(mocker): - mock = mocker.patch("subprocess.run", autospec=True) - return mock +def jobs(): + j = Jobs() + j.reset() + assert j.get_jobs() == [] + yield j + j.reset() @pytest.fixture -def mock_shutil_move(mocker): - mock = mocker.patch("shutil.move", autospec=True) - return mock - - -@pytest.fixture -def mock_shutil_chown(mocker): - mock = mocker.patch("shutil.chown", autospec=True) - return mock +def jobs_with_one_job(jobs): + test_job = jobs.add( + type_id="test", + name="Test job", + description="This is a test job.", + status=JobStatus.CREATED, + status_text="Status text", + progress=0, + ) + assert jobs.get_jobs() == [test_job] + return jobs