diff --git a/app/api/controller/check/check_report.go b/app/api/controller/check/check_report.go index 8a91420a6..2fc1b1be5 100644 --- a/app/api/controller/check/check_report.go +++ b/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 +} diff --git a/app/api/controller/check/check_report_test.go b/app/api/controller/check/check_report_test.go new file mode 100644 index 000000000..d1436652e --- /dev/null +++ b/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) + } + }) + } +} diff --git a/app/store/database.go b/app/store/database.go index 60fb5f987..43543cd28 100644 --- a/app/store/database.go +++ b/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 diff --git a/app/store/database/check.go b/app/store/database/check.go index cabc48e0c..9e2f9090e 100644 --- a/app/store/database/check.go +++ b/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, } } diff --git a/app/store/database/migrate/postgres/0038_alter_checks_add_started_ended.down.sql b/app/store/database/migrate/postgres/0038_alter_checks_add_started_ended.down.sql new file mode 100644 index 000000000..f791b9901 --- /dev/null +++ b/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; diff --git a/app/store/database/migrate/postgres/0038_alter_checks_add_started_ended.up.sql b/app/store/database/migrate/postgres/0038_alter_checks_add_started_ended.up.sql new file mode 100644 index 000000000..70b2065f6 --- /dev/null +++ b/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; diff --git a/app/store/database/migrate/sqlite/0038_alter_checks_add_started_ended.down.sql b/app/store/database/migrate/sqlite/0038_alter_checks_add_started_ended.down.sql new file mode 100644 index 000000000..f791b9901 --- /dev/null +++ b/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; diff --git a/app/store/database/migrate/sqlite/0038_alter_checks_add_started_ended.up.sql b/app/store/database/migrate/sqlite/0038_alter_checks_add_started_ended.up.sql new file mode 100644 index 000000000..70b2065f6 --- /dev/null +++ b/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; diff --git a/types/check.go b/types/check.go index dd0f45877..783c1734d 100644 --- a/types/check.go +++ b/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"` diff --git a/types/enum/check.go b/types/enum/check.go index e59755dca..e6d396115 100644 --- a/types/enum/check.go +++ b/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) +}