You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2018/02/28 16:57:33 UTC
[GitHub] dewrich closed pull request #1922: TO golang -- add capability for custom changelog message per object type
dewrich closed pull request #1922: TO golang -- add capability for custom changelog message per object type
URL: https://github.com/apache/incubator-trafficcontrol/pull/1922
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/traffic_ops/traffic_ops_golang/api/change_log.go b/traffic_ops/traffic_ops_golang/api/change_log.go
index ac47809e7a..4136bbd64d 100644
--- a/traffic_ops/traffic_ops_golang/api/change_log.go
+++ b/traffic_ops/traffic_ops_golang/api/change_log.go
@@ -37,6 +37,10 @@ type ChangeLog struct {
LastUpdated tc.Time `json:"lastUpdated" db:"last_updated"`
}
+type ChangeLogger interface {
+ ChangeLogMessage(action string) (string, error)
+}
+
const (
ApiChange = "APICHANGE"
Updated = "Updated"
@@ -45,10 +49,21 @@ const (
)
func CreateChangeLog(level string, action string, i Identifier, user auth.CurrentUser, db *sqlx.DB) error {
- query := `INSERT INTO log (level, message, tm_user) VALUES ($1, $2, $3)`
id, _ := i.GetID()
message := action + " " + i.GetType() + ": " + i.GetAuditName() + " id: " + strconv.Itoa(id)
- log.Debugf("about to exec ", query, " with ", message)
+ // if the object has its own log message generation, use it
+ if t, ok := i.(ChangeLogger); ok {
+ m, err := t.ChangeLogMessage(action)
+ if err != nil {
+ log.Errorf("error %++v creating log message for %++v", err, t)
+ // use the default message in this case
+ } else {
+ message = m
+ }
+ }
+
+ query := `INSERT INTO log (level, message, tm_user) VALUES ($1, $2, $3)`
+ log.Debugf("about to exec %s with %s", query, message)
_, err := db.Exec(query, level, message, user.ID)
if err != nil {
log.Errorf("received error: %++v from audit log insertion", err)
diff --git a/traffic_ops/traffic_ops_golang/api/shared_handlers.go b/traffic_ops/traffic_ops_golang/api/shared_handlers.go
index 903fa9f3d7..f1edb0da4c 100644
--- a/traffic_ops/traffic_ops_golang/api/shared_handlers.go
+++ b/traffic_ops/traffic_ops_golang/api/shared_handlers.go
@@ -287,10 +287,12 @@ func DeleteHandler(typeRef Deleter, db *sqlx.DB) http.HandlerFunc {
log.Debugf("calling delete on object: %++v", d) //should have id set now
err, errType := d.Delete(db, *user)
if err != nil {
+ log.Errorf("error deleting: %++v", err)
tc.HandleErrorsWithType([]error{err}, errType, handleErrs)
return
}
//audit here
+ log.Debugf("changelog for delete on object")
CreateChangeLog(ApiChange, Deleted, d, *user, db)
//
resp := struct {
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
index ed984e3a11..9b9fc8bcb3 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go
@@ -49,7 +49,7 @@ func GetRefType() *TODeliveryServiceRequest {
//Implementation of the Identifier, Validator interface functions
// GetID is part of the tc.Identifier interface
-func (req *TODeliveryServiceRequest) GetID() (int, bool) {
+func (req TODeliveryServiceRequest) GetID() (int, bool) {
if req.ID == nil {
return 0, false
}
@@ -57,15 +57,12 @@ func (req *TODeliveryServiceRequest) GetID() (int, bool) {
}
// GetAuditName is part of the tc.Identifier interface
-func (req *TODeliveryServiceRequest) GetAuditName() string {
- if req.ID == nil {
- return "0"
- }
- return strconv.Itoa(*req.ID)
+func (req TODeliveryServiceRequest) GetAuditName() string {
+ return req.getXMLID()
}
// GetType is part of the tc.Identifier interface
-func (req *TODeliveryServiceRequest) GetType() string {
+func (req TODeliveryServiceRequest) GetType() string {
return "deliveryservice_request"
}
@@ -160,7 +157,7 @@ LEFT OUTER JOIN tm_user e ON r.last_edited_by_id = e.id
}
// IsTenantAuthorized implements the Tenantable interface to ensure the user is authorized on the deliveryservice tenant
-func (req *TODeliveryServiceRequest) IsTenantAuthorized(user auth.CurrentUser, db *sqlx.DB) (bool, error) {
+func (req TODeliveryServiceRequest) IsTenantAuthorized(user auth.CurrentUser, db *sqlx.DB) (bool, error) {
ds := req.DeliveryService
if ds == nil {
// No deliveryservice applied yet -- wide open
@@ -375,6 +372,7 @@ func (req *TODeliveryServiceRequest) Create(db *sqlx.DB, user auth.CurrentUser)
// Delete removes the request from the db
func (req *TODeliveryServiceRequest) Delete(db *sqlx.DB, user auth.CurrentUser) (error, tc.ApiErrorType) {
var st tc.RequestStatus
+ log.Debugln("DELETING REQUEST WITH ID ", strconv.Itoa(*req.ID))
if req.ID == nil {
return errors.New("cannot delete deliveryservice_request -- ID is nil"), tc.SystemError
}
@@ -404,21 +402,26 @@ func (req *TODeliveryServiceRequest) Delete(db *sqlx.DB, user auth.CurrentUser)
log.Error.Println("could not begin transaction: ", err.Error())
return tc.DBError, tc.SystemError
}
- log.Debugf("about to run exec query: %s with ds request: %++v", deleteRequestQuery(), req)
- result, err := tx.NamedExec(deleteRequestQuery(), req)
+ query := `DELETE FROM deliveryservice_request WHERE id=` + strconv.Itoa(*req.ID)
+ log.Debugf("about to run exec query: %s", query)
+
+ result, err := tx.Exec(query)
if err != nil {
log.Errorln("received error from delete execution: ", err.Error())
return tc.DBError, tc.SystemError
}
rowsAffected, err := result.RowsAffected()
if err != nil {
+ log.Errorln("error getting rows affected: ", err.Error())
return tc.DBError, tc.SystemError
}
if rowsAffected < 1 {
+ log.Errorln("no request with that id found")
return errors.New("no request with that id found"), tc.DataMissingError
}
if rowsAffected > 1 {
- return fmt.Errorf("this create affected too many rows: %d", rowsAffected), tc.SystemError
+ log.Errorln("the delete affected too many rows")
+ return fmt.Errorf("this delete affected too many rows: %d", rowsAffected), tc.SystemError
}
err = tx.Commit()
if err != nil {
@@ -430,6 +433,25 @@ func (req *TODeliveryServiceRequest) Delete(db *sqlx.DB, user auth.CurrentUser)
return nil, tc.NoError
}
+func (req TODeliveryServiceRequest) getXMLID() string {
+ if req.DeliveryService == nil || req.DeliveryService.XMLID == nil {
+ id, _ := req.GetID()
+ return strconv.Itoa(id)
+ }
+ return *req.DeliveryService.XMLID
+}
+
+// ChangeLogMessage implements the api.ChangeLogger interface for a custom log message
+func (req TODeliveryServiceRequest) ChangeLogMessage(action string) (string, error) {
+ changeType := "unknown change type"
+ if req.ChangeType != nil {
+ changeType = *req.ChangeType
+ }
+ // use ID in case don't have access to XMLID (e.g. on DELETE)
+ message := action + ` ` + req.GetType() + ` of type '` + changeType + `' for deliveryservice '` + req.getXMLID() + `'`
+ return message, nil
+}
+
// isActiveRequest returns true if a request using this XMLID is currently in an active state
func isActiveRequest(db *sqlx.DB, XMLID string) (bool, error) {
q := `SELECT EXISTS(SELECT 1 FROM deliveryservice_request
@@ -482,7 +504,9 @@ WHERE id=:id`
////////////////////////////////////////////////////////////////
// Assignment change
-type deliveryServiceRequestAssignment TODeliveryServiceRequest
+type deliveryServiceRequestAssignment struct {
+ TODeliveryServiceRequest
+}
// GetAssignRefType is used to decode the JSON for deliveryservice_request assignment
func GetAssignRefType() *deliveryServiceRequestAssignment {
@@ -513,7 +537,7 @@ func (req *deliveryServiceRequestAssignment) Update(db *sqlx.DB, user auth.Curre
// Only assigneeID changes -- nothing else
assigneeID := req.AssigneeID
- *req = deliveryServiceRequestAssignment(current)
+ *req = deliveryServiceRequestAssignment{current}
req.AssigneeID = assigneeID
rollbackTransaction := true
@@ -557,31 +581,40 @@ func (req *deliveryServiceRequestAssignment) Update(db *sqlx.DB, user auth.Curre
log.Errorln("Could not commit transaction: ", err)
return tc.DBError, tc.SystemError
}
+
+ // update req with current info
+ err = db.QueryRowx(selectDeliveryServiceRequestsQuery() + ` WHERE r.id=` + strconv.Itoa(*req.ID)).StructScan(req)
+ if err != nil {
+ log.Errorf("Error querying DeliveryServiceRequests: %v", err)
+ return err, tc.SystemError
+ }
+
rollbackTransaction = false
return nil, tc.NoError
}
-func (req *deliveryServiceRequestAssignment) GetID() (int, bool) {
- return (*TODeliveryServiceRequest)(req).GetID()
-}
-
-func (req *deliveryServiceRequestAssignment) GetType() string {
- return (*TODeliveryServiceRequest)(req).GetType()
+func (req deliveryServiceRequestAssignment) Validate(db *sqlx.DB) []error {
+ return nil
}
-func (req *deliveryServiceRequestAssignment) GetAuditName() string {
- return (*TODeliveryServiceRequest)(req).GetAuditName()
-}
+// ChangeLogMessage implements the api.ChangeLogger interface for a custom log message
+func (req deliveryServiceRequestAssignment) ChangeLogMessage(action string) (string, error) {
+ a := "NONE"
+ if req.Assignee != nil {
+ a = *req.Assignee
+ }
+ message := `Changed assignee of ?` + req.getXMLID() + `? ` + req.GetType() + ` to '` + a + `'`
-func (req *deliveryServiceRequestAssignment) Validate(db *sqlx.DB) []error {
- return nil
+ return message, nil
}
////////////////////////////////////////////////////////////////
// Status change
// deliveryServiceRequestStatus implements interfaces needed to update the request status only
-type deliveryServiceRequestStatus TODeliveryServiceRequest
+type deliveryServiceRequestStatus struct {
+ TODeliveryServiceRequest
+}
// GetStatusRefType is used to decode the JSON for deliveryservice_request status change
func GetStatusRefType() *deliveryServiceRequestStatus {
@@ -595,7 +628,7 @@ func (req *deliveryServiceRequestStatus) Update(db *sqlx.DB, user auth.CurrentUs
// for status transition
// get original
- var current deliveryServiceRequestStatus
+ var current TODeliveryServiceRequest
if req.ID == nil {
log.Errorf("error updating DeliveryServiceRequestStatus: ID is nil")
}
@@ -611,7 +644,7 @@ func (req *deliveryServiceRequestStatus) Update(db *sqlx.DB, user auth.CurrentUs
// keep everything else the same -- only update status
st := req.Status
- *req = current
+ *req = deliveryServiceRequestStatus{current}
req.Status = st
rollbackTransaction := true
@@ -651,26 +684,25 @@ func (req *deliveryServiceRequestStatus) Update(db *sqlx.DB, user auth.CurrentUs
log.Errorln("Could not commit transaction: ", err)
return tc.DBError, tc.SystemError
}
- rollbackTransaction = false
- return nil, tc.NoError
-}
-
-// GetID from base type
-func (req *deliveryServiceRequestStatus) GetID() (int, bool) {
- return (*TODeliveryServiceRequest)(req).GetID()
-}
-// GetType from base type
-func (req *deliveryServiceRequestStatus) GetType() string {
- return (*TODeliveryServiceRequest)(req).GetType()
-}
+ // update req with current info
+ err = db.QueryRowx(selectDeliveryServiceRequestsQuery() + ` WHERE r.id=` + strconv.Itoa(*req.ID)).StructScan(req)
+ if err != nil {
+ log.Errorf("Error querying DeliveryServiceRequests: %v", err)
+ return err, tc.SystemError
+ }
-// GetAuditName from base type
-func (req *deliveryServiceRequestStatus) GetAuditName() string {
- return (*TODeliveryServiceRequest)(req).GetAuditName()
+ rollbackTransaction = false
+ return nil, tc.NoError
}
// Validate is not needed when only Status is updated
-func (req *deliveryServiceRequestStatus) Validate(db *sqlx.DB) []error {
+func (req deliveryServiceRequestStatus) Validate(db *sqlx.DB) []error {
return nil
}
+
+// ChangeLogMessage implements the api.ChangeLogger interface for a custom log message
+func (req deliveryServiceRequestStatus) ChangeLogMessage(action string) (string, error) {
+ message := `Changed status of ?` + req.getXMLID() + `? ` + req.GetType() + ` to '` + string(*req.Status) + `'`
+ return message, nil
+}
diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests_test.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests_test.go
index e038f69c77..5aec066c25 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests_test.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests_test.go
@@ -89,8 +89,8 @@ func TestGetDeliveryServiceRequest(t *testing.T) {
t.Errorf("expected ID to be %d, not %d", 10, id)
}
exp := "10"
- if r.GetAuditName() != exp {
- t.Errorf("expected AuditName to be %s, not %s", exp, r.GetAuditName())
+ if s != r.GetAuditName() {
+ t.Errorf("expected AuditName to be '%s', not '%s'", s, r.GetAuditName())
}
exp = "deliveryservice_request"
if r.GetType() != "deliveryservice_request" {
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services