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

mattjackson 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 f2ee630  POST request to /api/4.0/phys_locations accepts mismatch 
values for regionName (#6269)
f2ee630 is described below

commit f2ee630cd3e6cdd67dcc44cc2de810e419d7c573
Author: Srijeet Chatterjee <[email protected]>
AuthorDate: Tue Oct 19 14:46:05 2021 -0500

    POST request to /api/4.0/phys_locations accepts mismatch values for 
regionName (#6269)
    
    * check for mismatch values for regionName in POST and PUT /phys_locations
    
    * modify update prerequisites
---
 CHANGELOG.md                                       |  1 +
 traffic_ops/testing/api/v4/phys_locations_test.go  | 50 ++++++++++++++++++++++
 .../traffic_ops_golang/dbhelpers/db_helpers.go     | 12 ++++++
 traffic_ops/traffic_ops_golang/login/register.go   |  1 -
 .../physlocation/phys_locations.go                 | 27 +++++++++++-
 traffic_ops/v4-client/phys_location.go             |  2 +-
 6 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 47a9fa0..6784a37 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@ The format is based on [Keep a 
Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#6125](https://github.com/apache/trafficcontrol/issues/6125) - Fix 
`/cdns/{name}/federations?id=#` to search for CDN.
 - [#6255](https://github.com/apache/trafficcontrol/issues/6255) - Unreadable 
Prod Mode CDN Notifications in Traffic Portal
 - [#6259](https://github.com/apache/trafficcontrol/issues/6259) - Traffic 
Portal No Longer Allows Spaces in Server Object "Router Port Name"
+- [#6175](https://github.com/apache/trafficcontrol/issues/6175) - POST request 
to /api/4.0/phys_locations accepts mismatch values for regionName.
 
 ### Changed
 - Updated `t3c` to request less unnecessary deliveryservice-server assignment 
and invalidation jobs data via new query params supported by Traffic Ops
diff --git a/traffic_ops/testing/api/v4/phys_locations_test.go 
b/traffic_ops/testing/api/v4/phys_locations_test.go
index c787689..727aca8 100644
--- a/traffic_ops/testing/api/v4/phys_locations_test.go
+++ b/traffic_ops/testing/api/v4/phys_locations_test.go
@@ -26,6 +26,7 @@ import (
        "time"
 
        "github.com/apache/trafficcontrol/lib/go-rfc"
+       "github.com/apache/trafficcontrol/lib/go-tc"
        client "github.com/apache/trafficcontrol/traffic_ops/v4-client"
 )
 
@@ -50,6 +51,7 @@ func TestPhysLocations(t *testing.T) {
                header.Set(rfc.IfMatch, etag)
                UpdateTestPhysLocationsWithHeaders(t, header)
                GetTestPaginationSupportPhysLocation(t)
+               CreatePhysLocationWithMismatchedRegionNameAndID(t)
        })
 }
 
@@ -276,6 +278,54 @@ func DeleteTestPhysLocations(t *testing.T) {
        }
 }
 
+func CreatePhysLocationWithMismatchedRegionNameAndID(t *testing.T) {
+       resp, _, err := TOSession.GetRegions(client.NewRequestOptions())
+       if err != nil {
+               t.Errorf("Unexpected error getting regions: %v - alerts: %+v", 
err, resp.Alerts)
+       }
+       if len(resp.Response) < 2 {
+               t.Fatalf("expected at least two regions to be returned, but got 
none")
+       }
+       physLocation := tc.PhysLocation{
+               Address:    "100 blah lane",
+               City:       "foo",
+               Comments:   "comment",
+               Email:      "[email protected]",
+               Name:       "testPhysicalLocation",
+               Phone:      "111-222-3333",
+               RegionID:   resp.Response[0].ID,
+               RegionName: resp.Response[1].Name,
+               ShortName:  "testLocation1",
+               State:      "CO",
+               Zip:        "80602",
+       }
+       _, _, err = TOSession.CreatePhysLocation(physLocation, 
client.NewRequestOptions())
+       if err == nil {
+               t.Fatalf("expected an error about mismatched region name and 
ID, but got nothing")
+       }
+
+       physLocation.RegionName = resp.Response[0].Name
+       _, _, err = TOSession.CreatePhysLocation(physLocation, 
client.NewRequestOptions())
+       if err != nil {
+               t.Fatalf("expected no error while creating phys location, but 
got %v", err)
+       }
+
+       opts := client.NewRequestOptions()
+       opts.QueryParameters.Set("name", "testPhysicalLocation")
+       response, _, err := TOSession.GetPhysLocations(opts)
+       if err != nil {
+               t.Fatalf("cannot get Physical Location by name 
'testPhysicalLocation': %v - alerts: %+v", err, resp.Alerts)
+       }
+       if len(response.Response) != 1 {
+               t.Fatalf("Expected exactly one Physical Location to exist with 
name 'testPhysicalLocation', found: %d", len(resp.Response))
+       }
+
+       _, _, err = TOSession.DeletePhysLocation(response.Response[0].ID, 
client.NewRequestOptions())
+       if err != nil {
+               t.Errorf("error deleteing physical location 
'testPhysicalLocation': %v", err)
+       }
+}
+
 func GetTestPaginationSupportPhysLocation(t *testing.T) {
        opts := client.NewRequestOptions()
        opts.QueryParameters.Set("orderby", "id")
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go 
b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index 2e7128b..aef6e3a 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -1727,3 +1727,15 @@ func GetCDNNameDomain(cdnID int, tx *sql.Tx) (string, 
string, error) {
        }
        return cdnName, cdnDomain, nil
 }
+
+// GetRegionNameFromID returns the name of the region associated with the 
supplied ID.
+func GetRegionNameFromID(tx *sql.Tx, regionID int) (string, bool, error) {
+       var regionName string
+       if err := tx.QueryRow(`SELECT name FROM region WHERE id = $1`, 
regionID).Scan(&regionName); err != nil {
+               if errors.Is(err, sql.ErrNoRows) {
+                       return regionName, false, nil
+               }
+               return regionName, false, fmt.Errorf("querying region name from 
ID: %w", err)
+       }
+       return regionName, true, nil
+}
diff --git a/traffic_ops/traffic_ops_golang/login/register.go 
b/traffic_ops/traffic_ops_golang/login/register.go
index a10d7e4..8f664a3 100644
--- a/traffic_ops/traffic_ops_golang/login/register.go
+++ b/traffic_ops/traffic_ops_golang/login/register.go
@@ -225,7 +225,6 @@ func RegisterUser(w http.ResponseWriter, r *http.Request) {
        } else {
                req.Email = reqV4.Email
                req.TenantID = reqV4.TenantID
-               dbhelpers.GetRoleIDFromName(tx, reqV4.Role)
                roleID, ok, err := dbhelpers.GetRoleIDFromName(tx, reqV4.Role)
                if err != nil {
                        api.HandleErr(w, r, tx, http.StatusInternalServerError, 
nil, fmt.Errorf("error fetching ID from role name: %w", err))
diff --git a/traffic_ops/traffic_ops_golang/physlocation/phys_locations.go 
b/traffic_ops/traffic_ops_golang/physlocation/phys_locations.go
index da8922d..d6fbd85 100644
--- a/traffic_ops/traffic_ops_golang/physlocation/phys_locations.go
+++ b/traffic_ops/traffic_ops_golang/physlocation/phys_locations.go
@@ -20,6 +20,8 @@ package physlocation
  */
 
 import (
+       "errors"
+       "fmt"
        "net/http"
        "strconv"
        "time"
@@ -116,9 +118,30 @@ JOIN region r ON pl.region = r.id ` + where + orderBy + 
pagination +
        select max(last_updated) as t from last_deleted l where 
l.table_name='phys_location') as res`
 }
 
+// MatchRegionNameAndID checks to see if the supplied region name and ID in 
the phys_location body correspond to each other.
+func (pl *TOPhysLocation) MatchRegionNameAndID() (error, error, int) {
+       if pl.RegionName != nil {
+               regionName, ok, err := 
dbhelpers.GetRegionNameFromID(pl.APIInfo().Tx.Tx, *pl.RegionID)
+               if err != nil {
+                       return nil, fmt.Errorf("error fetching name from region 
ID: %w", err), http.StatusInternalServerError
+               } else if !ok {
+                       return errors.New("no such region"), nil, 
http.StatusNotFound
+               }
+               if regionName != *pl.RegionName {
+                       return errors.New("region name and ID do not match"), 
nil, http.StatusBadRequest
+               }
+       }
+       return nil, nil, http.StatusOK
+}
+
 func (pl *TOPhysLocation) Update(h http.Header) (error, error, int) { return 
api.GenericUpdate(h, pl) }
-func (pl *TOPhysLocation) Create() (error, error, int)              { return 
api.GenericCreate(pl) }
-func (pl *TOPhysLocation) Delete() (error, error, int)              { return 
api.GenericDelete(pl) }
+func (pl *TOPhysLocation) Create() (error, error, int) {
+       if userErr, sysErr, statusCode := pl.MatchRegionNameAndID(); userErr != 
nil || sysErr != nil {
+               return userErr, sysErr, statusCode
+       }
+       return api.GenericCreate(pl)
+}
+func (pl *TOPhysLocation) Delete() (error, error, int) { return 
api.GenericDelete(pl) }
 
 func selectQuery() string {
        return `
diff --git a/traffic_ops/v4-client/phys_location.go 
b/traffic_ops/v4-client/phys_location.go
index 5f43dea..c9c94f3 100644
--- a/traffic_ops/v4-client/phys_location.go
+++ b/traffic_ops/v4-client/phys_location.go
@@ -30,7 +30,7 @@ func (to *Session) CreatePhysLocation(pl tc.PhysLocation, 
opts RequestOptions) (
        if pl.RegionID == 0 && pl.RegionName != "" {
                regionOpts := NewRequestOptions()
                regionOpts.QueryParameters.Set("name", pl.RegionName)
-               regions, reqInf, err := to.GetRegions(opts)
+               regions, reqInf, err := to.GetRegions(regionOpts)
                if err != nil {
                        err = fmt.Errorf("resolving Region name '%s' to an ID", 
pl.RegionName)
                        return regions.Alerts, reqInf, err

Reply via email to