From 1c1071353fc92b6de1f8f41e056c2290c1c751a8 Mon Sep 17 00:00:00 2001 From: Rima Shah Date: Fri, 9 Dec 2022 09:00:12 -0700 Subject: [PATCH 1/2] Fixed error message for assignment of non-existent parameters --- CHANGELOG.md | 1 + lib/go-tc/parameters.go | 45 ++++++++++++++++++++++------------------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 782a32c4e5..7b26ea322b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#4654](https://github.com/apache/trafficcontrol/pull/4654) *Traffic Ops, Traffic Portal* Switched Delivery Service active state to a three-value system, adding a state that will be used to prevent cache servers from deploying DS configuration. ### Fixed +- [#6229](https://github.com/apache/trafficcontrol/issues/6229) *Traffic Ops* Fixed error message for assignment of non-existent parameters to a profile. - [#7231](https://github.com/apache/trafficcontrol/pull/7231) *Traffic Ops, Traffic Portal* Fixed `sharedUserNames` display while retrieving CDN locks. - [#7216](https://github.com/apache/trafficcontrol/pull/7216) *Traffic Portal* Fixed sort for Server's Capabilities Table - [#4428](https://github.com/apache/trafficcontrol/issues/4428) *Traffic Ops* Fixed Internal Server Error with POST to `profileparameters` when POST body is empty diff --git a/lib/go-tc/parameters.go b/lib/go-tc/parameters.go index 95baca4fbd..774af2842e 100644 --- a/lib/go-tc/parameters.go +++ b/lib/go-tc/parameters.go @@ -1,19 +1,5 @@ package tc -import ( - "bytes" - "database/sql" - "encoding/json" - "errors" - "fmt" - "strconv" - "strings" - - "github.com/apache/trafficcontrol/lib/go-util" - - "github.com/lib/pq" -) - /* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -33,6 +19,20 @@ import ( * under the License. */ +import ( + "bytes" + "database/sql" + "encoding/json" + "errors" + "fmt" + "strconv" + "strings" + + "github.com/apache/trafficcontrol/lib/go-util" + + "github.com/lib/pq" +) + // ParametersResponse is the type of the response from Traffic Ops to GET // requests made to the /parameters and /profiles/name/{{Name}}/parameters // endpoints of its API. @@ -210,10 +210,10 @@ func (pp *PostProfileParam) Validate(tx *sql.Tx) error { } if pp.ParamIDs == nil { errs = append(errs, errors.New("paramIds missing")) - } else if ok, err := ParamsExist(*pp.ParamIDs, tx); err != nil { + } else if ok, nonExistingIDs, err := ParamsExist(*pp.ParamIDs, tx); err != nil { errs = append(errs, errors.New(fmt.Sprintf("checking parameter IDs %v existence: "+err.Error(), *pp.ParamIDs))) } else if !ok { - errs = append(errs, errors.New(fmt.Sprintf("parameters with IDs %v don't all exist", *pp.ParamIDs))) + errs = append(errs, errors.New(fmt.Sprintf("parameters with IDs %v don't all exist", nonExistingIDs))) } if len(errs) > 0 { return util.JoinErrs(errs) @@ -277,12 +277,15 @@ func ParamExists(id int64, tx *sql.Tx) (bool, error) { // ParamsExist returns whether parameters exist for all the given ids, and any error. // TODO move to helper package. -func ParamsExist(ids []int64, tx *sql.Tx) (bool, error) { - count := 0 - if err := tx.QueryRow(`SELECT count(*) from parameter where id = ANY($1)`, pq.Array(ids)).Scan(&count); err != nil { - return false, errors.New("querying parameters existence from id: " + err.Error()) +func ParamsExist(ids []int64, tx *sql.Tx) (bool, []int64, error) { + var nonExistingIDs []int64 + if err := tx.QueryRow(`SELECT ARRAY_AGG(id) FROM UNNEST($1::INT[]) AS id WHERE id NOT IN (SELECT id FROM parameter)`, pq.Array(ids)).Scan(pq.Array(&nonExistingIDs)); err != nil { + return false, nil, errors.New("querying parameters existence from id: " + err.Error()) + } + if len(nonExistingIDs) >= 1 { + return false, nonExistingIDs, nil } - return count == len(ids), nil + return true, nil, nil } // ProfileParametersNullable is an object of the form returned by the Traffic Ops /profileparameters endpoint. From b43d9216c60354ac343b15f5a372c3d31c1932ce Mon Sep 17 00:00:00 2001 From: Rima Shah Date: Wed, 14 Dec 2022 14:59:26 -0700 Subject: [PATCH 2/2] Addressed review comments --- lib/go-tc/parameters.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/go-tc/parameters.go b/lib/go-tc/parameters.go index 774af2842e..57cce3206f 100644 --- a/lib/go-tc/parameters.go +++ b/lib/go-tc/parameters.go @@ -210,10 +210,10 @@ func (pp *PostProfileParam) Validate(tx *sql.Tx) error { } if pp.ParamIDs == nil { errs = append(errs, errors.New("paramIds missing")) - } else if ok, nonExistingIDs, err := ParamsExist(*pp.ParamIDs, tx); err != nil { - errs = append(errs, errors.New(fmt.Sprintf("checking parameter IDs %v existence: "+err.Error(), *pp.ParamIDs))) - } else if !ok { - errs = append(errs, errors.New(fmt.Sprintf("parameters with IDs %v don't all exist", nonExistingIDs))) + } else if nonExistingIDs, err := ParamsExist(*pp.ParamIDs, tx); err != nil { + errs = append(errs, fmt.Errorf("checking parameter IDs %v existence: %w", *pp.ParamIDs, err)) + } else if len(nonExistingIDs) >= 1 { + errs = append(errs, fmt.Errorf("parameters with IDs %v don't exist", nonExistingIDs)) } if len(errs) > 0 { return util.JoinErrs(errs) @@ -277,15 +277,15 @@ func ParamExists(id int64, tx *sql.Tx) (bool, error) { // ParamsExist returns whether parameters exist for all the given ids, and any error. // TODO move to helper package. -func ParamsExist(ids []int64, tx *sql.Tx) (bool, []int64, error) { +func ParamsExist(ids []int64, tx *sql.Tx) ([]int64, error) { var nonExistingIDs []int64 if err := tx.QueryRow(`SELECT ARRAY_AGG(id) FROM UNNEST($1::INT[]) AS id WHERE id NOT IN (SELECT id FROM parameter)`, pq.Array(ids)).Scan(pq.Array(&nonExistingIDs)); err != nil { - return false, nil, errors.New("querying parameters existence from id: " + err.Error()) + return nil, fmt.Errorf("querying parameters existence from id: %w", err) } if len(nonExistingIDs) >= 1 { - return false, nonExistingIDs, nil + return nonExistingIDs, nil } - return true, nil, nil + return nil, nil } // ProfileParametersNullable is an object of the form returned by the Traffic Ops /profileparameters endpoint.