Skip to content

Commit

Permalink
Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
sevein committed Jan 30, 2024
1 parent c8aa039 commit 9a77eff
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 101 deletions.
28 changes: 6 additions & 22 deletions internal/am/job_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package am
import (
context "context"
"database/sql"
"time"

"github.com/jonboulle/clockwork"
"go.artefactual.dev/amclient"
Expand Down Expand Up @@ -84,15 +83,6 @@ func (jt *JobTracker) savePreservationTasks(ctx context.Context, jobs []amclient
}

pt := ConvertJobToPreservationTask(job)
if !pt.StartedAt.Valid {
pt.StartedAt.Time = jt.clock.Now()
pt.StartedAt.Valid = true
}
if !pt.CompletedAt.Valid {
pt.CompletedAt.Time = jt.clock.Now()
pt.CompletedAt.Valid = true
}

pt.PreservationActionID = jt.presActionID

err := jt.pkgSvc.CreatePreservationTask(ctx, &pt)
Expand Down Expand Up @@ -133,34 +123,28 @@ func ConvertJobToPreservationTask(job amclient.Job) package_.PreservationTask {
}
}

// jobTimeRange calculates the overall start and end times for a job.
func jobTimeRange(job amclient.Job) (sql.NullTime, sql.NullTime) {
// Check if job has tasks.
if len(job.Tasks) == 0 {
return sql.NullTime{}, sql.NullTime{}
}
st := job.Tasks[0].StartedAt.Time
ct := job.Tasks[0].CompletedAt.Time

for _, t := range job.Tasks[1:] {
// If current st is after the task time set that as the startedAt time.
// Update st to the earliest task start time.
if st.After(t.StartedAt.Time) {
st = t.StartedAt.Time
}
// If current ct is before the task time set that as the completedAt time.
// Update ct to the latest task completion time.
if ct.Before(t.CompletedAt.Time) {
ct = t.CompletedAt.Time
}
}

var t time.Time
start := sql.NullTime{
Time: st,
Valid: st != t,
}
end := sql.NullTime{
Time: ct,
Valid: ct != t,
}
// Emit NULLs if we see the zero value.
start := sql.NullTime{Time: st, Valid: !st.IsZero()}
end := sql.NullTime{Time: ct, Valid: !ct.IsZero()}

return start, end
}
135 changes: 77 additions & 58 deletions internal/am/job_tracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"gotest.tools/v3/assert"

"github.com/artefactual-sdps/enduro/internal/am"
"github.com/artefactual-sdps/enduro/internal/package_"
fake_package "github.com/artefactual-sdps/enduro/internal/package_/fake"
)

Expand Down Expand Up @@ -176,86 +177,104 @@ func TestJobTracker(t *testing.T) {

func TestConvertJobToPreservationTask(t *testing.T) {
t.Parallel()
var og time.Time
jobs := []amclient.Job{
{
ID: "f60018ac-da79-4769-9509-c6c41d5efe7e",
LinkID: "70669a5b-01e4-4ea0-ac70-10292f87da05",
Microservice: "Verify SIP compliance",
Name: "Move to processing directory",
Status: amclient.JobStatusComplete,
Tasks: []amclient.Task{
{
ID: "c134198c-9485-4f68-8d94-4da1e03b5e1b",
ExitCode: 0,
CreatedAt: amclient.TaskDateTime{Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC)},
StartedAt: amclient.TaskDateTime{Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC)},
CompletedAt: amclient.TaskDateTime{Time: time.Date(2025, time.January, 18, 1, 27, 49, 0, time.UTC)},
Duration: amclient.TaskDuration(time.Second / 2),
},
},
},
{
ID: "c2128d39-2ace-47c5-8cac-39ded8d9c9ef",
LinkID: "208d441b-6938-44f9-b54a-bd73f05bc764",
Microservice: "Verify SIP compliance",
Name: "Verify SIP compliance",
Status: amclient.JobStatusComplete,
Tasks: []amclient.Task{
{
ID: "6f5beca3-71ad-446c-8f19-3bc4dea16c9b",
ExitCode: 0,
CreatedAt: amclient.TaskDateTime{Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC)},
StartedAt: amclient.TaskDateTime{Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC)},
CompletedAt: amclient.TaskDateTime{Time: og},
Duration: amclient.TaskDuration(time.Second / 2),
},
},
},
}

type test struct {
name string
job amclient.Job
want []sql.NullTime
want package_.PreservationTask
}

