This is an automated email from the ASF dual-hosted git repository.
ocket8888 pushed a commit to branch 5.0.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/5.0.x by this push:
new 7427589 Fix service categories if-not-modified/if-match functionality
(#5231)
7427589 is described below
commit 742758951687430fe4e01eab6c4691a80e9eb844
Author: Rawlin Peters <[email protected]>
AuthorDate: Wed Nov 4 17:24:17 2020 -0700
Fix service categories if-not-modified/if-match functionality (#5231)
* Fix service categories if-not-modified/if-match functionality
This functionality was accidentally lost due to a recent change. Also,
add the missing last_deleted functionality in order for deletions to
count as modifications.
* Improve error message
(cherry picked from commit f1a6d00c11f49ca8a94f565db405f24595a5f138)
---
...20110200000000_add_service_category_deleted.sql | 28 ++++++++++++++++++++++
traffic_ops/app/db/seeds.sql | 3 ++-
.../testing/api/v3/servicecategories_test.go | 26 ++++++++++++++++++--
traffic_ops/testing/api/v3/tc-fixtures.json | 6 ++---
traffic_ops/traffic_ops_golang/api/api.go | 13 ++++++++--
.../servicecategory/servicecategories.go | 22 +++++------------
traffic_ops/v3-client/serviceCategory.go | 14 ++++++-----
7 files changed, 81 insertions(+), 31 deletions(-)
diff --git
a/traffic_ops/app/db/migrations/2020110200000000_add_service_category_deleted.sql
b/traffic_ops/app/db/migrations/2020110200000000_add_service_category_deleted.sql
new file mode 100644
index 0000000..1c1dda5
--- /dev/null
+++
b/traffic_ops/app/db/migrations/2020110200000000_add_service_category_deleted.sql
@@ -0,0 +1,28 @@
+/*
+ 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.
+*/
+
+-- +goose Up
+-- SQL in section 'Up' is executed when this migration is applied
+
+CREATE TRIGGER on_delete_current_timestamp
+AFTER DELETE
+ON service_category
+FOR EACH ROW EXECUTE PROCEDURE
on_delete_current_timestamp_last_updated('service_category');
+
+CREATE INDEX service_category_last_updated_idx ON service_category
(last_updated DESC NULLS LAST);
+
+-- +goose Down
+-- SQL section 'Down' is executed when this migration is rolled back
+
+DROP INDEX IF EXISTS service_category_last_updated_idx;
+
+DROP TRIGGER IF EXISTS on_delete_current_timestamp ON service_category;
diff --git a/traffic_ops/app/db/seeds.sql b/traffic_ops/app/db/seeds.sql
index 06aafa3..fc08c36 100644
--- a/traffic_ops/app/db/seeds.sql
+++ b/traffic_ops/app/db/seeds.sql
@@ -950,4 +950,5 @@ insert into last_deleted (table_name) VALUES ('type') ON
CONFLICT (table_name) D
insert into last_deleted (table_name) VALUES ('user_role') ON CONFLICT
(table_name) DO NOTHING;
insert into last_deleted (table_name) VALUES ('server_capability') ON CONFLICT
(table_name) DO NOTHING;
insert into last_deleted (table_name) VALUES ('server_server_capability') ON
CONFLICT (table_name) DO NOTHING;
-insert into last_deleted (table_name) VALUES
('deliveryservices_required_capability') ON CONFLICT (table_name) DO NOTHING;
\ No newline at end of file
+insert into last_deleted (table_name) VALUES ('service_category') ON CONFLICT
(table_name) DO NOTHING;
+insert into last_deleted (table_name) VALUES
('deliveryservices_required_capability') ON CONFLICT (table_name) DO NOTHING;
diff --git a/traffic_ops/testing/api/v3/servicecategories_test.go
b/traffic_ops/testing/api/v3/servicecategories_test.go
index e484a30..ff2f6e7 100644
--- a/traffic_ops/testing/api/v3/servicecategories_test.go
+++ b/traffic_ops/testing/api/v3/servicecategories_test.go
@@ -34,10 +34,16 @@ func TestServiceCategories(t *testing.T) {
var header http.Header
header = make(map[string][]string)
header.Set(rfc.IfModifiedSince, time)
+ header.Set(rfc.IfUnmodifiedSince, time)
SortTestServiceCategories(t)
UpdateTestServiceCategories(t)
GetTestServiceCategories(t)
GetTestServiceCategoriesIMSAfterChange(t, header)
+ UpdateTestServiceCategoriesWithHeaders(t, header)
+ header = make(map[string][]string)
+ etag := rfc.ETag(currentTime)
+ header.Set(rfc.IfMatch, etag)
+ UpdateTestServiceCategoriesWithHeaders(t, header)
})
}
@@ -131,6 +137,22 @@ func SortTestServiceCategories(t *testing.T) {
}
}
+func UpdateTestServiceCategoriesWithHeaders(t *testing.T, h http.Header) {
+ firstServiceCategory := tc.ServiceCategory{}
+ if len(testData.ServiceCategories) > 0 {
+ firstServiceCategory = testData.ServiceCategories[0]
+ } else {
+ t.Fatalf("cannot UPDATE Service Category, test data does not
have service categories")
+ }
+ _, reqInf, err :=
TOSession.UpdateServiceCategoryByName(firstServiceCategory.Name,
firstServiceCategory, h)
+ if err == nil {
+ t.Errorf("attempting to update service category with headers -
expected: error, actual: nil")
+ }
+ if reqInf.StatusCode != http.StatusPreconditionFailed {
+ t.Errorf("expected status code: %d, actual: %d",
http.StatusPreconditionFailed, reqInf.StatusCode)
+ }
+}
+
func UpdateTestServiceCategories(t *testing.T) {
firstServiceCategory := tc.ServiceCategory{}
if len(testData.ServiceCategories) > 0 {
@@ -151,7 +173,7 @@ func UpdateTestServiceCategories(t *testing.T) {
remoteServiceCategory.Name = "ServiceCategory2"
var alert tc.Alerts
- alert, _, err =
TOSession.UpdateServiceCategoryByName(firstServiceCategory.Name,
remoteServiceCategory)
+ alert, _, err =
TOSession.UpdateServiceCategoryByName(firstServiceCategory.Name,
remoteServiceCategory, nil)
if err != nil {
t.Errorf("cannot UPDATE Service Category by name: %v -
%v", err, alert)
}
@@ -171,7 +193,7 @@ func UpdateTestServiceCategories(t *testing.T) {
}
// revert back to original name
- alert, _, err =
TOSession.UpdateServiceCategoryByName(remoteServiceCategory.Name,
firstServiceCategory)
+ alert, _, err =
TOSession.UpdateServiceCategoryByName(remoteServiceCategory.Name,
firstServiceCategory, nil)
if err != nil {
t.Errorf("cannot UPDATE Service Category by name: %v -
%v", err, alert)
}
diff --git a/traffic_ops/testing/api/v3/tc-fixtures.json
b/traffic_ops/testing/api/v3/tc-fixtures.json
index 703e250..688ab8b 100644
--- a/traffic_ops/testing/api/v3/tc-fixtures.json
+++ b/traffic_ops/testing/api/v3/tc-fixtures.json
@@ -3927,12 +3927,10 @@
],
"serviceCategories": [
{
- "name": "serviceCategory1",
- "tenant": "tenant3"
+ "name": "serviceCategory1"
},
{
- "name": "barServiceCategory2",
- "tenant": "tenant4"
+ "name": "barServiceCategory2"
}
],
"staticdnsentries": [
diff --git a/traffic_ops/traffic_ops_golang/api/api.go
b/traffic_ops/traffic_ops_golang/api/api.go
index 867a7be..737b878 100644
--- a/traffic_ops/traffic_ops_golang/api/api.go
+++ b/traffic_ops/traffic_ops_golang/api/api.go
@@ -1016,11 +1016,20 @@ func CheckIfUnModified(h http.Header, tx *sqlx.Tx, ID
int, tableName string) (er
return nil, nil, http.StatusOK
}
-// GetLastUpdated checks for the resource in the database, and returns its
last_updated timestamp, if available.
+// GetLastUpdated checks for the resource by ID in the database, and returns
its last_updated timestamp, if available.
func GetLastUpdated(tx *sqlx.Tx, ID int, tableName string) (*time.Time, bool,
error) {
+ return getLastUpdatedByIdentifier(tx, "id", ID, tableName)
+}
+
+// GetLastUpdatedByName checks for the resource by name in the database, and
returns its last_updated timestamp, if available.
+func GetLastUpdatedByName(tx *sqlx.Tx, name string, tableName string)
(*time.Time, bool, error) {
+ return getLastUpdatedByIdentifier(tx, "name", name, tableName)
+}
+
+func getLastUpdatedByIdentifier(tx *sqlx.Tx, IDColumn string, IDValue
interface{}, tableName string) (*time.Time, bool, error) {
lastUpdated := time.Time{}
found := false
- rows, err := tx.Query(fmt.Sprintf(`select last_updated from %s where
id=$1`, pq.QuoteIdentifier(tableName)), ID)
+ rows, err := tx.Query(fmt.Sprintf(`select last_updated from %s where %s
= $1`, pq.QuoteIdentifier(tableName), pq.QuoteIdentifier(IDColumn)), IDValue)
if err != nil {
return nil, found, errors.New("querying last_updated: " +
err.Error())
}
diff --git
a/traffic_ops/traffic_ops_golang/servicecategory/servicecategories.go
b/traffic_ops/traffic_ops_golang/servicecategory/servicecategories.go
index 10b55e5..30c4af9 100644
--- a/traffic_ops/traffic_ops_golang/servicecategory/servicecategories.go
+++ b/traffic_ops/traffic_ops_golang/servicecategory/servicecategories.go
@@ -40,21 +40,7 @@ type TOServiceCategory struct {
}
func (v *TOServiceCategory) GetLastUpdated() (*time.Time, bool, error) {
- found := false
- lastUpdated := time.Time{}
- rows, err := v.APIInfo().Tx.Query(`select last_updated from
service_category where name=$1`, v.Name)
- if err != nil {
- return nil, found, errors.New("querying last_updated: " +
err.Error())
- }
- defer rows.Close()
- if !rows.Next() {
- return nil, found, errors.New("no resource found with this id")
- }
- found = true
- if err := rows.Scan(&lastUpdated); err != nil {
- return nil, found, errors.New("scanning last_updated: " +
err.Error())
- }
- return &lastUpdated, found, nil
+ return api.GetLastUpdatedByName(v.APIInfo().Tx, v.Name,
"service_category")
}
func (v *TOServiceCategory) SetLastUpdated(t tc.TimeNoMod) { v.LastUpdated = t
}
@@ -149,7 +135,7 @@ func Update(w http.ResponseWriter, r *http.Request) {
}
var origSC TOServiceCategory
- if err := inf.Tx.QueryRow(`SELECT name FROM service_category WHERE name
= $1`, name).Scan(&origSC.Name); err != nil {
+ if err := inf.Tx.QueryRow(`SELECT name, last_updated FROM
service_category WHERE name = $1`, name).Scan(&origSC.Name,
&origSC.LastUpdated); err != nil {
if err == sql.ErrNoRows {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest,
errors.New("no service category found with name "+name), nil)
return
@@ -157,6 +143,10 @@ func Update(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError,
nil, err)
return
}
+ if !api.IsUnmodified(r.Header, origSC.LastUpdated.Time) {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusPreconditionFailed,
errors.New("service category could not be modified because the precondition
failed"), nil)
+ return
+ }
resp, err := inf.Tx.Tx.Exec(updateQuery(), newSC.Name, name)
if err != nil {
diff --git a/traffic_ops/v3-client/serviceCategory.go
b/traffic_ops/v3-client/serviceCategory.go
index fc266c2..9330bb6 100644
--- a/traffic_ops/v3-client/serviceCategory.go
+++ b/traffic_ops/v3-client/serviceCategory.go
@@ -51,8 +51,7 @@ func (to *Session) CreateServiceCategory(serviceCategory
tc.ServiceCategory) (tc
}
// UpdateServiceCategoryByName updates a service category by its unique name.
-func (to *Session) UpdateServiceCategoryByName(name string, serviceCategory
tc.ServiceCategory) (tc.Alerts, ReqInf, error) {
-
+func (to *Session) UpdateServiceCategoryByName(name string, serviceCategory
tc.ServiceCategory, header http.Header) (tc.Alerts, ReqInf, error) {
var remoteAddr net.Addr
reqBody, err := json.Marshal(serviceCategory)
reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr:
remoteAddr}
@@ -60,11 +59,14 @@ func (to *Session) UpdateServiceCategoryByName(name string,
serviceCategory tc.S
return tc.Alerts{}, reqInf, err
}
route := fmt.Sprintf("%s/%s", API_SERVICE_CATEGORIES, name)
- resp, remoteAddr, err := to.request(http.MethodPut, route, reqBody, nil)
+ resp, remoteAddr, err := to.request(http.MethodPut, route, reqBody,
header)
+ if resp != nil {
+ reqInf.StatusCode = resp.StatusCode
+ defer resp.Body.Close()
+ }
if err != nil {
return tc.Alerts{}, reqInf, err
}
- defer resp.Body.Close()
var alerts tc.Alerts
if err := json.NewDecoder(resp.Body).Decode(&alerts); err != nil {
return tc.Alerts{}, reqInf, err
@@ -74,8 +76,8 @@ func (to *Session) UpdateServiceCategoryByName(name string,
serviceCategory tc.S
// GetServiceCategoriesWithHdr gets a list of service categories by the passed
in url values and http headers.
func (to *Session) GetServiceCategoriesWithHdr(values *url.Values, header
http.Header) ([]tc.ServiceCategory, ReqInf, error) {
- url := fmt.Sprintf("%s?%s", API_SERVICE_CATEGORIES, values.Encode())
- resp, remoteAddr, err := to.request(http.MethodGet, url, nil, header)
+ path := fmt.Sprintf("%s?%s", API_SERVICE_CATEGORIES, values.Encode())
+ resp, remoteAddr, err := to.request(http.MethodGet, path, nil, header)
reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr:
remoteAddr}
if err != nil {
return nil, reqInf, err
