Skip to content

Commit

Permalink
APIInfo refactor (#7902)
Browse files Browse the repository at this point in the history 
* Rename api.APIInfo -> api.Info

* Move Info structures and related functionality to dedicated files

I also added some tests for some Info methods/related functions that
didn't have any tests, and I moved the APIInfoImpl type to the only
place where it's actually used.

* Fixup references to the old name for api.Info in the docs

* Remove lingering references to the old name for api.Info
  • Loading branch information
@ocket8888
ocket8888 committed Jan 19, 2024
1 parent 5504398 commit d5b3f30
Show file tree
Hide file tree
Showing 61 changed files with 1,413 additions and 720 deletions.
2 changes: 1 addition & 1 deletion docs/source/_ext/atc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ def to_godoc_role(
Example:
API endpoints make heavy use of :to-godoc:`api#APIInfo` objects.
API endpoints make heavy use of :to-godoc:`api#Info` objects.
"""
if options is None:
options = {}
Expand Down
2 changes: 1 addition & 1 deletion docs/source/development/documentation_guidelines.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,4 @@ godoc
atc-godoc
This is provided for convenience, and is identical to ``:godoc:`` except that it is assumed to be relative to the Apache Traffic Control project. For example, ``:atc-godoc:`lib/go-rfc.MimeType.Quality``` renders as :atc-godoc:`lib/go-rfc.MimeType.Quality`.
to-godoc
This is provided for convenience, and is identical to ``:godoc:`` except that it is assumed to be relative to the :atc-godoc:`traffic_ops/traffic_ops_golang` package. For example, ``:to-godoc:`api.APIInfo``` renders as :to-godoc:`api.APIInfo`.
This is provided for convenience, and is identical to ``:godoc:`` except that it is assumed to be relative to the :atc-godoc:`traffic_ops/traffic_ops_golang` package. For example, ``:to-godoc:`api.Info``` renders as :to-godoc:`api.Info`.
6 changes: 3 additions & 3 deletions docs/source/development/traffic_ops.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -694,12 +694,12 @@ The "Generic 'CRUDer'", as it's known, is a pattern of API endpoint development

.. seealso:: The :to-godoc:`api.GenericCreate`, :to-godoc:`api.GenericDelete`, :to-godoc:`api.GenericRead`, and :to-godoc:`api.GenericUpdate` helpers are often used to provide the default operations of creating, deleting, reading, and updating objects, respectively. When the API endpoint being written is only meant to perform these basic operations on an object or objects stored in the database, these should be totally sufficient.

This method offers a lot of functionality "out-of-the-box" as compared to the `APIInfo`_ method, but because of that is also restrictive. For example, it is not possible to write an endpoint that returns data not encoded as JSON using this method. That's an uncommon use-case, but not unheard-of.
This method offers a lot of functionality "out-of-the-box" as compared to the `API Info`_ method, but because of that is also restrictive. For example, it is not possible to write an endpoint that returns data not encoded as JSON using this method. That's an uncommon use-case, but not unheard-of.

This method is best used for basic creation, reading, update, and deletion operations performed on simple objects with no structural differences across API versions.

APIInfo
"""""""
API Info
""""""""
Endpoint handlers can also be defined by simply implementing the :godoc:`net/http.HandlerFunc` interface. The :godoc:`net/http.Request` reference passed into such handlers provides identifying information for the authenticated user (where applicable) in its context.

To easily obtain the information needed to identify a user and their associated permissions, as well as server configuration information and a database transaction handle, authors should use the :to-godoc:`api.NewInfo` function which will return all of that information in a single structure as well as any errors encountered during the process and an appropriate HTTP response code in case of such errors.
Expand Down
2 changes: 1 addition & 1 deletion traffic_ops/traffic_ops_golang/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func CreateV15(w http.ResponseWriter, r *http.Request) {
api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice creation was successful.", []tc.DeliveryServiceNullableV15{*res})
}

func createV15(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, reqDS tc.DeliveryServiceNullableV15) *tc.DeliveryServiceNullableV15 {
func createV15(w http.ResponseWriter, r *http.Request, inf *api.Info, reqDS tc.DeliveryServiceNullableV15) *tc.DeliveryServiceNullableV15 {
...
}
```
Expand Down
371 changes: 9 additions & 362 deletions traffic_ops/traffic_ops_golang/api/api.go
View file

Large diffs are not rendered by default.

206 changes: 0 additions & 206 deletions traffic_ops/traffic_ops_golang/api/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,11 @@ import (
"errors"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"time"

"github.com/lib/pq"

"github.com/apache/trafficcontrol/v8/lib/go-rfc"
"github.com/apache/trafficcontrol/v8/lib/go-tc"
)

Expand Down Expand Up @@ -266,205 +262,3 @@ func TestParseRestrictFKConstraint(t *testing.T) {
})
}
}

func TestAPIInfo_WriteOKResponse(t *testing.T) {
w := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodGet, "/", nil)

inf := APIInfo{
request: r,
w: w,
}
code, userErr, sysErr := inf.WriteOKResponse("test")
if code != http.StatusOK {
t.Errorf("WriteOKResponse should return a %d %s code, got: %d %s", http.StatusOK, http.StatusText(http.StatusOK), code, http.StatusText(code))
}
if userErr != nil {
t.Errorf("Unexpected user error: %v", userErr)
}
if sysErr != nil {
t.Errorf("Unexpected system error: %v", sysErr)
}

if w.Code != http.StatusOK {
t.Errorf("incorrect response status code; want: %d, got: %d", http.StatusOK, w.Code)
}
}
func TestAPIInfo_WriteOKResponseWithSummary(t *testing.T) {
w := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodGet, "/", nil)

inf := APIInfo{
request: r,
w: w,
}
code, userErr, sysErr := inf.WriteOKResponseWithSummary("test", 42)
if code != http.StatusOK {
t.Errorf("WriteOKResponseWithSummary should return a %d %s code, got: %d %s", http.StatusOK, http.StatusText(http.StatusOK), code, http.StatusText(code))
}
if userErr != nil {
t.Errorf("Unexpected user error: %v", userErr)
}
if sysErr != nil {
t.Errorf("Unexpected system error: %v", sysErr)
}

if w.Code != http.StatusOK {
t.Errorf("incorrect response status code; want: %d, got: %d", http.StatusOK, w.Code)
}
}
func TestAPIInfo_WriteNotModifiedResponse(t *testing.T) {
w := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodGet, "/", nil)

inf := APIInfo{
request: r,
w: w,
}
code, userErr, sysErr := inf.WriteNotModifiedResponse(time.Time{})
if code != http.StatusNotModified {
t.Errorf("WriteNotModifiedResponse should return a %d %s code, got: %d %s", http.StatusNotModified, http.StatusText(http.StatusNotModified), code, http.StatusText(code))
}
if userErr != nil {
t.Errorf("Unexpected user error: %v", userErr)
}
if sysErr != nil {
t.Errorf("Unexpected system error: %v", sysErr)
}

if w.Code != http.StatusNotModified {
t.Errorf("incorrect response status code; want: %d, got: %d", http.StatusNotModified, w.Code)
}
}

func TestAPIInfo_WriteSuccessResponse(t *testing.T) {
w := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodGet, "/", nil)

inf := APIInfo{
request: r,
w: w,
}
code, userErr, sysErr := inf.WriteSuccessResponse("test", "quest")
if code != http.StatusOK {
t.Errorf("WriteSuccessResponse should return a %d %s code, got: %d %s", http.StatusOK, http.StatusText(http.StatusOK), code, http.StatusText(code))
}
if userErr != nil {
t.Errorf("Unexpected user error: %v", userErr)
}
if sysErr != nil {
t.Errorf("Unexpected system error: %v", sysErr)
}

if w.Code != http.StatusOK {
t.Errorf("incorrect response status code; want: %d, got: %d", http.StatusOK, w.Code)
}
var alerts tc.Alerts
if err := json.NewDecoder(w.Body).Decode(&alerts); err != nil {
t.Fatalf("couldn't decode response body: %v", err)
}

if len(alerts.Alerts) != 1 {
t.Fatalf("expected exactly one alert; got: %d", len(alerts.Alerts))
}
alert := alerts.Alerts[0]
if alert.Level != tc.SuccessLevel.String() {
t.Errorf("Incorrect alert level; want: %s, got: %s", tc.SuccessLevel, alert.Level)
}
if alert.Text != "quest" {
t.Errorf("Incorrect alert text; want: 'quest', got: '%s'", alert.Text)
}
}

func TestAPIInfo_WriteCreatedResponse(t *testing.T) {
w := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodPost, "/", nil)

inf := APIInfo{
request: r,
Version: &Version{Major: 420, Minor: 9001},
w: w,
}
code, userErr, sysErr := inf.WriteCreatedResponse("test", "quest", "mypath")
if code != http.StatusCreated {
t.Errorf("WriteCreatedResponse should return a %d %s code, got: %d %s", http.StatusCreated, http.StatusText(http.StatusCreated), code, http.StatusText(code))
}
if userErr != nil {
t.Errorf("Unexpected user error: %v", userErr)
}
if sysErr != nil {
t.Errorf("Unexpected system error: %v", sysErr)
}

if w.Code != http.StatusCreated {
t.Errorf("incorrect response status code; want: %d, got: %d", http.StatusCreated, w.Code)
}
if locHdr := w.Header().Get(rfc.Location); locHdr != "/api/420.9001/mypath" {
t.Errorf("incorrect '%s' header value; want: '/api/420.9001/mypath', got: '%s'", rfc.Location, locHdr)
}
var alerts tc.Alerts
if err := json.NewDecoder(w.Body).Decode(&alerts); err != nil {
t.Fatalf("couldn't decode response body: %v", err)
}

if len(alerts.Alerts) != 1 {
t.Fatalf("expected exactly one alert; got: %d", len(alerts.Alerts))
}
alert := alerts.Alerts[0]
if alert.Level != tc.SuccessLevel.String() {
t.Errorf("Incorrect alert level; want: %s, got: %s", tc.SuccessLevel, alert.Level)
}
if alert.Text != "quest" {
t.Errorf("Incorrect alert text; want: 'quest', got: '%s'", alert.Text)
}
}

func TestAPIInfo_RequestHeaders(t *testing.T) {
r := httptest.NewRequest(http.MethodGet, "/", nil)
r.Header.Set("test", "quest")

inf := APIInfo{
request: r,
}
testHdr := inf.RequestHeaders().Get("test")
if testHdr != "quest" {
t.Errorf("should have retrieved the 'test' header (expected value: 'quest'), but found that header to have value: '%s'", testHdr)
}

}

func TestAPIInfo_SetLastModified(t *testing.T) {
w := httptest.NewRecorder()
inf := APIInfo{w: w}
tm := time.Now().Truncate(time.Second).UTC()
inf.SetLastModified(tm)

wLMHdr := w.Header().Get(rfc.LastModified)
lm, err := time.Parse(time.RFC1123, wLMHdr)
if err != nil {
t.Fatalf("Failed to parse the response writer's '%s' header as an RFC1123 timestamp: %v", rfc.LastModified, err)
}

// For unknown reasons, our API always adds a second to the truncated time
// value for LastModified headers. I suspect it's a poor attempt at rounding
// - for which the `Round` method ought to be used instead.
if expected := tm.Add(time.Second); lm != expected {
t.Errorf("Incorrect time set as '%s' header; want: %s, got: %s", rfc.LastModified, expected.Format(time.RFC3339Nano), lm.Format(time.RFC3339Nano))
}
}

func TestAPIInfo_DecodeBody(t *testing.T) {
inf := APIInfo{
request: httptest.NewRequest(http.MethodConnect, "/", strings.NewReader(`{"test": "quest"}`)),
}

var out struct {
Test string `json:"test"`
}
if err := inf.DecodeBody(&out); err != nil {
t.Fatalf("failed to decode body: %v", err)
}
if out.Test != "quest" {
t.Errorf(`incorrect request body parsed; want: {"test": "quest"}, got: {"test": "%s"}`, out.Test)
}
}
10 changes: 5 additions & 5 deletions traffic_ops/traffic_ops_golang/api/generic_crud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ import (

type GenericCreator interface {
GetType() string
APIInfo() *APIInfo
APIInfo() *Info
SetKeys(map[string]interface{})
SetLastUpdated(tc.TimeNoMod)
InsertQuery() string
}

type GenericReader interface {
GetType() string
APIInfo() *APIInfo
APIInfo() *Info
ParamColumns() map[string]dbhelpers.WhereColumnInfo
NewReadObj() interface{}
SelectQuery() string
Expand All @@ -54,22 +54,22 @@ type GenericReader interface {

type GenericUpdater interface {
GetType() string
APIInfo() *APIInfo
APIInfo() *Info
SetLastUpdated(tc.TimeNoMod)
UpdateQuery() string
GetLastUpdated() (*time.Time, bool, error)
}

type GenericDeleter interface {
GetType() string
APIInfo() *APIInfo
APIInfo() *Info
DeleteQuery() string
}

// GenericOptionsDeleter can use any key listed in DeleteKeyOptions() to delete a resource.
type GenericOptionsDeleter interface {
GetType() string
APIInfo() *APIInfo
APIInfo() *Info
DeleteKeyOptions() map[string]dbhelpers.WhereColumnInfo
DeleteQueryBase() string
}
Expand Down
Loading

0 comments on commit d5b3f30

Please sign in to comment.