From 0a841a49fddfa50355312ccdb3f9e0eb3522180c Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 26 Aug 2014 13:36:09 -0400 Subject: [PATCH] Naming of Reason constants is inconsistent Changed ReasonType to StatusReason (to match the field) and changed CauseReason to CauseType and renamed StatusCause.Reason to Type. --- pkg/api/errors/errors.go | 2 +- pkg/api/types.go | 61 +++++++++++++++++---------------- pkg/api/v1beta1/types.go | 59 +++++++++++++++---------------- pkg/apiserver/apiserver_test.go | 4 +-- pkg/apiserver/errors.go | 24 ++++++------- pkg/apiserver/errors_test.go | 28 +++++++-------- pkg/apiserver/operation.go | 2 +- 7 files changed, 91 insertions(+), 89 deletions(-) diff --git a/pkg/api/errors/errors.go b/pkg/api/errors/errors.go index e20d678eb65..149653e0226 100644 --- a/pkg/api/errors/errors.go +++ b/pkg/api/errors/errors.go @@ -25,7 +25,7 @@ import ( // ValidationErrorType is a machine readable value providing more detail about why // a field is invalid. These values are expected to match 1-1 with -// CauseReasonType in api/types.go. +// CauseType in api/types.go. type ValidationErrorType string const ( diff --git a/pkg/api/types.go b/pkg/api/types.go index f283b351dc6..b1dfaeb3846 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -368,7 +368,7 @@ type Status struct { // "failure" or "working" status. If this value is empty there // is no information available. A Reason clarifies an HTTP status // code but does not override it. - Reason ReasonType `json:"reason,omitempty" yaml:"reason,omitempty"` + Reason StatusReason `json:"reason,omitempty" yaml:"reason,omitempty"` // Extended data associated with the reason. Each reason may define its // own extended details. This field is optional and the data returned // is not guaranteed to conform to any schema except that defined by @@ -385,14 +385,14 @@ type Status struct { // and should assume that any attribute may be empty, invalid, or under // defined. type StatusDetails struct { - // The ID attribute of the resource associated with the status ReasonType + // The ID attribute of the resource associated with the status StatusReason // (when there is a single ID which can be described). ID string `json:"id,omitempty" yaml:"id,omitempty"` - // The kind attribute of the resource associated with the status ReasonType. + // The kind attribute of the resource associated with the status StatusReason. // On some operations may differ from the requested resource Kind. Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` - // The Causes array includes more details associated with the ReasonType - // failure. Not all ReasonTypes may provide detailed causes. + // The Causes array includes more details associated with the StatusReason + // failure. Not all StatusReasons may provide detailed causes. Causes []StatusCause `json:"causes,omitempty" yaml:"causes,omitempty"` } @@ -403,19 +403,19 @@ const ( StatusWorking = "working" ) -// ReasonType is an enumeration of possible failure causes. Each ReasonType +// StatusReason is an enumeration of possible failure causes. Each StatusReason // must map to a single HTTP status code, but multiple reasons may map // to the same HTTP status code. // TODO: move to apiserver -type ReasonType string +type StatusReason string const ( - // ReasonTypeUnknown means the server has declined to indicate a specific reason. + // StatusReasonUnknown means the server has declined to indicate a specific reason. // The details field may contain other information about this error. // Status code 500. - ReasonTypeUnknown ReasonType = "" + StatusReasonUnknown StatusReason = "" - // ReasonTypeWorking means the server is processing this request and will complete + // StatusReasonWorking means the server is processing this request and will complete // at a future time. // Details (optional): // "kind" string - the name of the resource being referenced ("operation" today) @@ -425,7 +425,7 @@ const ( // "Location" - HTTP header populated with a URL that can retrieved the final // status of this operation. // Status code 202 - ReasonTypeWorking ReasonType = "working" + StatusReasonWorking StatusReason = "working" // ResourceTypeNotFound means one or more resources required for this operation // could not be found. @@ -435,21 +435,21 @@ const ( // resource. // "id" string - the identifier of the missing resource // Status code 404 - ReasonTypeNotFound ReasonType = "not_found" + StatusReasonNotFound StatusReason = "not_found" - // ReasonTypeAlreadyExists means the resource you are creating already exists. + // StatusReasonAlreadyExists means the resource you are creating already exists. // Details (optional): // "kind" string - the kind attribute of the conflicting resource // "id" string - the identifier of the conflicting resource // Status code 409 - ReasonTypeAlreadyExists ReasonType = "already_exists" + StatusReasonAlreadyExists StatusReason = "already_exists" // ResourceTypeConflict means the requested update operation cannot be completed // due to a conflict in the operation. The client may need to alter the request. // Each resource may define custom details that indicate the nature of the // conflict. // Status code 409 - ReasonTypeConflict ReasonType = "conflict" + StatusReasonConflict StatusReason = "conflict" // ResourceTypeInvalid means the requested create or update operation cannot be // completed due to invalid data provided as part of the request. The client may @@ -462,7 +462,7 @@ const ( // provided resource that was invalid. The code, message, and // field attributes will be set. // Status code 422 - ReasonTypeInvalid ReasonType = "invalid" + StatusReasonInvalid StatusReason = "invalid" ) // StatusCause provides more information about an api.Status failure, including @@ -470,7 +470,7 @@ const ( type StatusCause struct { // A machine-readable description of the cause of the error. If this value is // empty there is no information available. - Reason CauseReasonType `json:"reason,omitempty" yaml:"reason,omitempty"` + Type CauseType `json:"reason,omitempty" yaml:"reason,omitempty"` // A human-readable description of the cause of the error. This field may be // presented as-is to a reader. Message string `json:"message,omitempty" yaml:"message,omitempty"` @@ -486,26 +486,27 @@ type StatusCause struct { Field string `json:"field,omitempty" yaml:"field,omitempty"` } -// CauseReasonType is a machine readable value providing more detail about why -// an operation failed. An operation may have multiple causes for a failure. -type CauseReasonType string +// CauseType is a machine readable value providing more detail about what +// occured in a status response. An operation may have multiple causes for a +// status (whether failure, success, or working). +type CauseType string const ( - // CauseReasonTypeFieldValueNotFound is used to report failure to find a requested value + // CauseTypeFieldValueNotFound is used to report failure to find a requested value // (e.g. looking up an ID). - CauseReasonTypeFieldValueNotFound CauseReasonType = "fieldValueNotFound" - // CauseReasonTypeFieldValueInvalid is used to report required values that are not + CauseTypeFieldValueNotFound CauseType = "fieldValueNotFound" + // CauseTypeFieldValueInvalid is used to report required values that are not // provided (e.g. empty strings, null values, or empty arrays). - CauseReasonTypeFieldValueRequired CauseReasonType = "fieldValueRequired" - // CauseReasonTypeFieldValueDuplicate is used to report collisions of values that must be + CauseTypeFieldValueRequired CauseType = "fieldValueRequired" + // CauseTypeFieldValueDuplicate is used to report collisions of values that must be // unique (e.g. unique IDs). - CauseReasonTypeFieldValueDuplicate CauseReasonType = "fieldValueDuplicate" - // CauseReasonTypeFieldValueInvalid is used to report malformed values (e.g. failed regex + CauseTypeFieldValueDuplicate CauseType = "fieldValueDuplicate" + // CauseTypeFieldValueInvalid is used to report malformed values (e.g. failed regex // match). - CauseReasonTypeFieldValueInvalid CauseReasonType = "fieldValueInvalid" - // CauseReasonTypeFieldValueNotSupported is used to report valid (as per formatting rules) + CauseTypeFieldValueInvalid CauseType = "fieldValueInvalid" + // CauseTypeFieldValueNotSupported is used to report valid (as per formatting rules) // values that can not be handled (e.g. an enumerated string). - CauseReasonTypeFieldValueNotSupported CauseReasonType = "fieldValueNotSupported" + CauseTypeFieldValueNotSupported CauseType = "fieldValueNotSupported" ) // ServerOp is an operation delivered to API clients. diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index c0969003421..74468ce3961 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -372,7 +372,7 @@ type Status struct { // "failure" or "working" status. If this value is empty there // is no information available. A Reason clarifies an HTTP status // code but does not override it. - Reason ReasonType `json:"reason,omitempty" yaml:"reason,omitempty"` + Reason StatusReason `json:"reason,omitempty" yaml:"reason,omitempty"` // Extended data associated with the reason. Each reason may define its // own extended details. This field is optional and the data returned // is not guaranteed to conform to any schema except that defined by @@ -389,14 +389,14 @@ type Status struct { // and should assume that any attribute may be empty, invalid, or under // defined. type StatusDetails struct { - // The ID attribute of the resource associated with the status ReasonType + // The ID attribute of the resource associated with the status StatusReason // (when there is a single ID which can be described). ID string `json:"id,omitempty" yaml:"id,omitempty"` - // The kind attribute of the resource associated with the status ReasonType. + // The kind attribute of the resource associated with the status StatusReason. // On some operations may differ from the requested resource Kind. Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` - // The Causes array includes more details associated with the ReasonType - // failure. Not all ReasonTypes may provide detailed causes. + // The Causes array includes more details associated with the StatusReason + // failure. Not all StatusReasons may provide detailed causes. Causes []StatusCause `json:"causes,omitempty" yaml:"causes,omitempty"` } @@ -407,19 +407,19 @@ const ( StatusWorking = "working" ) -// ReasonType is an enumeration of possible failure causes. Each ReasonType +// StatusReason is an enumeration of possible failure causes. Each StatusReason // must map to a single HTTP status code, but multiple reasons may map // to the same HTTP status code. // TODO: move to apiserver -type ReasonType string +type StatusReason string const ( - // ReasonTypeUnknown means the server has declined to indicate a specific reason. + // StatusReasonUnknown means the server has declined to indicate a specific reason. // The details field may contain other information about this error. // Status code 500. - ReasonTypeUnknown ReasonType = "" + StatusReasonUnknown StatusReason = "" - // ReasonTypeWorking means the server is processing this request and will complete + // StatusReasonWorking means the server is processing this request and will complete // at a future time. // Details (optional): // "kind" string - the name of the resource being referenced ("operation" today) @@ -429,7 +429,7 @@ const ( // "Location" - HTTP header populated with a URL that can retrieved the final // status of this operation. // Status code 202 - ReasonTypeWorking ReasonType = "working" + StatusReasonWorking StatusReason = "working" // ResourceTypeNotFound means one or more resources required for this operation // could not be found. @@ -439,21 +439,21 @@ const ( // resource. // "id" string - the identifier of the missing resource // Status code 404 - ReasonTypeNotFound ReasonType = "notFound" + StatusReasonNotFound StatusReason = "notFound" - // ReasonTypeAlreadyExists means the resource you are creating already exists. + // StatusReasonAlreadyExists means the resource you are creating already exists. // Details (optional): // "kind" string - the kind attribute of the conflicting resource // "id" string - the identifier of the conflicting resource // Status code 409 - ReasonTypeAlreadyExists ReasonType = "alreadyExists" + StatusReasonAlreadyExists StatusReason = "alreadyExists" // ResourceTypeConflict means the requested update operation cannot be completed // due to a conflict in the operation. The client may need to alter the request. // Each resource may define custom details that indicate the nature of the // conflict. // Status code 409 - ReasonTypeConflict ReasonType = "conflict" + StatusReasonConflict StatusReason = "conflict" ) // StatusCause provides more information about an api.Status failure, including @@ -461,7 +461,7 @@ const ( type StatusCause struct { // A machine-readable description of the cause of the error. If this value is // empty there is no information available. - Reason CauseReasonType `json:"reason,omitempty" yaml:"reason,omitempty"` + Type CauseType `json:"reason,omitempty" yaml:"reason,omitempty"` // A human-readable description of the cause of the error. This field may be // presented as-is to a reader. Message string `json:"message,omitempty" yaml:"message,omitempty"` @@ -477,26 +477,27 @@ type StatusCause struct { Field string `json:"field,omitempty" yaml:"field,omitempty"` } -// CauseReasonType is a machine readable value providing more detail about why -// an operation failed. An operation may have multiple causes for a failure. -type CauseReasonType string +// CauseType is a machine readable value providing more detail about what +// occured in a status response. An operation may have multiple causes for a +// status (whether failure, success, or working). +type CauseType string const ( - // CauseReasonTypeFieldValueNotFound is used to report failure to find a requested value + // CauseTypeFieldValueNotFound is used to report failure to find a requested value // (e.g. looking up an ID). - CauseReasonTypeFieldValueNotFound CauseReasonType = "fieldValueNotFound" - // CauseReasonTypeFieldValueInvalid is used to report required values that are not + CauseTypeFieldValueNotFound CauseType = "fieldValueNotFound" + // CauseTypeFieldValueInvalid is used to report required values that are not // provided (e.g. empty strings, null values, or empty arrays). - CauseReasonTypeFieldValueRequired CauseReasonType = "fieldValueRequired" - // CauseReasonTypeFieldValueDuplicate is used to report collisions of values that must be + CauseTypeFieldValueRequired CauseType = "fieldValueRequired" + // CauseTypeFieldValueDuplicate is used to report collisions of values that must be // unique (e.g. unique IDs). - CauseReasonTypeFieldValueDuplicate CauseReasonType = "fieldValueDuplicate" - // CauseReasonTypeFieldValueInvalid is used to report malformed values (e.g. failed regex + CauseTypeFieldValueDuplicate CauseType = "fieldValueDuplicate" + // CauseTypeFieldValueInvalid is used to report malformed values (e.g. failed regex // match). - CauseReasonTypeFieldValueInvalid CauseReasonType = "fieldValueInvalid" - // CauseReasonTypeFieldValueNotSupported is used to report valid (as per formatting rules) + CauseTypeFieldValueInvalid CauseType = "fieldValueInvalid" + // CauseTypeFieldValueNotSupported is used to report valid (as per formatting rules) // values that can not be handled (e.g. an enumerated string). - CauseReasonTypeFieldValueNotSupported CauseReasonType = "fieldValueNotSupported" + CauseTypeFieldValueNotSupported CauseType = "fieldValueNotSupported" ) // ServerOp is an operation delivered to API clients. diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 59809d1d19c..6f1ca93ffe0 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -626,7 +626,7 @@ func TestAsyncDelayReturnsError(t *testing.T) { server := httptest.NewServer(handler) status := expectApiStatus(t, "DELETE", fmt.Sprintf("%s/prefix/version/foo/bar", server.URL), nil, http.StatusConflict) - if status.Status != api.StatusFailure || status.Message == "" || status.Details == nil || status.Reason != api.ReasonTypeAlreadyExists { + if status.Status != api.StatusFailure || status.Message == "" || status.Details == nil || status.Reason != api.StatusReasonAlreadyExists { t.Errorf("Unexpected status %#v", status) } } @@ -687,7 +687,7 @@ func TestWriteJSONDecodeError(t *testing.T) { writeJSON(http.StatusOK, api.Codec, &T{"Undecodable"}, w) })) status := expectApiStatus(t, "GET", server.URL, nil, http.StatusInternalServerError) - if status.Reason != api.ReasonTypeUnknown { + if status.Reason != api.StatusReasonUnknown { t.Errorf("unexpected reason %#v", status) } if !strings.Contains(status.Message, "type apiserver.T is not registered") { diff --git a/pkg/apiserver/errors.go b/pkg/apiserver/errors.go index f1f6682623f..471b34f3284 100644 --- a/pkg/apiserver/errors.go +++ b/pkg/apiserver/errors.go @@ -40,7 +40,7 @@ func NewNotFoundErr(kind, name string) error { return &apiServerError{api.Status{ Status: api.StatusFailure, Code: http.StatusNotFound, - Reason: api.ReasonTypeNotFound, + Reason: api.StatusReasonNotFound, Details: &api.StatusDetails{ Kind: kind, ID: name, @@ -54,7 +54,7 @@ func NewAlreadyExistsErr(kind, name string) error { return &apiServerError{api.Status{ Status: api.StatusFailure, Code: http.StatusConflict, - Reason: api.ReasonTypeAlreadyExists, + Reason: api.StatusReasonAlreadyExists, Details: &api.StatusDetails{ Kind: kind, ID: name, @@ -68,7 +68,7 @@ func NewConflictErr(kind, name string, err error) error { return &apiServerError{api.Status{ Status: api.StatusFailure, Code: http.StatusConflict, - Reason: api.ReasonTypeConflict, + Reason: api.StatusReasonConflict, Details: &api.StatusDetails{ Kind: kind, ID: name, @@ -83,7 +83,7 @@ func NewInvalidErr(kind, name string, errs errors.ErrorList) error { for i := range errs { if err, ok := errs[i].(errors.ValidationError); ok { causes = append(causes, api.StatusCause{ - Reason: api.CauseReasonType(err.Type), + Type: api.CauseType(err.Type), Message: err.Error(), Field: err.Field, }) @@ -92,7 +92,7 @@ func NewInvalidErr(kind, name string, errs errors.ErrorList) error { return &apiServerError{api.Status{ Status: api.StatusFailure, Code: 422, // RFC 4918 - Reason: api.ReasonTypeInvalid, + Reason: api.StatusReasonInvalid, Details: &api.StatusDetails{ Kind: kind, ID: name, @@ -104,30 +104,30 @@ func NewInvalidErr(kind, name string, errs errors.ErrorList) error { // IsNotFound returns true if the specified error was created by NewNotFoundErr func IsNotFound(err error) bool { - return reasonForError(err) == api.ReasonTypeNotFound + return reasonForError(err) == api.StatusReasonNotFound } // IsAlreadyExists determines if the err is an error which indicates that a specified resource already exists. func IsAlreadyExists(err error) bool { - return reasonForError(err) == api.ReasonTypeAlreadyExists + return reasonForError(err) == api.StatusReasonAlreadyExists } // IsConflict determines if the err is an error which indicates the provided update conflicts func IsConflict(err error) bool { - return reasonForError(err) == api.ReasonTypeConflict + return reasonForError(err) == api.StatusReasonConflict } // IsInvalid determines if the err is an error which indicates the provided resource is not valid func IsInvalid(err error) bool { - return reasonForError(err) == api.ReasonTypeInvalid + return reasonForError(err) == api.StatusReasonInvalid } -func reasonForError(err error) api.ReasonType { +func reasonForError(err error) api.StatusReason { switch t := err.(type) { case *apiServerError: return t.Status.Reason } - return api.ReasonTypeUnknown + return api.StatusReasonUnknown } // errToAPIStatus converts an error to an api.Status object. @@ -148,7 +148,7 @@ func errToAPIStatus(err error) *api.Status { return &api.Status{ Status: api.StatusFailure, Code: status, - Reason: api.ReasonTypeUnknown, + Reason: api.StatusReasonUnknown, Message: err.Error(), } } diff --git a/pkg/apiserver/errors_test.go b/pkg/apiserver/errors_test.go index 213b9581702..d887faa9874 100644 --- a/pkg/apiserver/errors_test.go +++ b/pkg/apiserver/errors_test.go @@ -35,7 +35,7 @@ func TestErrorNew(t *testing.T) { t.Errorf("expected to not be confict") } if IsNotFound(err) { - t.Errorf(fmt.Sprintf("expected to not be %s", api.ReasonTypeNotFound)) + t.Errorf(fmt.Sprintf("expected to not be %s", api.StatusReasonNotFound)) } if IsInvalid(err) { t.Errorf("expected to not be invalid") @@ -63,8 +63,8 @@ func TestNewInvalidErr(t *testing.T) { Kind: "kind", ID: "name", Causes: []api.StatusCause{{ - Reason: api.CauseReasonTypeFieldValueDuplicate, - Field: "field[0].name", + Type: api.CauseTypeFieldValueDuplicate, + Field: "field[0].name", }}, }, }, @@ -74,8 +74,8 @@ func TestNewInvalidErr(t *testing.T) { Kind: "kind", ID: "name", Causes: []api.StatusCause{{ - Reason: api.CauseReasonTypeFieldValueInvalid, - Field: "field[0].name", + Type: api.CauseTypeFieldValueInvalid, + Field: "field[0].name", }}, }, }, @@ -85,8 +85,8 @@ func TestNewInvalidErr(t *testing.T) { Kind: "kind", ID: "name", Causes: []api.StatusCause{{ - Reason: api.CauseReasonTypeFieldValueNotFound, - Field: "field[0].name", + Type: api.CauseTypeFieldValueNotFound, + Field: "field[0].name", }}, }, }, @@ -96,8 +96,8 @@ func TestNewInvalidErr(t *testing.T) { Kind: "kind", ID: "name", Causes: []api.StatusCause{{ - Reason: api.CauseReasonTypeFieldValueNotSupported, - Field: "field[0].name", + Type: api.CauseTypeFieldValueNotSupported, + Field: "field[0].name", }}, }, }, @@ -107,8 +107,8 @@ func TestNewInvalidErr(t *testing.T) { Kind: "kind", ID: "name", Causes: []api.StatusCause{{ - Reason: api.CauseReasonTypeFieldValueRequired, - Field: "field[0].name", + Type: api.CauseTypeFieldValueRequired, + Field: "field[0].name", }}, }, }, @@ -118,7 +118,7 @@ func TestNewInvalidErr(t *testing.T) { expected.Causes[0].Message = vErr.Error() err := NewInvalidErr("kind", "name", apierrors.ErrorList{vErr}) status := errToAPIStatus(err) - if status.Code != 422 || status.Reason != api.ReasonTypeInvalid { + if status.Code != 422 || status.Reason != api.StatusReasonInvalid { t.Errorf("unexpected status: %#v", status) } if !reflect.DeepEqual(expected, status.Details) { @@ -130,13 +130,13 @@ func TestNewInvalidErr(t *testing.T) { func Test_errToAPIStatus(t *testing.T) { err := &apiServerError{} status := errToAPIStatus(err) - if status.Reason != api.ReasonTypeUnknown || status.Status != api.StatusFailure { + if status.Reason != api.StatusReasonUnknown || status.Status != api.StatusFailure { t.Errorf("unexpected status object: %#v", status) } } func Test_reasonForError(t *testing.T) { - if e, a := api.ReasonTypeUnknown, reasonForError(nil); e != a { + if e, a := api.StatusReasonUnknown, reasonForError(nil); e != a { t.Errorf("unexpected reason type: %#v", a) } } diff --git a/pkg/apiserver/operation.go b/pkg/apiserver/operation.go index 62677c1f33a..7efa08341a6 100644 --- a/pkg/apiserver/operation.go +++ b/pkg/apiserver/operation.go @@ -192,7 +192,7 @@ func (op *Operation) StatusOrResult() (description interface{}, finished bool) { if op.finished == nil { return &api.Status{ Status: api.StatusWorking, - Reason: api.ReasonTypeWorking, + Reason: api.StatusReasonWorking, Details: &api.StatusDetails{ID: op.ID, Kind: "operation"}, }, false }