This is an automated email from the ASF dual-hosted git repository.
ocket8888 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new e831c58 Rewrite DELETE federation delivery service to golang (#3967)
e831c58 is described below
commit e831c587b7f1016d49620fbcad03b81e655f258f
Author: Michael Hoppal <[email protected]>
AuthorDate: Fri Oct 11 20:42:33 2019 -0600
Rewrite DELETE federation delivery service to golang (#3967)
* Rewrite DELETE federation delivery service to golang
* Fix test case
* Dont change path as part of rewrite
* Minor fix
* Update ds.go
---
.../api/federations_id_deliveryservices_id.rst | 2 +-
traffic_ops/client/federation.go | 16 ++++
traffic_ops/testing/api/v14/federations_test.go | 34 ++++++--
traffic_ops/traffic_ops_golang/federations/ds.go | 94 ++++++++++++++++++++-
.../traffic_ops_golang/federations/ds_test.go | 98 ++++++++++++++++++++++
traffic_ops/traffic_ops_golang/routing/routes.go | 1 +
6 files changed, 236 insertions(+), 9 deletions(-)
diff --git a/docs/source/api/federations_id_deliveryservices_id.rst
b/docs/source/api/federations_id_deliveryservices_id.rst
index 58edb2a..a982fbe 100644
--- a/docs/source/api/federations_id_deliveryservices_id.rst
+++ b/docs/source/api/federations_id_deliveryservices_id.rst
@@ -70,6 +70,6 @@ Response Structure
{ "alerts": [
{
"level": "success",
- "text": "Removed delivery service [ test ] from
federation [ foo.bar. ]"
+ "text": "federation deliveryservice was deleted."
}
]}
diff --git a/traffic_ops/client/federation.go b/traffic_ops/client/federation.go
index 10908fa..64138a9 100644
--- a/traffic_ops/client/federation.go
+++ b/traffic_ops/client/federation.go
@@ -89,3 +89,19 @@ func (to *Session)
GetFederationDeliveryServices(federationID int) ([]tc.Federat
inf, err := get(to, fmt.Sprintf("%s/federations/%v/deliveryservices",
apiBase, federationID), &data)
return data.Response, inf, err
}
+
+// DeleteFederationDeliveryService Deletes a given Delivery Service from a
Federation
+func (to *Session) DeleteFederationDeliveryService(federationID,
deliveryServiceID int) (tc.Alerts, ReqInf, error) {
+ route := fmt.Sprintf("%s/federations/%v/deliveryservices/%v", apiBase,
federationID, deliveryServiceID)
+ resp, remoteAddr, err := to.request(http.MethodDelete, route, nil)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr:
remoteAddr}
+ 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
+ }
+ return alerts, reqInf, nil
+}
diff --git a/traffic_ops/testing/api/v14/federations_test.go
b/traffic_ops/testing/api/v14/federations_test.go
index a757abb..23fd672 100644
--- a/traffic_ops/testing/api/v14/federations_test.go
+++ b/traffic_ops/testing/api/v14/federations_test.go
@@ -21,7 +21,7 @@ import (
func TestFederations(t *testing.T) {
WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles,
Statuses, Divisions, Regions, PhysLocations, CacheGroups, DeliveryServices,
UsersDeliveryServices, CDNFederations}, func() {
- PostTestFederationsDeliveryServices(t)
+ PostDeleteTestFederationsDeliveryServices(t)
GetTestFederations(t)
})
}
@@ -76,7 +76,7 @@ func GetTestFederations(t *testing.T) {
}
}
-func PostTestFederationsDeliveryServices(t *testing.T) {
+func PostDeleteTestFederationsDeliveryServices(t *testing.T) {
dses, _, err := TOSession.GetDeliveryServices()
if err != nil {
t.Fatalf("cannot GET DeliveryServices: %v - %v\n", err, dses)
@@ -85,24 +85,46 @@ func PostTestFederationsDeliveryServices(t *testing.T) {
t.Fatalf("no delivery services, must have at least 1 ds to test
federations deliveryservices\n")
}
ds := dses[0]
+ ds1 := dses[1]
+
if len(fedIDs) == 0 {
t.Fatalf("no federations, must have at least 1 federation to
test federations deliveryservices\n")
}
fedID := fedIDs[0]
- if _, err = TOSession.CreateFederationDeliveryServices(fedID,
[]int{ds.ID}, true); err != nil {
+ if _, err = TOSession.CreateFederationDeliveryServices(fedID,
[]int{ds.ID, ds1.ID}, true); err != nil {
t.Fatalf("creating federations delivery services: %v\n", err)
}
- // Test get created Federation Delivery Service
+ // Test get created Federation Delivery Services
fedDSes, _, err := TOSession.GetFederationDeliveryServices(fedID)
if err != nil {
t.Fatalf("cannot GET Federation DeliveryServices: %v\n", err)
}
+ if len(fedDSes) != 2 {
+ t.Fatalf("two Federation DeliveryService exepected for
Federation %v, %v was returned\n", fedID, len(fedDSes))
+ }
+
+ // Delete one of the Delivery Services from the Federation
+ _, _, err = TOSession.DeleteFederationDeliveryService(fedID, ds.ID)
+ if err != nil {
+ t.Fatalf("cannot Delete Federation %v DeliveryService %v:
%v\n", fedID, ds.ID, err)
+ }
+
+ // Make sure it is deleted
+
+ // Test get created Federation Delivery Services
+ fedDSes, _, err = TOSession.GetFederationDeliveryServices(fedID)
+ if err != nil {
+ t.Fatalf("cannot GET Federation DeliveryServices: %v\n", err)
+ }
if len(fedDSes) != 1 {
t.Fatalf("one Federation DeliveryService exepected for
Federation %v, %v was returned\n", fedID, len(fedDSes))
}
- if *fedDSes[0].ID != ds.ID {
- t.Errorf("expected DeliveryService %v to be returned for
Federation %v DeliveryServices, %v was instead returned\n", ds.ID, fedID,
*fedDSes[0].ID)
+
+ // Attempt to delete the last one which should fail as you cannot
remove the last
+ _, _, err = TOSession.DeleteFederationDeliveryService(fedID, ds1.ID)
+ if err == nil {
+ t.Fatalf("expected to receive error from attempting to delete
last Delivery Service from a Federation\n")
}
}
diff --git a/traffic_ops/traffic_ops_golang/federations/ds.go
b/traffic_ops/traffic_ops_golang/federations/ds.go
index d2951e7..1e844c3 100644
--- a/traffic_ops/traffic_ops_golang/federations/ds.go
+++ b/traffic_ops/traffic_ops_golang/federations/ds.go
@@ -27,6 +27,7 @@ import (
"strconv"
"github.com/apache/trafficcontrol/lib/go-tc"
+
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
@@ -87,6 +88,12 @@ func deleteDSFeds(tx *sql.Tx, fedID int) error {
return err
}
+func deleteFedDS(tx *sql.Tx, fedID, dsID int) error {
+ qry := `DELETE FROM federation_deliveryservice WHERE federation = $1
AND deliveryservice = $2`
+ _, err := tx.Exec(qry, fedID, dsID)
+ return err
+}
+
func insertDSFeds(tx *sql.Tx, fedID int, dsIDs []int) error {
qry := `
INSERT INTO federation_deliveryservice (federation, deliveryservice)
@@ -108,9 +115,10 @@ func getFedNameByID(tx *sql.Tx, id int) (string, bool,
error) {
return name, true, nil
}
-// TOFedDSes data structure to use on read of federation deliveryservices
+// TOFedDSes data structure to use on read/delete of federation
deliveryservices
type TOFedDSes struct {
api.APIInfoImpl `json:"-"`
+ fedID *int
tc.FederationDeliveryServiceNullable
}
@@ -123,11 +131,93 @@ func (v *TOFedDSes) ParamColumns()
map[string]dbhelpers.WhereColumnInfo {
}
}
func (v *TOFedDSes) GetType() string {
- return "federation_deliveryservice"
+ return "federation deliveryservice"
+}
+
+func (v *TOFedDSes) SetKeys(keys map[string]interface{}) {
+ i, _ := keys["id"].(int)
+ v.fedID = &i
+}
+
+func (v *TOFedDSes) GetKeys() (map[string]interface{}, bool) {
+ if v.fedID == nil {
+ return map[string]interface{}{"id": 0}, false
+ }
+ return map[string]interface{}{"id": *v.fedID}, true
+}
+
+func (v *TOFedDSes) GetAuditName() string {
+ if v.XMLID != nil {
+ return *v.XMLID
+ }
+ return strconv.Itoa(*v.ID)
+}
+
+func (v *TOFedDSes) GetKeyFieldsInfo() []api.KeyFieldInfo {
+ return []api.KeyFieldInfo{
+ {"id", api.GetIntKey},
+ }
}
func (v *TOFedDSes) Read() ([]interface{}, error, error, int) { return
api.GenericRead(v) }
+func (v *TOFedDSes) Delete() (error, error, int) {
+ dsIDStr, ok := v.APIInfo().Params["dsID"]
+ if !ok {
+ return errors.New("dsID must be specified for deletion"), nil,
http.StatusBadRequest
+ }
+ dsID, err := strconv.Atoi(dsIDStr)
+ if err != nil {
+ return errors.New("dsID must be an integer"), nil,
http.StatusBadRequest
+ }
+ v.ID = &dsID
+
+ // Check that we can delete it
+ if respCode, usrErr, sysErr := checkFedDSDeletion(v.APIInfo().Tx.Tx,
*v.fedID, dsID); usrErr != nil || sysErr != nil {
+ if usrErr != nil {
+ return usrErr, sysErr, respCode
+ }
+ return usrErr, sysErr, respCode
+ }
+
+ // Actually delete the DS from the Federation
+ if err := deleteFedDS(v.APIInfo().Tx.Tx, *v.fedID, dsID); err != nil {
+ return api.ParseDBError(err)
+ }
+
+ return nil, nil, http.StatusOK
+}
+
+func checkFedDSDeletion(tx *sql.Tx, fedID, dsID int) (int, error, error) {
+
+ q := `SELECT ARRAY(SELECT deliveryservice FROM
federation_deliveryservice WHERE federation=$1)`
+ dsIDs := []int64{} // pq.Array does not support int slice needs to be
int64
+ err := tx.QueryRow(q, fedID).Scan(pq.Array(&dsIDs))
+ if err != nil {
+ return http.StatusInternalServerError, nil,
fmt.Errorf("querying federation %v delivery services - %v", fedID, err)
+ }
+
+ if len(dsIDs) == 0 {
+ return http.StatusNotFound, fmt.Errorf("federation %v not
found", fedID), nil
+ }
+
+ if len(dsIDs) < 2 {
+ return http.StatusBadRequest, fmt.Errorf("a federation must
have at least one delivery service assigned"), nil
+ }
+ found := false
+ dsID64 := int64(dsID) // need in order to compare
+ for _, id := range dsIDs {
+ if id == dsID64 {
+ found = true
+ break
+ }
+ }
+ if !found {
+ return http.StatusBadRequest, fmt.Errorf("delivery service %v
is not associated with federation %v", dsID, fedID), nil
+ }
+ return http.StatusOK, nil, nil
+}
+
func selectQuery() string {
query := `SELECT
diff --git a/traffic_ops/traffic_ops_golang/federations/ds_test.go
b/traffic_ops/traffic_ops_golang/federations/ds_test.go
new file mode 100644
index 0000000..b56b29f
--- /dev/null
+++ b/traffic_ops/traffic_ops_golang/federations/ds_test.go
@@ -0,0 +1,98 @@
+package federations
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+import (
+ "testing"
+
+ "github.com/jmoiron/sqlx"
+ "gopkg.in/DATA-DOG/go-sqlmock.v1"
+)
+
+func TestCheckFedDSDeletion(t *testing.T) {
+ var testCases = []struct {
+ description string
+ returnArray string
+ expectUsrErr bool
+ expectUsrStr string
+ inputDSID int
+ }{
+ {
+ description: "Success: Deleted Federation Delivery
Service",
+ returnArray: "{1,2}",
+ expectUsrErr: false,
+ expectUsrStr: "",
+ inputDSID: 1,
+ },
+ {
+ description: "Failure: Remove last Federation Delivery
Service",
+ returnArray: "{1}",
+ expectUsrErr: true,
+ expectUsrStr: "a federation must have at least one
delivery service assigned",
+ inputDSID: 1,
+ },
+ {
+ description: "Failure: Federation not found",
+ returnArray: "{}",
+ expectUsrErr: true,
+ expectUsrStr: "federation 1 not found",
+ inputDSID: 1,
+ },
+ {
+ description: "Failure: Delivery Service not found",
+ returnArray: "{3,4}",
+ expectUsrErr: true,
+ expectUsrStr: "delivery service 1 is not associated
with federation 1",
+ inputDSID: 1,
+ },
+ }
+ for _, tc := range testCases {
+ t.Run(tc.description, func(t *testing.T) {
+ t.Log("Starting test scenario: ", tc.description)
+ 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()
+ rows := sqlmock.NewRows([]string{"array"})
+ rows = rows.AddRow(tc.returnArray)
+ mock.ExpectBegin()
+ mock.ExpectQuery("SELECT").WillReturnRows(rows)
+ mock.ExpectCommit()
+ _, usrErr, sysErr :=
checkFedDSDeletion(db.MustBegin().Tx, 1, tc.inputDSID)
+ if tc.expectUsrErr {
+ if usrErr == nil {
+ t.Errorf("User error expected: received
none")
+ }
+ if usrErr.Error() != tc.expectUsrStr {
+ t.Errorf("Expected error with text %v:
received %v", tc.expectUsrStr, usrErr.Error())
+ }
+ }
+ if !tc.expectUsrErr && usrErr != nil {
+ t.Errorf("User error not expected: received
error %v", err)
+ }
+ if sysErr != nil {
+ t.Errorf("System error not expected: received
error %v", err)
+ }
+ })
+ }
+}
diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go
b/traffic_ops/traffic_ops_golang/routing/routes.go
index 5f90653..28a47ae 100644
--- a/traffic_ops/traffic_ops_golang/routing/routes.go
+++ b/traffic_ops/traffic_ops_golang/routing/routes.go
@@ -444,6 +444,7 @@ func Routes(d ServerData) ([]Route, []RawRoute,
http.Handler, error) {
{1.1, http.MethodGet, `federations/?(\.json)?$`,
federations.Get, auth.PrivLevelFederation, Authenticated, nil},
{1.1, http.MethodPost,
`federations/{id}/deliveryservices?(\.json)?$`, federations.PostDSes,
auth.PrivLevelAdmin, Authenticated, nil},
{1.1, http.MethodGet,
`federations/{id}/deliveryservices?(\.json)?$`,
api.ReadHandler(&federations.TOFedDSes{}), auth.PrivLevelReadOnly,
Authenticated, nil},
+ {1.1, http.MethodDelete,
`federations/{id}/deliveryservices/{dsID}/?(\.json)?$`,
api.DeleteHandler(&federations.TOFedDSes{}), auth.PrivLevelAdmin,
Authenticated, nil},
////DeliveryServices
{1.1, http.MethodGet, `deliveryservices/?(\.json)?$`,
api.ReadHandler(&deliveryservice.TODeliveryService{}), auth.PrivLevelReadOnly,
Authenticated, nil},
