From 48f6ad70d8df397d8511f70b61c898766f40e3a4 Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Mon, 26 Jun 2023 19:50:18 +0530 Subject: [PATCH 1/7] RFC3339 for Divisions V5 API --- lib/go-tc/divisions.go | 30 +++ .../dbhelpers/db_helpers.go | 15 ++ .../traffic_ops_golang/division/divisions.go | 211 ++++++++++++++++++ .../traffic_ops_golang/routing/routes.go | 8 +- 4 files changed, 260 insertions(+), 4 deletions(-) diff --git a/lib/go-tc/divisions.go b/lib/go-tc/divisions.go index c4d8080707..003c6ef46b 100644 --- a/lib/go-tc/divisions.go +++ b/lib/go-tc/divisions.go @@ -1,5 +1,7 @@ package tc +import "time" + /* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -60,3 +62,31 @@ type DivisionNullable struct { LastUpdated *TimeNoMod `json:"lastUpdated" db:"last_updated"` Name *string `json:"name" db:"name"` } + +// DivisionsResponseV5 is an alias for the latest minor version for the major version 5. +type DivisionsResponseV5 DivisionsResponseV50 + +// DivisionResponseV5 is an alias for the latest minor version for the major version 5. +type DivisionResponseV5 DivisionResponseV50 + +// DivisionV5 is an alias for the latest minor version for the major version 5. +type DivisionV5 DivisionV50 + +// DivisionsResponseV50 is a list of Divisions as a response. +type DivisionsResponseV50 struct { + Response []DivisionV5 `json:"response"` + Alerts +} + +// DivisionResponseV50 is a single Division response for Update and Create to +// depict what changed. +type DivisionResponseV50 struct { + Response DivisionV5 `json:"response"` +} + +// A DivisionV50 is a named collection of Regions. +type DivisionV50 struct { + ID int `json:"id" db:"id"` + LastUpdated time.Time `json:"lastUpdated" db:"last_updated"` + Name string `json:"name" db:"name"` +} diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go index e1e7d32bd8..dde6815757 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go @@ -2217,3 +2217,18 @@ func ASNExists(tx *sql.Tx, id string) (bool, error) { } return true, nil } + +// DivisionExists confirms whether the division exists, and an error (if one occurs). +func DivisionExists(tx *sql.Tx, id string) (bool, error) { + var count int + if err := tx.QueryRow("SELECT count(id) FROM division AS div WHERE div.id=$1", id).Scan(&count); err != nil { + return false, fmt.Errorf("error getting divisions info: %w", err) + } + if count == 0 { + return false, nil + } + if count != 1 { + return false, fmt.Errorf("getting division info - expected row count: 1, actual: %d", count) + } + return true, nil +} diff --git a/traffic_ops/traffic_ops_golang/division/divisions.go b/traffic_ops/traffic_ops_golang/division/divisions.go index 3022203242..2a66a7c3bd 100644 --- a/traffic_ops/traffic_ops_golang/division/divisions.go +++ b/traffic_ops/traffic_ops_golang/division/divisions.go @@ -20,11 +20,16 @@ package division */ import ( + "database/sql" + "encoding/json" + "errors" + "fmt" "net/http" "strconv" "strings" "time" + "github.com/apache/trafficcontrol/lib/go-log" "github.com/apache/trafficcontrol/lib/go-tc" "github.com/apache/trafficcontrol/lib/go-tc/tovalidate" "github.com/apache/trafficcontrol/lib/go-util" @@ -137,3 +142,209 @@ WHERE id=:id RETURNING last_updated` func deleteQuery() string { return `DELETE FROM division WHERE id=:id` } + +func GetDivisions(w http.ResponseWriter, r *http.Request) { + var div tc.DivisionV5 + inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) + tx := inf.Tx + if userErr != nil || sysErr != nil { + api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr) + return + } + defer inf.Close() + + // Query Parameters to Database Query column mappings + queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{ + "name": {Column: "d.name", Checker: nil}, + } + if _, ok := inf.Params["orderby"]; !ok { + inf.Params["orderby"] = "name" + } + where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols) + if len(errs) > 0 { + api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil) + return + } + + selectQuery := "SELECT id, name, last_updated FROM division d" + query := selectQuery + where + orderBy + pagination + rows, err := tx.NamedQuery(query, queryValues) + if err != nil { + api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("Divisions read: error getting divison(s): %w", err)) + return + } + defer log.Close(rows, "unable to close DB connection") + + var divList []tc.DivisionV5 + for rows.Next() { + if err = rows.Scan(&div.ID, &div.Name, &div.LastUpdated); err != nil { + api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting division(s): %w", err)) + return + } + divList = append(divList, div) + } + + api.WriteResp(w, r, divList) + return +} + +func readAndValidateJsonStruct(r *http.Request) (tc.DivisionV5, error) { + var div tc.DivisionV5 + if err := json.NewDecoder(r.Body).Decode(&div); err != nil { + userErr := fmt.Errorf("error decoding POST request body into DivisionV5 struct %w", err) + return div, userErr + } + + // validate JSON body + rule := validation.NewStringRule(tovalidate.IsAlphanumericUnderscoreDash, "must consist of only alphanumeric, dash, or underscore characters") + errs := tovalidate.ToErrors(validation.Errors{ + "name": validation.Validate(div.Name, validation.Required, rule), + }) + if len(errs) > 0 { + userErr := util.JoinErrs(errs) + return div, userErr + } + return div, nil +} + +func CreateDivision(w http.ResponseWriter, r *http.Request) { + inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) + if userErr != nil || sysErr != nil { + api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + return + } + defer inf.Close() + tx := inf.Tx.Tx + + div, readValErr := readAndValidateJsonStruct(r) + if readValErr != nil { + api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil) + return + } + + // check if division already exists + var count int + err := tx.QueryRow("SELECT count(*) from division where name = $1", div.Name).Scan(&count) + if err != nil { + api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("error: %w, when checking if division with name %s exists", err, div.Name)) + return + } + if count == 1 { + api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("division name '%s' already exists", div.Name), nil) + return + } + + // create division + query := `INSERT INTO division (name) VALUES ($1) RETURNING id, last_updated` + err = tx.QueryRow(query, div.Name).Scan(&div.ID, &div.LastUpdated) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + api.HandleErr(w, r, tx, http.StatusInternalServerError, fmt.Errorf("error: %w in creating division with name: %s", err, div.Name), nil) + return + } + usrErr, sysErr, code := api.ParseDBError(err) + api.HandleErr(w, r, tx, code, usrErr, sysErr) + return + } + alerts := tc.CreateAlerts(tc.SuccessLevel, "division was created.") + w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/divisons?name=%s", inf.Version.Major, inf.Version.Minor, div.Name)) + api.WriteAlertsObj(w, r, http.StatusCreated, alerts, div) + return +} + +func UpdateDivision(w http.ResponseWriter, r *http.Request) { + inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) + if userErr != nil || sysErr != nil { + api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + return + } + defer inf.Close() + + tx := inf.Tx.Tx + div, readValErr := readAndValidateJsonStruct(r) + if readValErr != nil { + api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil) + return + } + + requestedID := inf.Params["id"] + // check if the entity was already updated + userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedID, "division") + if userErr != nil || sysErr != nil { + api.HandleErr(w, r, tx, errCode, userErr, sysErr) + return + } + + //update name and description of a division + query := `UPDATE division div SET + name = $2 + WHERE div.id = $1 + RETURNING div.id, div.last_updated` + + err := tx.QueryRow(query, requestedID, div.Name).Scan(&div.ID, &div.LastUpdated) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("division with ID: %s not found", div.ID), nil) + return + } + usrErr, sysErr, code := api.ParseDBError(err) + api.HandleErr(w, r, tx, code, usrErr, sysErr) + return + } + alerts := tc.CreateAlerts(tc.SuccessLevel, "division was updated") + api.WriteAlertsObj(w, r, http.StatusOK, alerts, div) + return +} + +func DeleteDivision(w http.ResponseWriter, r *http.Request) { + inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) + tx := inf.Tx.Tx + if userErr != nil || sysErr != nil { + api.HandleErr(w, r, tx, errCode, userErr, sysErr) + return + } + defer inf.Close() + + id := inf.Params["id"] + exists, err := dbhelpers.DivisionExists(tx, id) + if err != nil { + api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) + return + } + if !exists { + if id != "" { + api.HandleErr(w, r, tx, http.StatusNotFound, fmt.Errorf("no divisions exists by id: %s", id), nil) + return + } else { + api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("no divisions exists for empty id: %s", id), nil) + return + } + } + + assignedRegions := 0 + if err := inf.Tx.Get(&assignedRegions, "SELECT count(id) FROM region reg WHERE reg.division=$1", id); err != nil { + api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("Divisions delete, counting assigned Regions: %w", err)) + return + } else if assignedRegions != 0 { + api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("can not delete a division with %d assigned region", assignedRegions), nil) + return + } + + res, err := tx.Exec("DELETE FROM division AS div WHERE div.id=$1", id) + if err != nil { + api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) + return + } + rowsAffected, err := res.RowsAffected() + if err != nil { + api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("determining rows affected for delete division: %w", err)) + return + } + if rowsAffected == 0 { + api.HandleErr(w, r, tx, http.StatusInternalServerError, fmt.Errorf("no rows deleted for division"), nil) + return + } + alerts := tc.CreateAlerts(tc.SuccessLevel, "division was deleted.") + api.WriteAlertsObj(w, r, http.StatusOK, alerts, inf.Params) + return +} diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go index 396db1766c..ba2f12d167 100644 --- a/traffic_ops/traffic_ops_golang/routing/routes.go +++ b/traffic_ops/traffic_ops_golang/routing/routes.go @@ -214,10 +214,10 @@ func Routes(d ServerData) ([]Route, http.Handler, error) { {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `dbdump/?`, Handler: dbdump.DBDump, RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"DBDUMP:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 42401664731}, //Division: CRUD - {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `divisions/?$`, Handler: api.ReadHandler(&division.TODivision{}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"DIVISION:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 408518153431}, - {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPut, Path: `divisions/{id}$`, Handler: api.UpdateHandler(&division.TODivision{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DIVISION:UPDATE", "DIVISION:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 40636914031}, - {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPost, Path: `divisions/?$`, Handler: api.CreateHandler(&division.TODivision{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DIVISION:CREATE", "DIVISION:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 45371380031}, - {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `divisions/{id}$`, Handler: api.DeleteHandler(&division.TODivision{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DIVISION:DELETE", "DIVISION:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 432538223731}, + {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `divisions/?$`, Handler: division.GetDivisions, RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"DIVISION:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 408518153431}, + {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPut, Path: `divisions/{id}$`, Handler: division.UpdateDivision, RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DIVISION:UPDATE", "DIVISION:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 40636914031}, + {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPost, Path: `divisions/?$`, Handler: division.CreateDivision, RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DIVISION:CREATE", "DIVISION:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 45371380031}, + {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `divisions/{id}$`, Handler: division.DeleteDivision, RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DIVISION:DELETE", "DIVISION:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 432538223731}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `logs/?$`, Handler: logs.Getv40, RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"LOG:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 44834055031}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `logs/newcount/?$`, Handler: logs.GetNewCount, RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"LOG:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 440583301231}, From 5fb9b2fdbaad49a8ae08bb63d13797a78cad1cb4 Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Mon, 3 Jul 2023 16:04:59 +0530 Subject: [PATCH 2/7] Implemented IfModifiedSince and IfUnModifiedSince --- .../traffic_ops_golang/division/divisions.go | 36 +++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/division/divisions.go b/traffic_ops/traffic_ops_golang/division/divisions.go index 2a66a7c3bd..48e10d4dc8 100644 --- a/traffic_ops/traffic_ops_golang/division/divisions.go +++ b/traffic_ops/traffic_ops_golang/division/divisions.go @@ -35,6 +35,7 @@ import ( "github.com/apache/trafficcontrol/lib/go-util" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims" validation "github.com/go-ozzo/ozzo-validation" ) @@ -144,7 +145,8 @@ func deleteQuery() string { } func GetDivisions(w http.ResponseWriter, r *http.Request) { - var div tc.DivisionV5 + var runSecond bool + var maxTime time.Time inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) tx := inf.Tx if userErr != nil || sysErr != nil { @@ -155,6 +157,7 @@ func GetDivisions(w http.ResponseWriter, r *http.Request) { // Query Parameters to Database Query column mappings queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{ + "id": {Column: "d.id", Checker: nil}, "name": {Column: "d.name", Checker: nil}, } if _, ok := inf.Params["orderby"]; !ok { @@ -166,6 +169,19 @@ func GetDivisions(w http.ResponseWriter, r *http.Request) { return } + if inf.Config.UseIMS { + runSecond, maxTime = ims.TryIfModifiedSinceQuery(tx, r.Header, queryValues, selectMaxLastUpdatedQuery(where)) + if !runSecond { + log.Debugln("IMS HIT") + api.AddLastModifiedHdr(w, maxTime) + w.WriteHeader(http.StatusNotModified) + return + } + log.Debugln("IMS MISS") + } else { + log.Debugln("Non IMS request") + } + selectQuery := "SELECT id, name, last_updated FROM division d" query := selectQuery + where + orderBy + pagination rows, err := tx.NamedQuery(query, queryValues) @@ -175,7 +191,8 @@ func GetDivisions(w http.ResponseWriter, r *http.Request) { } defer log.Close(rows, "unable to close DB connection") - var divList []tc.DivisionV5 + div := tc.DivisionV5{} + divList := []tc.DivisionV5{} for rows.Next() { if err = rows.Scan(&div.ID, &div.Name, &div.LastUpdated); err != nil { api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting division(s): %w", err)) @@ -268,8 +285,13 @@ func UpdateDivision(w http.ResponseWriter, r *http.Request) { } requestedID := inf.Params["id"] + + intRequestId, convErr := strconv.Atoi(requestedID) + if convErr != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, convErr, nil) + } // check if the entity was already updated - userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedID, "division") + userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, intRequestId, "division") if userErr != nil || sysErr != nil { api.HandleErr(w, r, tx, errCode, userErr, sysErr) return @@ -348,3 +370,11 @@ func DeleteDivision(w http.ResponseWriter, r *http.Request) { api.WriteAlertsObj(w, r, http.StatusOK, alerts, inf.Params) return } + +// selectMaxLastUpdatedQuery used for TryIfModifiedSinceQuery() +func selectMaxLastUpdatedQuery(where string) string { + return `SELECT max(t) from ( + SELECT max(a.last_updated) as t from division a` + where + + ` UNION ALL + select max(last_updated) as t from last_deleted l where l.table_name='division') as res` +} From 2235d152891e9ec9202750a77ebbe62f573b6f17 Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Mon, 3 Jul 2023 16:06:22 +0530 Subject: [PATCH 3/7] Updated Integration Tests for Divisions V5 APIs --- traffic_ops/testing/api/v5/divisions_test.go | 16 ++++++++-------- .../testing/api/v5/traffic_control_test.go | 2 +- traffic_ops/v5-client/division.go | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/traffic_ops/testing/api/v5/divisions_test.go b/traffic_ops/testing/api/v5/divisions_test.go index 8a1685310c..86f8f6f3ea 100644 --- a/traffic_ops/testing/api/v5/divisions_test.go +++ b/traffic_ops/testing/api/v5/divisions_test.go @@ -38,7 +38,7 @@ func TestDivisions(t *testing.T) { currentTimeRFC := currentTime.Format(time.RFC1123) tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123) - methodTests := utils.TestCase[client.Session, client.RequestOptions, tc.Division]{ + methodTests := utils.TestCase[client.Session, client.RequestOptions, tc.DivisionV5]{ "GET": { "NOT MODIFIED when NO CHANGES made": { ClientSession: TOSession, @@ -110,7 +110,7 @@ func TestDivisions(t *testing.T) { "OK when VALID request": { EndpointID: GetDivisionID(t, "cdn-div2"), ClientSession: TOSession, - RequestBody: tc.Division{ + RequestBody: tc.DivisionV5{ Name: "testdivision", }, Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), @@ -120,7 +120,7 @@ func TestDivisions(t *testing.T) { EndpointID: GetDivisionID(t, "division1"), ClientSession: TOSession, RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}}}, - RequestBody: tc.Division{ + RequestBody: tc.DivisionV5{ Name: "division1", }, Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)), @@ -128,7 +128,7 @@ func TestDivisions(t *testing.T) { "PRECONDITION FAILED when updating with IFMATCH ETAG Header": { EndpointID: GetDivisionID(t, "division1"), ClientSession: TOSession, - RequestBody: tc.Division{ + RequestBody: tc.DivisionV5{ Name: "division1", }, RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}}}, @@ -191,7 +191,7 @@ func TestDivisions(t *testing.T) { func validateDivisionFields(expectedResp map[string]interface{}) utils.CkReqFunc { return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) { assert.RequireNotNil(t, resp, "Expected Division response to not be nil.") - divisionResp := resp.([]tc.Division) + divisionResp := resp.([]tc.DivisionV5) for field, expected := range expectedResp { for _, division := range divisionResp { switch field { @@ -218,7 +218,7 @@ func validateDivisionUpdateCreateFields(name string, expectedResp map[string]int func validateDivisionPagination(paginationParam string) utils.CkReqFunc { return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) { - paginationResp := resp.([]tc.Division) + paginationResp := resp.([]tc.DivisionV5) opts := client.NewRequestOptions() opts.QueryParameters.Set("orderby", "id") respBase, _, err := TOSession.GetDivisions(opts) @@ -241,7 +241,7 @@ func validateDivisionSort() utils.CkReqFunc { return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) { assert.RequireNotNil(t, resp, "Expected Division response to not be nil.") var divisionNames []string - divisionResp := resp.([]tc.Division) + divisionResp := resp.([]tc.DivisionV5) for _, division := range divisionResp { divisionNames = append(divisionNames, division.Name) } @@ -252,7 +252,7 @@ func validateDivisionSort() utils.CkReqFunc { func validateDivisionDescSort() utils.CkReqFunc { return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) { assert.RequireNotNil(t, resp, "Expected Division response to not be nil.") - divisionDescResp := resp.([]tc.Division) + divisionDescResp := resp.([]tc.DivisionV5) var descSortedList []string var ascSortedList []string assert.RequireGreaterOrEqual(t, len(divisionDescResp), 2, "Need at least 2 Divisions in Traffic Ops to test desc sort, found: %d", len(divisionDescResp)) diff --git a/traffic_ops/testing/api/v5/traffic_control_test.go b/traffic_ops/testing/api/v5/traffic_control_test.go index 4279332f36..789e475021 100644 --- a/traffic_ops/testing/api/v5/traffic_control_test.go +++ b/traffic_ops/testing/api/v5/traffic_control_test.go @@ -34,7 +34,7 @@ type TrafficControl struct { DeliveryServicesRequiredCapabilities []tc.DeliveryServicesRequiredCapability `json:"deliveryservicesRequiredCapabilities"` DeliveryServiceServerAssignments []tc.DeliveryServiceServers `json:"deliveryServiceServerAssignments"` TopologyBasedDeliveryServicesRequiredCapabilities []tc.DeliveryServicesRequiredCapability `json:"topologyBasedDeliveryServicesRequiredCapabilities"` - Divisions []tc.Division `json:"divisions"` + Divisions []tc.DivisionV5 `json:"divisions"` Federations []tc.CDNFederation `json:"federations"` FederationResolvers []tc.FederationResolver `json:"federation_resolvers"` Jobs []tc.InvalidationJobCreateV4 `json:"jobs"` diff --git a/traffic_ops/v5-client/division.go b/traffic_ops/v5-client/division.go index 880fe3baa1..d0f4f9666b 100644 --- a/traffic_ops/v5-client/division.go +++ b/traffic_ops/v5-client/division.go @@ -26,7 +26,7 @@ import ( const apiDivisions = "/divisions" // CreateDivision creates the given Division. -func (to *Session) CreateDivision(division tc.Division, opts RequestOptions) (tc.Alerts, toclientlib.ReqInf, error) { +func (to *Session) CreateDivision(division tc.DivisionV5, opts RequestOptions) (tc.Alerts, toclientlib.ReqInf, error) { var alerts tc.Alerts reqInf, err := to.post(apiDivisions, opts, division, &alerts) return alerts, reqInf, err @@ -34,7 +34,7 @@ func (to *Session) CreateDivision(division tc.Division, opts RequestOptions) (tc // UpdateDivision replaces the Division identified by 'id' with the one // provided. -func (to *Session) UpdateDivision(id int, division tc.Division, opts RequestOptions) (tc.Alerts, toclientlib.ReqInf, error) { +func (to *Session) UpdateDivision(id int, division tc.DivisionV5, opts RequestOptions) (tc.Alerts, toclientlib.ReqInf, error) { route := fmt.Sprintf("%s/%d", apiDivisions, id) var alerts tc.Alerts reqInf, err := to.put(route, opts, division, &alerts) @@ -42,8 +42,8 @@ func (to *Session) UpdateDivision(id int, division tc.Division, opts RequestOpti } // GetDivisions returns Divisions from Traffic Ops. -func (to *Session) GetDivisions(opts RequestOptions) (tc.DivisionsResponse, toclientlib.ReqInf, error) { - var data tc.DivisionsResponse +func (to *Session) GetDivisions(opts RequestOptions) (tc.DivisionsResponseV5, toclientlib.ReqInf, error) { + var data tc.DivisionsResponseV5 reqInf, err := to.get(apiDivisions, opts, &data) return data, reqInf, err } From 153555c2226e7ed77e4b2af0924d339864948b76 Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Mon, 3 Jul 2023 16:34:45 +0530 Subject: [PATCH 4/7] Updated Docs for divisions v5 rfc3339 changes. --- docs/source/api/v5/divisions.rst | 19 ++++++++++++++----- docs/source/api/v5/divisions_id.rst | 25 +++++++++++++------------ 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/docs/source/api/v5/divisions.rst b/docs/source/api/v5/divisions.rst index ba6edf6188..c542bfc802 100644 --- a/docs/source/api/v5/divisions.rst +++ b/docs/source/api/v5/divisions.rst @@ -53,10 +53,19 @@ Request Structure | | defined to make use of ``page``. | +-----------+---------------------------------------------------------------------------------------------------------------+ +.. code-block:: http + :caption: Request Example + + GET /api/5.0/divisions HTTP/1.1 + Host: trafficops.infra.ciab.test + User-Agent: curl/7.47.0 + Accept: */* + Cookie: mojolicious=... + Response Structure ------------------ :id: An integral, unique identifier for this Division -:lastUpdated: The date and time at which this Division was last modified, in :ref:`non-rfc-datetime` +:lastUpdated: The date and time at which this Division was last modified, in :rfc:`3339` :name: The Division name .. code-block:: http @@ -77,12 +86,12 @@ Response Structure { "response": [ { "id": 1, - "lastUpdated": "2018-11-29 18:38:28+00", + "lastUpdated": "2018-11-29T09:39:09.761097+05:30", "name": "Quebec" }, { "id": 2, - "lastUpdated": "2018-11-29 18:38:28+00", + "lastUpdated": "2018-11-29T15:29:31.872822+05:30", "name": "USA" } ]} @@ -117,7 +126,7 @@ Request Structure Response Structure ------------------ :id: An integral, unique identifier for this Division -:lastUpdated: The date and time at which this Division was last modified, in :ref:`non-rfc-datetime` +:lastUpdated: The date and time at which this Division was last modified, in :rfc:`3339` :name: The Division name .. code-block:: http @@ -143,6 +152,6 @@ Response Structure ], "response": { "id": 3, - "lastUpdated": "2018-11-29 19:52:06+00", + "lastUpdated": "2018-11-29T19:52:06.872822+05:30", "name": "test" }} diff --git a/docs/source/api/v5/divisions_id.rst b/docs/source/api/v5/divisions_id.rst index b6e18cc697..e95f019091 100644 --- a/docs/source/api/v5/divisions_id.rst +++ b/docs/source/api/v5/divisions_id.rst @@ -57,7 +57,7 @@ Request Structure Response Structure ------------------ :id: An integral, unique identifier for this Division -:lastUpdated: The date and time at which this Division was last modified, in :ref:`non-rfc-datetime` +:lastUpdated: The date and time at which this Division was last modified, in :rfc:`3339` :name: The Division name .. code-block:: http @@ -83,7 +83,7 @@ Response Structure ], "response": { "id": 3, - "lastUpdated": "2018-11-29 20:10:36+00", + "lastUpdated": "2018-11-29T16:23:53.696397+05:30", "name": "quest" }} @@ -118,13 +118,9 @@ Request Structure Content-Length: 2 Content-Type: application/json - {} Response Structure ------------------ -:id: An integral, unique identifier for this Division -:lastUpdated: The date and time at which this Division was last modified, in :ref:`non-rfc-datetime` -:name: The Division name .. code-block:: http :caption: Response Example @@ -141,9 +137,14 @@ Response Structure Date: Thu, 29 Nov 2018 20:10:36 GMT Content-Length: 83 - { "alerts": [ - { - "text": "division was deleted.", - "level": "success" - } - ]} + { + "alerts": [ + { + "text": "division was deleted.", + "level": "success" + } + ], + "response": { + "id": "3" + } + } From c4d017c0615ebe569ead0552bfa01d735fe3ac0f Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Mon, 3 Jul 2023 16:56:36 +0530 Subject: [PATCH 5/7] Added Unit test for DivisionExists func --- lib/go-tc/divisions.go | 4 +- .../dbhelpers/db_helpers_test.go | 51 +++++++++++++++++++ .../traffic_ops_golang/division/divisions.go | 40 +++++++-------- 3 files changed, 73 insertions(+), 22 deletions(-) diff --git a/lib/go-tc/divisions.go b/lib/go-tc/divisions.go index 003c6ef46b..484e58bd3e 100644 --- a/lib/go-tc/divisions.go +++ b/lib/go-tc/divisions.go @@ -1,7 +1,5 @@ package tc -import "time" - /* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -21,6 +19,8 @@ import "time" * under the License. */ +import "time" + // DivisionsResponse is a list of Divisions as a response. // swagger:response DivisionsResponse type DivisionsResponse struct { diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go index 8007f1f221..0c9bda1586 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go @@ -562,3 +562,54 @@ func TestASNExists(t *testing.T) { }) } } + +func TestDivisionExists(t *testing.T) { + var testCases = []struct { + description string + id string + expectedError error + exists bool + }{ + { + description: "Success: Get valid Division", + id: "1", + expectedError: nil, + exists: true, + }, + { + description: "Failure: Division not in DB", + id: "10", + expectedError: sql.ErrNoRows, + exists: false, + }, + } + for _, testCase := range testCases { + t.Run(testCase.description, func(t *testing.T) { + mockDB, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) + } + defer mockDB.Close() + + db := sqlx.NewDb(mockDB, "sqlmock") + defer db.Close() + + mock.ExpectBegin() + rows := sqlmock.NewRows([]string{"count"}) + if testCase.exists { + rows = rows.AddRow(1) + } + mock.ExpectQuery("SELECT").WillReturnRows(rows) + mock.ExpectCommit() + + divisionExists, err := DivisionExists(db.MustBegin().Tx, testCase.id) + if testCase.exists != divisionExists { + t.Errorf("Expected return exists: %t, actual %t", testCase.exists, divisionExists) + } + + if !errors.Is(err, testCase.expectedError) { + t.Errorf("DivisionExists Error. expected: %s, actual: %s", testCase.expectedError, err) + } + }) + } +} diff --git a/traffic_ops/traffic_ops_golang/division/divisions.go b/traffic_ops/traffic_ops_golang/division/divisions.go index 48e10d4dc8..c939219ef4 100644 --- a/traffic_ops/traffic_ops_golang/division/divisions.go +++ b/traffic_ops/traffic_ops_golang/division/divisions.go @@ -205,25 +205,6 @@ func GetDivisions(w http.ResponseWriter, r *http.Request) { return } -func readAndValidateJsonStruct(r *http.Request) (tc.DivisionV5, error) { - var div tc.DivisionV5 - if err := json.NewDecoder(r.Body).Decode(&div); err != nil { - userErr := fmt.Errorf("error decoding POST request body into DivisionV5 struct %w", err) - return div, userErr - } - - // validate JSON body - rule := validation.NewStringRule(tovalidate.IsAlphanumericUnderscoreDash, "must consist of only alphanumeric, dash, or underscore characters") - errs := tovalidate.ToErrors(validation.Errors{ - "name": validation.Validate(div.Name, validation.Required, rule), - }) - if len(errs) > 0 { - userErr := util.JoinErrs(errs) - return div, userErr - } - return div, nil -} - func CreateDivision(w http.ResponseWriter, r *http.Request) { inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil) if userErr != nil || sysErr != nil { @@ -306,7 +287,7 @@ func UpdateDivision(w http.ResponseWriter, r *http.Request) { err := tx.QueryRow(query, requestedID, div.Name).Scan(&div.ID, &div.LastUpdated) if err != nil { if errors.Is(err, sql.ErrNoRows) { - api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("division with ID: %s not found", div.ID), nil) + api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("division with ID: %v not found", div.ID), nil) return } usrErr, sysErr, code := api.ParseDBError(err) @@ -378,3 +359,22 @@ func selectMaxLastUpdatedQuery(where string) string { ` UNION ALL select max(last_updated) as t from last_deleted l where l.table_name='division') as res` } + +func readAndValidateJsonStruct(r *http.Request) (tc.DivisionV5, error) { + var div tc.DivisionV5 + if err := json.NewDecoder(r.Body).Decode(&div); err != nil { + userErr := fmt.Errorf("error decoding POST request body into DivisionV5 struct %w", err) + return div, userErr + } + + // validate JSON body + rule := validation.NewStringRule(tovalidate.IsAlphanumericUnderscoreDash, "must consist of only alphanumeric, dash, or underscore characters") + errs := tovalidate.ToErrors(validation.Errors{ + "name": validation.Validate(div.Name, validation.Required, rule), + }) + if len(errs) > 0 { + userErr := util.JoinErrs(errs) + return div, userErr + } + return div, nil +} From 5dd8470644e2649db3458b6c4a4cb715df750aaf Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Mon, 3 Jul 2023 17:01:42 +0530 Subject: [PATCH 6/7] Updated CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c44cfab32..95ff1ef3d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### Fixed - [#7608](https://github.com/apache/trafficcontrol/pull/7608) *Traffic Monitor* Use stats_over_http(plugin.system_stats.timestamp_ms) timestamp field to calculate bandwidth for TM's caches. +- [#7612](https://github.com/apache/trafficcontrol/pull/7612) *Traffic Ops* Fixes Divisions V5 apis to respond with RFC3339 date/time Format - [#6318](https://github.com/apache/trafficcontrol/issues/6318) *Docs* Included docs for POST, PUT, DELETE (v3,v4,v5) for statuses and statuses{id} - [#7561](https://github.com/apache/trafficcontrol/pull/7561) *Traffic Ops* *Traffic Ops* Fixed `ASN` V5 apis to respond with `RFC3339` date/time Format. - [#7598](https://github.com/apache/trafficcontrol/pull/7598) *Traffic Ops* Fixes Server Capability V5 Type Name Minor version From de81ab9cfb84843794856599587add52d7d52249 Mon Sep 17 00:00:00 2001 From: Jagan Parthiban Date: Wed, 5 Jul 2023 22:26:41 +0530 Subject: [PATCH 7/7] Updated Error Message --- traffic_ops/traffic_ops_golang/division/divisions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/traffic_ops/traffic_ops_golang/division/divisions.go b/traffic_ops/traffic_ops_golang/division/divisions.go index c939219ef4..406773ad09 100644 --- a/traffic_ops/traffic_ops_golang/division/divisions.go +++ b/traffic_ops/traffic_ops_golang/division/divisions.go @@ -269,7 +269,7 @@ func UpdateDivision(w http.ResponseWriter, r *http.Request) { intRequestId, convErr := strconv.Atoi(requestedID) if convErr != nil { - api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, convErr, nil) + api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, fmt.Errorf("division update error: %w, while converting from string to int", convErr), nil) } // check if the entity was already updated userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, intRequestId, "division")