Skip to content

Commit

Permalink
many: switch to use http numeric statuses as agreed (#3486)
Browse files Browse the repository at this point in the history
- replace http.Status* with well-known numeric values in tests
- store: replace http.Status* with well-known numeric values and dont' convert 401 into please buy, atm is just confusing
- replace http.Status* with well-known numeric values in retry checks
- replace http.Status* with well-known numeric values in fake services
- run-check static check
- replace http.Status* with well-known numeric values in client and daemon
  • Loading branch information
pedronis committed Jun 15, 2017
1 parent cd8b523 commit 1383502
Show file tree
Hide file tree
Showing 24 changed files with 206 additions and 191 deletions.
3 changes: 1 addition & 2 deletions client/asserts.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"bytes"
"fmt"
"io"
"net/http"
"net/url"
"strconv"

Expand Down Expand Up @@ -59,7 +58,7 @@ func (client *Client) Known(assertTypeName string, headers map[string]string) ([
return nil, fmt.Errorf("failed to query assertions: %v", err)
}
defer response.Body.Close()
if response.StatusCode != http.StatusOK {
if response.StatusCode != 200 {
return nil, parseError(response)
}

Expand Down
6 changes: 3 additions & 3 deletions client/asserts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (cs *clientSuite) TestClientAssertsHttpError(c *C) {
}

func (cs *clientSuite) TestClientAssertsJSONError(c *C) {
cs.status = http.StatusBadRequest
cs.status = 400
cs.header = http.Header{}
cs.header.Add("Content-type", "application/json")
cs.rsp = `{
Expand Down Expand Up @@ -129,7 +129,7 @@ func (cs *clientSuite) TestClientAssertsNoAssertions(c *C) {
cs.header = http.Header{}
cs.header.Add("X-Ubuntu-Assertions-Count", "0")
cs.rsp = ""
cs.status = http.StatusOK
cs.status = 200
a, err := cs.cli.Known("snap-revision", nil)
c.Assert(err, IsNil)
c.Check(a, HasLen, 0)
Expand All @@ -139,7 +139,7 @@ func (cs *clientSuite) TestClientAssertsMissingAssertions(c *C) {
cs.header = http.Header{}
cs.header.Add("X-Ubuntu-Assertions-Count", "4")
cs.rsp = ""
cs.status = http.StatusOK
cs.status = 200
_, err := cs.cli.Known("snap-build", nil)
c.Assert(err, ErrorMatches, "response did not have the expected number of assertions")
}
2 changes: 1 addition & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ func (client *Client) doAsync(method, path string, query url.Values, headers map
if rsp.Type != "async" {
return "", fmt.Errorf("expected async response for %q on %q, got %q", method, path, rsp.Type)
}
if rsp.StatusCode != http.StatusAccepted {
if rsp.StatusCode != 202 {
return "", fmt.Errorf("operation not accepted")
}
if rsp.Change == "" {
Expand Down
2 changes: 1 addition & 1 deletion client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (cs *clientSuite) SetUpTest(c *C) {
cs.rsps = nil
cs.req = nil
cs.header = nil
cs.status = http.StatusOK
cs.status = 200
cs.doCalls = 0

dirs.SetRootDir(c.MkDir())
Expand Down
3 changes: 1 addition & 2 deletions client/icons.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package client
import (
"fmt"
"io/ioutil"
"net/http"
"regexp"
)

Expand All @@ -42,7 +41,7 @@ func (c *Client) Icon(pkgID string) (*Icon, error) {
}
defer response.Body.Close()

if response.StatusCode != http.StatusOK {
if response.StatusCode != 200 {
return nil, fmt.Errorf("%s: Not Found", errPrefix)
}

Expand Down
2 changes: 1 addition & 1 deletion client/icons_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (cs *clientSuite) TestClientIconHttpError(c *C) {
}

func (cs *clientSuite) TestClientIconResponseNotFound(c *C) {
cs.status = http.StatusNotFound
cs.status = 404
_, err := cs.cli.Icon(pkgID)
c.Assert(err, ErrorMatches, `.*Not Found`)
}
Expand Down
8 changes: 4 additions & 4 deletions cmd/snap/cmd_snap_op_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (t *snapOpTestServer) handle(w http.ResponseWriter, r *http.Request) {
case 0:
t.checker(r)
t.c.Check(r.Method, check.Equals, "POST")
w.WriteHeader(http.StatusAccepted)
w.WriteHeader(202)
fmt.Fprintln(w, `{"type":"async", "change": "42", "status-code": 202}`)
case 1:
t.c.Check(r.Method, check.Equals, "GET")
Expand Down Expand Up @@ -768,7 +768,7 @@ func (s *SnapOpSuite) TestTryClassic(c *check.C) {
func (s *SnapOpSuite) TestTryNoSnapDirErrors(c *check.C) {
s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) {
c.Check(r.Method, check.Equals, "POST")
w.WriteHeader(http.StatusAccepted)
w.WriteHeader(202)
fmt.Fprintln(w, `
{
"type": "error",
Expand Down Expand Up @@ -895,7 +895,7 @@ func (s *SnapOpSuite) TestRemoveMany(c *check.C) {
})

c.Check(r.Method, check.Equals, "POST")
w.WriteHeader(http.StatusAccepted)
w.WriteHeader(202)
fmt.Fprintln(w, `{"type":"async", "change": "42", "status-code": 202}`)
case 1:
c.Check(r.Method, check.Equals, "GET")
Expand Down Expand Up @@ -947,7 +947,7 @@ func (s *SnapOpSuite) TestInstallMany(c *check.C) {
})

c.Check(r.Method, check.Equals, "POST")
w.WriteHeader(http.StatusAccepted)
w.WriteHeader(202)
fmt.Fprintln(w, `{"type":"async", "change": "42", "status-code": 202}`)
case 1:
c.Check(r.Method, check.Equals, "GET")
Expand Down
24 changes: 12 additions & 12 deletions daemon/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response {
Kind: errorKindInvalidAuthData,
Value: map[string][]string{"email": {"invalid"}},
},
Status: http.StatusBadRequest,
Status: 400,
}, nil)
}

