feat: add start and end time to check (#958)

app/api/controller/check/check_report.go

@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"regexp"
 	"time"
@@ -25,6 +26,7 @@ import (
 	"github.com/harness/gitness/app/api/usererror"
 	"github.com/harness/gitness/app/auth"
 	"github.com/harness/gitness/git"
+	"github.com/harness/gitness/store"
 	"github.com/harness/gitness/types"
 	"github.com/harness/gitness/types/enum"
 )
@@ -35,6 +37,9 @@ type ReportInput struct {
 	Summary  string             `json:"summary"`
 	Link     string             `json:"link"`
 	Payload  types.CheckPayload `json:"payload"`
+
+	Started int64 `json:"started,omitempty"`
+	Ended   int64 `json:"ended,omitempty"`
 }
 
 var regexpCheckUID = "^[0-9a-zA-Z-_.$]{1,127}$"
@@ -67,6 +72,10 @@ func (in *ReportInput) Validate(
 		return fmt.Errorf("payload validation failed: %w", err)
 	}
 
+	if in.Ended != 0 && in.Ended < in.Started {
+		return usererror.BadRequest("started time reported after ended time")
+	}
+
 	return nil
 }
 
@@ -134,6 +143,15 @@ func (c *Controller) Report(
 
 	metadataJSON, _ := json.Marshal(metadata)
 
+	existingCheck, err := c.checkStore.Find(ctx, repo.ID, commitSHA, in.CheckUID)
+
+	if err != nil && !errors.Is(err, store.ErrResourceNotFound) {
+		return nil, fmt.Errorf("failed to find existing check for UID=%q: %w", in.CheckUID, err)
+	}
+
+	started := getStartTime(in, existingCheck, now)
+	ended := getEndTime(in, now)
+
 	statusCheckReport := &types.Check{
 		CreatedBy:  session.Principal.ID,
 		Created:    now,
@@ -147,6 +165,8 @@ func (c *Controller) Report(
 		Payload:    in.Payload,
 		Metadata:   metadataJSON,
 		ReportedBy: *session.Principal.ToPrincipalInfo(),
+		Started:    started,
+		Ended:      ended,
 	}
 
 	err = c.checkStore.Upsert(ctx, statusCheckReport)
@@ -156,3 +176,51 @@ func (c *Controller) Report(
 
 	return statusCheckReport, nil
 }
+
+func getStartTime(in *ReportInput, check types.Check, now int64) int64 {
+	// start value came in api
+	if in.Started != 0 {
+		return in.Started
+	}
+	// in.started has no value we smartly put value for started
+
+	// in case of pending we assume check has not started running
+	if in.Status == enum.CheckStatusPending {
+		return 0
+	}
+
+	// new check
+	if check.Started == 0 {
+		return now
+	}
+
+	// The incoming check status can now be running or terminal.
+
+	// in case we already have running status we don't update time else we return current time as check has started
+	// running.
+	if check.Status == enum.CheckStatusRunning {
+		return check.Started
+	}
+
+	// Note: In case of reporting terminal statuses again and again we have assumed its
+	// a report of new status check everytime.
+
+	// In case someone reports any status before marking running return current time.
+	// This can happen if someone only reports terminal status or marks running status again after terminal.
+	return now
+}
+
+func getEndTime(in *ReportInput, now int64) int64 {
+	// end value came in api
+	if in.Ended != 0 {
+		return in.Ended
+	}
+
+	// if we get terminal status i.e. error, failure or success we return current time.
+	if in.Status.IsCompleted() {
+		return now
+	}
+
+	// in case of other status we return value as 0, which means we have not yet completed the check.
+	return 0
+}

app/api/controller/check/check_report_test.go

@@ -0,0 +1,215 @@
+// Copyright 2023 Harness, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package check
+
+import (
+	"testing"
+
+	"github.com/harness/gitness/types"
+	"github.com/harness/gitness/types/enum"
+)
+
+func Test_getStartedTime(t *testing.T) {
+	type args struct {
+		in    *ReportInput
+		check types.Check
+		now   int64
+	}
+	tests := []struct {
+		name string
+		args args
+		want int64
+	}{
+		{
+			name: "nothing to pending",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusPending},
+				check: types.Check{},
+				now:   1234,
+			},
+			want: 0,
+		},
+
+		{
+			name: "nothing to running",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusRunning},
+				check: types.Check{},
+				now:   1234,
+			},
+			want: 1234,
+		},
+		{
+			name: "nothing to completed",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusSuccess},
+				check: types.Check{},
+				now:   1234,
+			},
+			want: 1234,
+		},
+		{
+			name: "nothing to completed1",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusSuccess, Started: 1},
+				check: types.Check{},
+				now:   1234,
+			},
+			want: 1,
+		},
+		{
+			name: "pending to pending",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusPending, Started: 1},
+				check: types.Check{Status: enum.CheckStatusPending, Started: 0},
+				now:   1234,
+			},
+			want: 1,
+		},
+		{
+			name: "pending to pending1",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusPending},
+				check: types.Check{Status: enum.CheckStatusPending, Started: 0},
+				now:   1234,
+			},
+			want: 0,
+		},
+		{
+			name: "pending to running",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusRunning},
+				check: types.Check{Status: enum.CheckStatusPending, Started: 0},
+				now:   1234,
+			},
+			want: 1234,
+		},
+		{
+			name: "pending to running1",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusRunning, Started: 1},
+				check: types.Check{Status: enum.CheckStatusPending, Started: 0},
+				now:   1234,
+			},
+			want: 1,
+		},
+		{
+			name: "pending to completed",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusSuccess, Started: 1},
+				check: types.Check{Status: enum.CheckStatusPending, Started: 0},
+				now:   1234,
+			},
+			want: 1,
+		},
+		{
+			name: "pending to completed1",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusSuccess},
+				check: types.Check{Status: enum.CheckStatusPending, Started: 0},
+				now:   1234,
+			},
+			want: 1234,
+		},
+		{
+			name: "running to pending",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusPending, Started: 1},
+				check: types.Check{Status: enum.CheckStatusRunning, Started: 9876},
+				now:   1234,
+			},
+			want: 1,
+		},
+		{
+			name: "running to pending1",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusPending},
+				check: types.Check{Status: enum.CheckStatusRunning, Started: 9876},
+				now:   1234,
+			},
+			want: 0,
+		},
+		{
+			name: "running to running",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusRunning},
+				check: types.Check{Status: enum.CheckStatusRunning, Started: 9876},
+				now:   1234,
+			},
+			want: 9876,
+		},
+		{
+			name: "running to running1",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusRunning, Started: 1},
+				check: types.Check{Status: enum.CheckStatusRunning, Started: 9876},
+				now:   1234,
+			},
+			want: 1,
+		},
+		{
+			name: "running to completed",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusSuccess},
+				check: types.Check{Status: enum.CheckStatusRunning, Started: 9876},
+				now:   1234,
+			},
+			want: 9876,
+		},
+		{
+			name: "running to completed",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusSuccess, Started: 1},
+				check: types.Check{Status: enum.CheckStatusRunning, Started: 9876},
+				now:   1234,
+			},
+			want: 1,
+		},
+		{
+			name: "completed to pending",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusPending},
+				check: types.Check{Status: enum.CheckStatusSuccess, Started: 9876},
+				now:   1234,
+			},
+			want: 0,
+		},
+		{
+			name: "completed to running",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusRunning},
+				check: types.Check{Status: enum.CheckStatusSuccess, Started: 9876},
+				now:   1234,
+			},
+			want: 1234,
+		},
+		{
+			name: "completed to completed",
+			args: args{
+				in:    &ReportInput{Status: enum.CheckStatusSuccess},
+				check: types.Check{Status: enum.CheckStatusSuccess, Started: 9876},
+				now:   1234,
+			},
+			want: 1234,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := getStartTime(tt.args.in, tt.args.check, tt.args.now); got != tt.want {
+				t.Errorf("getStartTime() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

app/store/database.go

@@ -473,6 +473,9 @@ type (
 	}
 
 	CheckStore interface {
+		// Find returns status check result for given unique key.
+		Find(ctx context.Context, repoID int64, commitSHA string, uid string) (types.Check, error)
+
 		// Upsert creates new or updates an existing status check result.
 		Upsert(ctx context.Context, check *types.Check) error
 

app/store/database/check.go

@@ -65,7 +65,14 @@ const (
 		,check_payload
 		,check_metadata
 		,check_payload_kind
-		,check_payload_version`
+		,check_payload_version
+		,check_started
+		,check_ended`
+
+	//nolint:goconst
+	checkSelectBase = `
+    SELECT` + checkColumns + `
+	FROM checks`
 )
 
 type check struct {
@@ -83,6 +90,23 @@ type check struct {
 	Metadata       json.RawMessage       `db:"check_metadata"`
 	PayloadKind    enum.CheckPayloadKind `db:"check_payload_kind"`
 	PayloadVersion string                `db:"check_payload_version"`
+	Started        int64                 `db:"check_started"`
+	Ended          int64                 `db:"check_ended"`
+}
+
+// Find returns status check result for given unique key.
+func (s *CheckStore) Find(ctx context.Context, repoID int64, commitSHA string, uid string) (types.Check, error) {
+	const sqlQuery = checkSelectBase + `
+		WHERE check_repo_id = $1 AND check_uid = $2 AND check_commit_sha = $3`
+
+	db := dbtx.GetAccessor(ctx, s.db)
+
+	dst := new(check)
+	if err := db.GetContext(ctx, dst, sqlQuery, repoID, uid, commitSHA); err != nil {
+		return types.Check{}, database.ProcessSQLErrorf(err, "Failed to find check")
+	}
+
+	return mapCheck(dst), nil
 }
 
 // Upsert creates new or updates an existing status check result.
@@ -102,6 +126,8 @@ func (s *CheckStore) Upsert(ctx context.Context, check *types.Check) error {
 		,check_metadata
 		,check_payload_kind
 		,check_payload_version
+		,check_started
+		,check_ended
 	) VALUES (
 		 :check_created_by
 		,:check_created
@@ -116,6 +142,8 @@ func (s *CheckStore) Upsert(ctx context.Context, check *types.Check) error {
 		,:check_metadata
 		,:check_payload_kind
 		,:check_payload_version
+		,:check_started
+		,:check_ended
 	)
 	ON CONFLICT (check_repo_id, check_commit_sha, check_uid) DO
 	UPDATE SET
@@ -127,6 +155,8 @@ func (s *CheckStore) Upsert(ctx context.Context, check *types.Check) error {
 		,check_metadata = :check_metadata
 		,check_payload_kind = :check_payload_kind
 		,check_payload_version = :check_payload_version
+	    	,check_started = :check_started
+	    	,check_ended = :check_ended
 	RETURNING check_id, check_created_by, check_created`
 
 	db := dbtx.GetAccessor(ctx, s.db)
@@ -296,6 +326,8 @@ func mapInternalCheck(c *types.Check) *check {
 		Metadata:       c.Metadata,
 		PayloadKind:    c.Payload.Kind,
 		PayloadVersion: c.Payload.Version,
+		Started:        c.Started,
+		Ended:          c.Ended,
 	}
 
 	return m
@@ -320,6 +352,8 @@ func mapCheck(c *check) types.Check {
 			Data:    c.Payload,
 		},
 		ReportedBy: types.PrincipalInfo{},
+		Started:    c.Started,
+		Ended:      c.Ended,
 	}
 }
 

app/store/database/migrate/postgres/0038_alter_checks_add_started_ended.down.sql

@@ -0,0 +1,2 @@
+ALTER TABLE checks DROP COLUMN check_started;
+ALTER TABLE checks DROP COLUMN check_ended;

app/store/database/migrate/postgres/0038_alter_checks_add_started_ended.up.sql

@@ -0,0 +1,10 @@
+ALTER TABLE checks
+    ADD COLUMN check_started INTEGER NOT NULL DEFAULT 0;
+ALTER TABLE checks
+    ADD COLUMN check_ended INTEGER NOT NULL DEFAULT 0;
+
+UPDATE checks
+SET check_started = check_created;
+
+UPDATE checks
+SET check_ended = check_updated;

app/store/database/migrate/sqlite/0038_alter_checks_add_started_ended.down.sql

@@ -0,0 +1,2 @@
+ALTER TABLE checks DROP COLUMN check_started;
+ALTER TABLE checks DROP COLUMN check_ended;

app/store/database/migrate/sqlite/0038_alter_checks_add_started_ended.up.sql

@@ -0,0 +1,10 @@
+ALTER TABLE checks
+    ADD COLUMN check_started INTEGER NOT NULL DEFAULT 0;
+ALTER TABLE checks
+    ADD COLUMN check_ended INTEGER NOT NULL DEFAULT 0;
+
+UPDATE checks
+SET check_started = check_created;
+
+UPDATE checks
+SET check_ended = check_updated;

types/check.go

@@ -32,6 +32,8 @@ type Check struct {
 	Summary   string           `json:"summary"`
 	Link      string           `json:"link"`
 	Metadata  json.RawMessage  `json:"metadata"`
+	Started   int64            `json:"started"`
+	Ended     int64            `json:"ended"`
 
 	Payload    CheckPayload  `json:"payload"`
 	ReportedBy PrincipalInfo `json:"reported_by"`

types/enum/check.go

@@ -14,6 +14,8 @@
 
 package enum
 
+import "golang.org/x/exp/slices"
+
 // CheckStatus defines status check status.
 type CheckStatus string
 
@@ -38,6 +40,8 @@ var checkStatuses = sortEnum([]CheckStatus{
 	CheckStatusError,
 })
 
+var terminalCheckStatuses = []CheckStatus{CheckStatusFailure, CheckStatusSuccess, CheckStatusError}
+
 // CheckPayloadKind defines status payload type.
 type CheckPayloadKind string
 
@@ -63,3 +67,7 @@ var checkPayloadTypes = sortEnum([]CheckPayloadKind{
 	CheckPayloadKindMarkdown,
 	CheckPayloadKindPipeline,
 })
+
+func (s CheckStatus) IsCompleted() bool {
+	return slices.Contains(terminalCheckStatuses, s)
+}