for _, tt := range []test{
{
name: "Returns correct time values for preservation task.",
job: jobs[0],
want: []sql.NullTime{
{
Time: jobs[0].Tasks[0].StartedAt.Time,
name: "Returns preservation task with computed time range",
job: amclient.Job{
ID: "f60018ac-da79-4769-9509-c6c41d5efe7e",
LinkID: "70669a5b-01e4-4ea0-ac70-10292f87da05",
Microservice: "Verify SIP compliance",
Name: "Move to processing directory",
Status: amclient.JobStatusComplete,
Tasks: []amclient.Task{
{
ID: "c134198c-9485-4f68-8d94-4da1e03b5e1b",
ExitCode: 0,
CreatedAt: amclient.TaskDateTime{Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC)},
StartedAt: amclient.TaskDateTime{Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC)},
CompletedAt: amclient.TaskDateTime{Time: time.Date(2025, time.January, 18, 1, 27, 49, 0, time.UTC)},
Duration: amclient.TaskDuration(time.Second / 2),
},
{
ID: "6e5edf16-ff93-47c0-a7d1-e623c110fa09",
ExitCode: 0,
CreatedAt: amclient.TaskDateTime{Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC)},
StartedAt: amclient.TaskDateTime{Time: time.Date(2025, time.January, 18, 1, 27, 49, 0, time.UTC)},
CompletedAt: amclient.TaskDateTime{Time: time.Date(2026, time.January, 18, 1, 27, 49, 0, time.UTC)},
Duration: amclient.TaskDuration(time.Second / 2),
},
},
},
want: package_.PreservationTask{
TaskID: "f60018ac-da79-4769-9509-c6c41d5efe7e",
Name: "Move to processing directory",
Status: package_.PreservationTaskStatus(package_.StatusDone),
StartedAt: sql.NullTime{
Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC),
Valid: true,
},
{
Time: jobs[0].Tasks[0].CompletedAt.Time,
CompletedAt: sql.NullTime{
Time: time.Date(2026, time.January, 18, 1, 27, 49, 0, time.UTC),
Valid: true,
},
},
},
{
name: "Returns zero value of time if time is not valid",
job: jobs[1],
want: []sql.NullTime{
{
Time: jobs[1].Tasks[0].StartedAt.Time,
Valid: true,
name: "Returns NULL completedAt if job is still processing",
job: amclient.Job{
ID: "c2128d39-2ace-47c5-8cac-39ded8d9c9ef",
LinkID: "208d441b-6938-44f9-b54a-bd73f05bc764",
Microservice: "Verify SIP compliance",
Name: "Verify SIP compliance",
Status: amclient.JobStatusProcessing,
Tasks: []amclient.Task{
{
ID: "6f5beca3-71ad-446c-8f19-3bc4dea16c9b",
ExitCode: 0,
CreatedAt: amclient.TaskDateTime{Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC)},
StartedAt: amclient.TaskDateTime{Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC)},
CompletedAt: amclient.TaskDateTime{Time: time.Time{}},
Duration: amclient.TaskDuration(time.Second / 2),
},
{
ID: "6f5beca3-71ad-446c-8f19-3bc4dea16c9b",
ExitCode: 0,
CreatedAt: amclient.TaskDateTime{Time: time.Date(2025, time.January, 18, 1, 27, 49, 0, time.UTC)},
StartedAt: amclient.TaskDateTime{Time: time.Date(2025, time.January, 18, 1, 27, 49, 0, time.UTC)},
CompletedAt: amclient.TaskDateTime{Time: time.Time{}},
Duration: amclient.TaskDuration(time.Second / 2),
},
},
{
Time: jobs[1].Tasks[0].CompletedAt.Time,
Valid: false,
},
want: package_.PreservationTask{
TaskID: "c2128d39-2ace-47c5-8cac-39ded8d9c9ef",
Name: "Verify SIP compliance",
Status: package_.PreservationTaskStatus(package_.StatusInProgress),
StartedAt: sql.NullTime{
Time: time.Date(2024, time.January, 18, 1, 27, 49, 0, time.UTC),
Valid: true,
},
CompletedAt: sql.NullTime{},
},
},
{
name: "Returns NULL timestamps in the job has no tasks",
job: amclient.Job{},
want: package_.PreservationTask{},
},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
presTask := am.ConvertJobToPreservationTask(tt.job)
got := []sql.NullTime{
presTask.StartedAt, presTask.CompletedAt,
}

got := am.ConvertJobToPreservationTask(tt.job)

assert.DeepEqual(t, got, tt.want)
})
Expand Down
8 changes: 0 additions & 8 deletions internal/am/poll_ingest_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package am_test

import (
"database/sql"
"net/http"
"testing"
"time"
Expand All @@ -25,7 +24,6 @@ import (

func TestPollIngestActivity(t *testing.T) {
clock := clockwork.NewFakeClock()
nullTime := sql.NullTime{Time: clock.Now(), Valid: true}
path := "/var/archivematica/fake/sip"
presActionID := uint(2)
sipID := uuid.New().String()
Expand Down Expand Up @@ -160,12 +158,6 @@ func TestPollIngestActivity(t *testing.T) {
for i, job := range jobs {
pt := am.ConvertJobToPreservationTask(job)
pt.PreservationActionID = presActionID
if !pt.CompletedAt.Valid {
pt.CompletedAt = nullTime
}
if !pt.StartedAt.Valid {
pt.StartedAt = nullTime
}

tasks[i] = &pt
}
Expand Down
14 changes: 1 addition & 13 deletions internal/am/poll_transfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ func TestPollTransferActivity(t *testing.T) {
presActionID := uint(1)
sipID := uuid.New().String()
path := "/var/archivematica/fake/sip"
ttime, err := time.Parse(time.RFC3339, "2023-12-05T12:02:00Z")
if err != nil {
t.Fatal("Invalid test time")
}

jobs := []amclient.Job{
{
Expand Down Expand Up @@ -158,14 +154,6 @@ func TestPollTransferActivity(t *testing.T) {
for _, job := range jobs {
pt := am.ConvertJobToPreservationTask(job)
pt.PreservationActionID = presActionID
if !pt.CompletedAt.Valid {
pt.CompletedAt.Time = ttime
pt.CompletedAt.Valid = true
}
if !pt.StartedAt.Valid {
pt.StartedAt.Time = ttime
pt.StartedAt.Valid = true
}
m.CreatePreservationTask(mockutil.Context(), &pt).Return(nil)

}
Expand Down Expand Up @@ -310,7 +298,7 @@ func TestPollTransferActivity(t *testing.T) {
am.NewPollTransferActivity(
logr.Discard(),
&am.Config{PollInterval: time.Millisecond * 10},
clockwork.NewFakeClockAt(ttime),
clockwork.NewFakeClock(),
trfSvc,
jobSvc,
pkgSvc,
Expand Down

0 comments on commit 9a77eff

Please sign in to comment.