Expand All @@ -329,7 +329,7 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response {
Kind: errorKindTwoFactorRequired,
Message: err.Error(),
},
Status: http.StatusUnauthorized,
Status: 401,
}, nil)
case store.Err2faFailed:
return SyncResponse(&resp{
Expand All @@ -338,7 +338,7 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response {
Kind: errorKindTwoFactorFailed,
Message: err.Error(),
},
Status: http.StatusUnauthorized,
Status: 401,
}, nil)
default:
switch err := err.(type) {
Expand All @@ -350,7 +350,7 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response {
Kind: errorKindInvalidAuthData,
Value: err,
},
Status: http.StatusBadRequest,
Status: 400,
}, nil)
case store.PasswordPolicyError:
return SyncResponse(&resp{
Expand All @@ -360,7 +360,7 @@ func loginUser(c *Command, r *http.Request, user *auth.UserState) Response {
Kind: errorKindPasswordPolicy,
Value: err,
},
Status: http.StatusUnauthorized,
Status: 401,
}, nil)
}
return Unauthorized(err.Error())
Expand Down Expand Up @@ -1126,7 +1126,7 @@ func (inst *snapInstruction) errToResponse(err error) Response {
return SyncResponse(&resp{
Type: ResponseTypeError,
Result: &errorResult{Message: err.Error(), Kind: kind},
Status: http.StatusBadRequest,
Status: 400,
}, nil)
}

Expand Down Expand Up @@ -1208,7 +1208,7 @@ func trySnap(c *Command, r *http.Request, user *auth.UserState, trydir string, f
Message: err.Error(),
Kind: errorKindNotSnap,
},
Status: http.StatusBadRequest,
Status: 400,
}, nil)
}
if err != nil {
Expand Down Expand Up @@ -1722,7 +1722,7 @@ func doAssert(c *Command, r *http.Request, user *auth.UserState) Response {
// TODO: what more info do we want to return on success?
return &resp{
Type: ResponseTypeSync,
Status: http.StatusOK,
Status: 200,
}
}

Expand Down Expand Up @@ -2212,7 +2212,7 @@ func convertBuyError(err error) Response {
Message: err.Error(),
Kind: errorKindLoginRequired,
},
Status: http.StatusBadRequest,
Status: 400,
}, nil)
case store.ErrTOSNotAccepted:
return SyncResponse(&resp{
Expand All @@ -2221,7 +2221,7 @@ func convertBuyError(err error) Response {
Message: err.Error(),
Kind: errorKindTermsNotAccepted,
},
Status: http.StatusBadRequest,
Status: 400,
}, nil)
case store.ErrNoPaymentMethods:
return SyncResponse(&resp{
Expand All @@ -2230,7 +2230,7 @@ func convertBuyError(err error) Response {
Message: err.Error(),
Kind: errorKindNoPaymentMethods,
},
Status: http.StatusBadRequest,
Status: 400,
}, nil)
case store.ErrPaymentDeclined:
return SyncResponse(&resp{
Expand All @@ -2239,7 +2239,7 @@ func convertBuyError(err error) Response {
Message: err.Error(),
Kind: errorKindPaymentDeclined,
},
Status: http.StatusBadRequest,
Status: 400,
}, nil)
default:
return InternalError("%v", err)
Expand Down
Loading

0 comments on commit 1383502

Please sign in to comment.