This is an automated email from the ASF dual-hosted git repository.

rawlin 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 78871ce  Ability to change the name of a topology (#5495)
78871ce is described below

commit 78871cee1ada8c2ee9cba1d103e09fc707def72c
Author: Rima Shah <[email protected]>
AuthorDate: Fri Feb 5 10:43:31 2021 -0700

    Ability to change the name of a topology (#5495)
    
    * Updated TP to pass old topology name in query param and in TO updated the 
update query
    
    * Removed setting of the primary key to query param, to allow name to be 
mutable.
    
    * Added test case for api/v4
    
    * Update toplogies to update name after parents and cachegroup have been 
deleted.
    
    * Removed extra comma from sql query
    
    * Updated query and function name
    
    * Checking for original topology name in Validate()
    
    * Updated Changelog
    
    * Updated based on review comments.
---
 CHANGELOG.md                                       |  1 +
 traffic_ops/testing/api/v4/topologies_test.go      | 38 ++++++++++++++++++++++
 .../traffic_ops_golang/topology/topologies.go      | 37 ++++++++++++---------
 .../app/src/common/api/TopologyService.js          |  4 +--
 .../topology/edit/FormEditTopologyController.js    |  9 ++---
 .../modules/form/topology/form.topology.tpl.html   |  5 +--
 .../app/src/common/service/utils/TopologyUtils.js  |  8 ++---
 .../test/end_to_end/topologies/topologies-spec.js  |  1 -
 8 files changed, 74 insertions(+), 29 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 66f18fa..2cd6e29 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@ The format is based on [Keep a 
Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ## [unreleased]
 ### Added
+- Traffic Portal: 
[#5361](https://github.com/apache/trafficcontrol/issues/5361) - Added the 
ability to change the name of a topology.
 - Traffic Portal: 
[#5340](https://github.com/apache/trafficcontrol/issues/5340) - Added the 
ability to resend a user registration from user screen.
 - Traffic Portal: 
[#5394](https://github.com/apache/trafficcontrol/issues/5394) - Converts the 
tenant table to a tenant tree for usability
 - Traffic Portal: upgraded delivery service UI tables to use more 
powerful/performant ag-grid component
diff --git a/traffic_ops/testing/api/v4/topologies_test.go 
b/traffic_ops/testing/api/v4/topologies_test.go
index b078e08..a9a9333 100644
--- a/traffic_ops/testing/api/v4/topologies_test.go
+++ b/traffic_ops/testing/api/v4/topologies_test.go
@@ -41,6 +41,7 @@ func TestTopologies(t *testing.T) {
                ValidationTestTopologies(t)
                UpdateValidateTopologyORGServerCacheGroup(t)
                EdgeParentOfEdgeSucceedsWithWarning(t)
+               UpdateTopologyName(t)
        })
 }
 
@@ -294,6 +295,43 @@ func UpdateValidateTopologyORGServerCacheGroup(t 
*testing.T) {
        }
 }
 
+func UpdateTopologyName(t *testing.T) {
+       currentTopologyName := "top-used-by-cdn1-and-cdn2"
+
+       // Get details on existing topology
+       resp, _, err := TOSession.GetTopologyWithHdr(currentTopologyName, nil)
+       if err != nil {
+               t.Errorf("unable to get topology with name: %v", 
currentTopologyName)
+       }
+       newTopologyName := "test-topology"
+       resp.Name = newTopologyName
+
+       // Update topology with new name
+       updateResponse, _, err := TOSession.UpdateTopology(currentTopologyName, 
*resp)
+       if err != nil {
+               t.Errorf("cannot PUT topology: %v - %v", err, updateResponse)
+       }
+       if updateResponse.Response.Name != newTopologyName {
+               t.Errorf("update topology name failed, expected: %v but 
got:%v", newTopologyName, updateResponse.Response.Name)
+       }
+
+       //To check whether the primary key change trickled down to DS table
+       resp1, _, err := 
TOSession.GetDeliveryServiceByXMLIDNullableWithHdr("top-ds-in-cdn2", nil)
+       if err != nil {
+               t.Errorf("failed to get details on DS: %v", err)
+       }
+       if *resp1[0].Topology != newTopologyName {
+               t.Errorf("topology name change failed to trickle to delivery 
service table, expected: %v but got:%v", newTopologyName, *resp1[0].Topology)
+       }
+
+       // Set everything back as it was for further testing.
+       resp.Name = currentTopologyName
+       r, _, err := TOSession.UpdateTopology(newTopologyName, *resp)
+       if err != nil {
+               t.Errorf("cannot PUT topology: %v - %v", err, r)
+       }
+}
+
 func DeleteTestTopologies(t *testing.T) {
        for _, top := range testData.Topologies {
                delResp, _, err := TOSession.DeleteTopology(top.Name)
diff --git a/traffic_ops/traffic_ops_golang/topology/topologies.go 
b/traffic_ops/traffic_ops_golang/topology/topologies.go
index 4e0f9f5..08d293c 100644
--- a/traffic_ops/traffic_ops_golang/topology/topologies.go
+++ b/traffic_ops/traffic_ops_golang/topology/topologies.go
@@ -46,6 +46,7 @@ import (
 type TOTopology struct {
        api.APIInfoImpl `json:"-"`
        Alerts          tc.Alerts `json:"-"`
+       RequestedName   string
        tc.Topology
 }
 
@@ -89,6 +90,7 @@ func (topology *TOTopology) GetType() string {
 
 // Validate is a requirement of the api.Validator interface.
 func (topology *TOTopology) Validate() error {
+       currentTopoName := topology.APIInfoImpl.ReqInfo.Params["name"]
        nameRule := 
validation.NewStringRule(tovalidate.IsAlphanumericUnderscoreDash, "must consist 
of only alphanumeric, dash, or underscore characters.")
        rules := validation.Errors{}
        rules["name"] = validation.Validate(topology.Name, validation.Required, 
nameRule)
@@ -142,17 +144,17 @@ func (topology *TOTopology) Validate() error {
        for index, cacheGroup := range cacheGroups {
                cacheGroupIds[index] = *cacheGroup.ID
        }
-       dsCDNs, err := 
dbhelpers.GetDeliveryServiceCDNsByTopology(topology.ReqInfo.Tx.Tx, 
topology.Name)
+       dsCDNs, err := 
dbhelpers.GetDeliveryServiceCDNsByTopology(topology.ReqInfo.Tx.Tx, 
currentTopoName)
        if err != nil {
                log.Errorf("validating topology: %v", err)
                return errors.New("unable to validate topology")
        }
        rules["empty cachegroups"] = 
topology_validation.CheckForEmptyCacheGroups(topology.ReqInfo.Tx, 
cacheGroupIds, dsCDNs, false, nil)
-       rules["required capabilities"] = 
topology.validateDSRequiredCapabilities()
+       rules["required capabilities"] = 
topology.validateDSRequiredCapabilities(currentTopoName)
 
        //Get current Topology-CG for the requested change.
        topoCachegroupNames := topology.getCachegroupNames()
-       userErr, sysErr, _ = 
dbhelpers.CheckTopologyOrgServerCGInDSCG(topology.ReqInfo.Tx.Tx, dsCDNs, 
topology.Name, topoCachegroupNames)
+       userErr, sysErr, _ = 
dbhelpers.CheckTopologyOrgServerCGInDSCG(topology.ReqInfo.Tx.Tx, dsCDNs, 
currentTopoName, topoCachegroupNames)
        if userErr != nil {
                return userErr
        }
@@ -176,6 +178,7 @@ func (topology *TOTopology) Validate() error {
 }
 
 func (topology *TOTopology) nodesInOtherTopologies() ([]tc.TopologyNode, 
map[string][]string, error) {
+       currentTopoName := topology.APIInfoImpl.ReqInfo.Params["name"]
        baseError := errors.New("unable to verify that there are no cycles 
across all topologies")
        where := `WHERE name != :topology_name`
        query := selectQueryWithParentNames() + where + `
@@ -183,7 +186,7 @@ func (topology *TOTopology) nodesInOtherTopologies() 
([]tc.TopologyNode, map[str
                UNION ` + selectNonTopologyParentCacheGroupsQuery()
 
        parameters := map[string]interface{}{
-               "topology_name":    topology.Name,
+               "topology_name":    currentTopoName,
                "edge_type_prefix": strings.ToLower(tc.EdgeTypePrefix) + "%",
                "mid_type_prefix":  strings.ToLower(tc.MidTypePrefix) + "%",
        }
@@ -268,12 +271,12 @@ func (topology *TOTopology) nodesInOtherTopologies() 
([]tc.TopologyNode, map[str
        return nodes, topologiesByCacheGroup, nil
 }
 
-func (topology TOTopology) validateDSRequiredCapabilities() error {
+func (topology TOTopology) validateDSRequiredCapabilities(currentTopoName 
string) error {
        baseError := errors.New("unable to verify that delivery service 
required capabilities are satisfied")
        tx := topology.APIInfo().Tx.Tx
-       dsRequiredCapabilities, dsCDNs, err := 
getDSRequiredCapabilitiesByTopology(topology.Name, tx)
+       dsRequiredCapabilities, dsCDNs, err := 
getDSRequiredCapabilitiesByTopology(currentTopoName, tx)
        if err != nil {
-               log.Errorf("validating delivery service required capabilities 
for topology %s: %v", topology.Name, err)
+               log.Errorf("validating delivery service required capabilities 
for topology %s: %v", currentTopoName, err)
                return baseError
        }
        if len(dsRequiredCapabilities) == 0 {
@@ -310,7 +313,7 @@ GROUP BY s.id, s.cdn_id, c.name
        }
        cachegroupServers, serverCapabilities, serverCDNs, err := 
dbhelpers.ScanCachegroupsServerCapabilities(rows)
        if err != nil {
-               log.Errorf("validating delivery service required capabilities 
for topology %s: %v", topology.Name, err)
+               log.Errorf("validating delivery service required capabilities 
for topology %s: %v", currentTopoName, err)
                return baseError
        }
 
@@ -395,6 +398,7 @@ func (topology TOTopology) GetKeys() 
(map[string]interface{}, bool) {
 // SetKeys is a requirement of the api.Updater interface and is called by
 // api.UpdateHandler().
 func (topology *TOTopology) SetKeys(keys map[string]interface{}) {
+       topology.RequestedName = topology.Name
        topology.Name, _ = keys["name"].(string)
 }
 
@@ -572,10 +576,10 @@ func (topology *TOTopology) addParents() (error, error, 
int) {
        return nil, nil, http.StatusOK
 }
 
-func (topology *TOTopology) setDescription() (error, error, int) {
-       rows, err := topology.ReqInfo.Tx.Query(updateQuery(), 
topology.Description, topology.Name)
+func (topology *TOTopology) setTopologyDetails() (error, error, int) {
+       rows, err := topology.ReqInfo.Tx.Query(updateQuery(), 
topology.RequestedName, topology.Description, topology.Name)
        if err != nil {
-               return nil, fmt.Errorf("topology update: error setting the 
description for topology %v: %v", topology.Name, err.Error()), 
http.StatusInternalServerError
+               return nil, fmt.Errorf("topology update: error setting the name 
and/or description for topology %v: %v", topology.Name, err.Error()), 
http.StatusInternalServerError
        }
        defer log.Close(rows, "unable to close DB connection")
        for rows.Next() {
@@ -597,9 +601,6 @@ func (topology *TOTopology) Update(h http.Header) (error, 
error, int) {
                return fmt.Errorf("cannot find exactly 1 topology with the 
query string provided"), nil, http.StatusBadRequest
        }
        oldTopology := TOTopology{APIInfoImpl: topology.APIInfoImpl, Topology: 
topologies[0].(tc.Topology)}
-       if userErr, sysErr, errCode := topology.setDescription(); userErr != 
nil || sysErr != nil {
-               return userErr, sysErr, errCode
-       }
 
        if err := oldTopology.removeParents(); err != nil {
                return nil, err, http.StatusInternalServerError
@@ -624,6 +625,9 @@ func (topology *TOTopology) Update(h http.Header) (error, 
error, int) {
                        return nil, err, http.StatusInternalServerError
                }
        }
+       if userErr, sysErr, errCode := topology.setTopologyDetails(); userErr 
!= nil || sysErr != nil {
+               return userErr, sysErr, errCode
+       }
        if userErr, sysErr, errCode := topology.addNodes(); userErr != nil || 
sysErr != nil {
                return userErr, sysErr, errCode
        }
@@ -781,8 +785,9 @@ WHERE tcp.child IN
 func updateQuery() string {
        query := `
 UPDATE topology t SET
-description = $1
-WHERE t.name = $2
+name = $1,
+description = $2
+WHERE t.name = $3
 RETURNING t.name, t.description, t.last_updated
 `
        return query
diff --git a/traffic_portal/app/src/common/api/TopologyService.js 
b/traffic_portal/app/src/common/api/TopologyService.js
index 59d1fe0..7deddba 100644
--- a/traffic_portal/app/src/common/api/TopologyService.js
+++ b/traffic_portal/app/src/common/api/TopologyService.js
@@ -42,8 +42,8 @@ var TopologyService = function($http, ENV, locationUtils, 
messageModel) {
                );
        };
 
-       this.updateTopology = function(topology) {
-               return $http.put(ENV.api['root'] + 'topologies', topology, { 
params: { name: topology.name } }).then(
+       this.updateTopology = function(topology, currentName) {
+               return $http.put(ENV.api['root'] + 'topologies', topology, { 
params: { name: currentName } }).then(
                        function(result) {
                                return result;
                        },
diff --git 
a/traffic_portal/app/src/common/modules/form/topology/edit/FormEditTopologyController.js
 
b/traffic_portal/app/src/common/modules/form/topology/edit/FormEditTopologyController.js
index c0fcc75..ff8e772 100644
--- 
a/traffic_portal/app/src/common/modules/form/topology/edit/FormEditTopologyController.js
+++ 
b/traffic_portal/app/src/common/modules/form/topology/edit/FormEditTopologyController.js
@@ -37,11 +37,12 @@ var FormEditTopologyController = function(topologies, 
cacheGroups, $scope, $cont
                saveLabel: 'Update'
        };
 
-       $scope.save = function(name, description, topologyTree) {
-               let normalizedTopology = 
topologyUtils.getNormalizedTopology(name, description, topologyTree);
-               topologyService.updateTopology(normalizedTopology).
+       $scope.save = function(currentName, newName, description, topologyTree) 
{
+               let normalizedTopology = 
topologyUtils.getNormalizedTopology(newName, description, topologyTree);
+               topologyService.updateTopology(normalizedTopology, currentName).
                        then(function(result) {
-                               messageModel.setMessages(result.data.alerts, 
false);
+                               messageModel.setMessages(result.data.alerts, 
true);
+                               
locationUtils.navigateToPath('/topologies/edit?name=' + 
result.data.response.name);
                        });
        };
 
diff --git 
a/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html 
b/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html
index da35785..98ed62e 100644
--- a/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html
+++ b/traffic_portal/app/src/common/modules/form/topology/form.topology.tpl.html
@@ -62,7 +62,7 @@ under the License.
             <div class="form-group" ng-class="{'has-error': 
hasError(topologyForm.name), 'has-feedback': hasError(topologyForm.name)}">
                 <label class="control-label col-md-2 col-sm-2 col-xs-12">Name 
*</label>
                 <div class="col-md-10 col-sm-10 col-xs-12">
-                    <input name="name" type="text" class="form-control" 
ng-model="topology.name" ng-disabled="!settings.isNew" 
pattern="^[a-zA-Z0-9\-_]+$" required autofocus>
+                    <input name="name" type="text" class="form-control" 
ng-model="topology.name" pattern="^[a-zA-Z0-9\-_]+$" required autofocus>
                     <small class="input-error" 
ng-show="hasPropertyError(topologyForm.name, 'required')">Required</small>
                     <small class="input-error" 
ng-show="hasPropertyError(topologyForm.name, 'pattern')">Alphanumeric, dash, or 
underscore characters only</small>
                     <span ng-show="hasError(topologyForm.name)" 
class="form-control-feedback"><i class="fa fa-times"></i></span>
@@ -87,7 +87,8 @@ under the License.
             </div>
             <div class="modal-footer">
                 <button type="button" class="btn btn-danger" 
ng-show="!settings.isNew" ng-click="confirmDelete(topology)">Delete</button>
-                <button type="button" class="btn btn-success" 
ng-disabled="topologyForm.$pristine || topologyForm.$invalid" 
ng-click="save(topology.name, topology.description, 
topologyTree)">{{settings.saveLabel}}</button>
+                <button ng-if="settings.isNew" type="button" class="btn 
btn-success" ng-disabled="topologyForm.$pristine || topologyForm.$invalid" 
ng-click="save(topology.name, topology.description, 
topologyTree)">{{settings.saveLabel}}</button>
+                <button ng-if="!settings.isNew" type="button" class="btn 
btn-success" ng-disabled="topologyForm.$pristine || topologyForm.$invalid" 
ng-click="save(topologyName, topology.name, topology.description, 
topologyTree)">{{settings.saveLabel}}</button>
             </div>
         </form>
     </div>
diff --git a/traffic_portal/app/src/common/service/utils/TopologyUtils.js 
b/traffic_portal/app/src/common/service/utils/TopologyUtils.js
index 086bea2..09e6288 100644
--- a/traffic_portal/app/src/common/service/utils/TopologyUtils.js
+++ b/traffic_portal/app/src/common/service/utils/TopologyUtils.js
@@ -76,16 +76,16 @@ var TopologyUtils = function() {
                Object.keys(all).forEach(function (guid) {
                        let item = all[guid];
                        if (!('children' in item)) {
-                               item.children = []
+                               item.children = [];
                        }
                        if (item.parents.length === 0) {
                                item.parent = { name: '', type: '' };
                                item.secParent = { name: '', type: '' };
-                               roots.push(item)
+                               roots.push(item);
                        } else if (item.parents[0] in all) {
-                               let p = all[item.parents[0]]
+                               let p = all[item.parents[0]];
                                if (!('children' in p)) {
-                                       p.children = []
+                                       p.children = [];
                                }
                                p.children.push(item);
                                // add parent to each node
diff --git a/traffic_portal/test/end_to_end/topologies/topologies-spec.js 
b/traffic_portal/test/end_to_end/topologies/topologies-spec.js
index 548e47d..1fb0bf6 100644
--- a/traffic_portal/test/end_to_end/topologies/topologies-spec.js
+++ b/traffic_portal/test/end_to_end/topologies/topologies-spec.js
@@ -77,7 +77,6 @@ describe('Traffic Portal Topologies Test Suite', function() {
                        });
                }).get(0).click();
                expect(pageData.updateButton.isEnabled()).toBe(false);
-               expect(pageData.name.getAttribute('disabled')).toBe('true');
                pageData.description.clear().then(function() {
                        pageData.description.sendKeys("Updated description");
                });

Reply via email to