[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5615: Added check so no two users can modify topology name at the same time.

[GitBox <gi...@apache.org>]

rawlinp commented on a change in pull request #5615:
URL: https://github.com/apache/trafficcontrol/pull/5615#discussion_r593265286



##########
File path: traffic_ops/traffic_ops_golang/topology/topologies.go
##########
@@ -600,6 +600,20 @@ func (topology *TOTopology) Update(h http.Header) (error, error, int) {
 	if len(topologies) != 1 {
 		return fmt.Errorf("cannot find exactly 1 topology with the query string provided"), nil, http.StatusBadRequest
 	}
+
+	// check if the name field was being updated by someone else
+	var existingLastUpdated *tc.TimeNoMod

Review comment:
       This is pretty much the same code as https://github.com/apache/trafficcontrol/pull/5605/files#diff-cc0495e268314edbdfc50fba81e5ad6943ff1a3cd940fd94ef7de741862710f3R135
   
   We have a shared function that already does this for `id`-based objects (https://github.com/apache/trafficcontrol/blob/master/traffic_ops/traffic_ops_golang/api/api.go#L1131), but we should add another similar function `CheckIfUnModifiedByName()` for `name`-based objects, which calls `GetLastUpdatedByName` instead of `GetLastUpdated`. Additionally, I'm thinking we should update the method to skip the `LastUpdated` IMS check if the request headers don't even contain `if-match`/`if-modified-since`. Otherwise, it's just a wasted query.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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