Merge pull request #7215 from nikhiljindal/errHandle

Updating serviceErrorHandler to use apiVersion specific codec

Godeps/Godeps.json

@@ -155,8 +155,8 @@
 		},
 		{
 			"ImportPath": "github.com/emicklei/go-restful",
-			"Comment": "v1.1.3-34-g5e1952e",
+			"Comment": "v1.1.3-40-g4f30cbd",
-			"Rev": "5e1952ed0806503c059e4463c2654200660f484b"
+			"Rev": "4f30cbd5bd858a523d8fe9bd484f44513f50eeec"
 		},
 		{
 			"ImportPath": "github.com/evanphx/json-patch",

Godeps/_workspace/src/github.com/emicklei/go-restful/container.go

@@ -54,8 +54,9 @@ func (c *Container) RecoverHandler(handler RecoverHandleFunction) {
 }
 
 // ServiceErrorHandleFunction declares functions that can be used to handle a service error situation.
-// The first argument is the service error. The second must be used to communicate an error response.
+// The first argument is the service error, the second is the request that resulted in the error and
-type ServiceErrorHandleFunction func(ServiceError, *Response)
+// the third must be used to communicate an error response.
+type ServiceErrorHandleFunction func(ServiceError, *Request, *Response)
 
 // ServiceErrorHandler changes the default function (writeServiceError) to be called
 // when a ServiceError is detected.
@@ -143,7 +144,7 @@ func logStackOnRecover(panicReason interface{}, httpWriter http.ResponseWriter)
 // writeServiceError is the default ServiceErrorHandleFunction and is called
 // when a ServiceError is returned during route selection. Default implementation
 // calls resp.WriteErrorString(err.Code, err.Message)
-func writeServiceError(err ServiceError, resp *Response) {
+func writeServiceError(err ServiceError, req *Request, resp *Response) {
 	resp.WriteErrorString(err.Code, err.Message)
 }
 
@@ -194,7 +195,7 @@ func (c *Container) dispatch(httpWriter http.ResponseWriter, httpRequest *http.R
 			switch err.(type) {
 			case ServiceError:
 				ser := err.(ServiceError)
-				c.serviceErrorHandleFunc(ser, resp)
+				c.serviceErrorHandleFunc(ser, req, resp)
 			}
 			// TODO
 		}}

Godeps/_workspace/src/github.com/emicklei/go-restful/swagger/CHANGES.md

@@ -1,5 +1,8 @@
 Change history of swagger
 =
+2015-04-09
+- add ModelBuildable interface for customization of Model
+
 2015-03-17
 - preserve order of Routes per WebService in Swagger listing
 - fix use of $ref and type in Swagger models

Godeps/_workspace/src/github.com/emicklei/go-restful/swagger/model_builder.go

@@ -6,22 +6,40 @@ import (
 	"strings"
 )
 
+// ModelBuildable is used for extending Structs that need more control over
+// how the Model appears in the Swagger api declaration.
+type ModelBuildable interface {
+	PostBuildModel(m *Model) *Model
+}
+
 type modelBuilder struct {
 	Models map[string]Model
 }
 
-func (b modelBuilder) addModel(st reflect.Type, nameOverride string) {
+// addModelFrom creates and adds a Model to the builder and detects and calls
+// the post build hook for customizations
+func (b modelBuilder) addModelFrom(sample interface{}) {
+	if modelOrNil := b.addModel(reflect.TypeOf(sample), ""); modelOrNil != nil {
+		// allow customizations
+		if buildable, ok := sample.(ModelBuildable); ok {
+			modelOrNil = buildable.PostBuildModel(modelOrNil)
+			b.Models[modelOrNil.Id] = *modelOrNil
+		}
+	}
+}
+
+func (b modelBuilder) addModel(st reflect.Type, nameOverride string) *Model {
 	modelName := b.keyFrom(st)
 	if nameOverride != "" {
 		modelName = nameOverride
 	}
 	// no models needed for primitive types
 	if b.isPrimitiveType(modelName) {
-		return
+		return nil
 	}
 	// see if we already have visited this model
 	if _, ok := b.Models[modelName]; ok {
-		return
+		return nil
 	}
 	sm := Model{
 		Id:         modelName,
@@ -34,11 +52,11 @@ func (b modelBuilder) addModel(st reflect.Type, nameOverride string) {
 	// check for slice or array
 	if st.Kind() == reflect.Slice || st.Kind() == reflect.Array {
 		b.addModel(st.Elem(), "")
-		return
+		return &sm
 	}
 	// check for structure or primitive type
 	if st.Kind() != reflect.Struct {
-		return
+		return &sm
 	}
 	for i := 0; i < st.NumField(); i++ {
 		field := st.Field(i)
@@ -55,9 +73,10 @@ func (b modelBuilder) addModel(st reflect.Type, nameOverride string) {
 			sm.Properties[jsonName] = prop
 		}
 	}
-
 	// update model builder with completed model
 	b.Models[modelName] = sm
+
+	return &sm
 }
 
 func (b modelBuilder) isPropertyRequired(field reflect.StructField) bool {
@@ -107,12 +126,12 @@ func (b modelBuilder) buildProperty(field reflect.StructField, model *Model, mod
 	case fieldKind == reflect.Ptr:
 		return b.buildPointerTypeProperty(field, jsonName, modelName)
 	case fieldKind == reflect.String:
-	     	stringt := "string"
+		stringt := "string"
 		prop.Type = &stringt
 		return jsonName, prop
 	case fieldKind == reflect.Map:
-                // if it's a map, it's unstructured, and swagger 1.2 can't handle it
+		// if it's a map, it's unstructured, and swagger 1.2 can't handle it
-	        anyt := "any"
+		anyt := "any"
 		prop.Type = &anyt
 		return jsonName, prop
 	}
@@ -134,6 +153,19 @@ func (b modelBuilder) buildProperty(field reflect.StructField, model *Model, mod
 	return jsonName, prop
 }
 
+func hasNamedJSONTag(field reflect.StructField) bool {
+	parts := strings.Split(field.Tag.Get("json"), ",")
+	if len(parts) == 0 {
+		return false
+	}
+	for _, s := range parts[1:] {
+		if s == "inline" {
+			return false
+		}
+	}
+	return len(parts[0]) > 0
+}
+
 func (b modelBuilder) buildStructTypeProperty(field reflect.StructField, jsonName string, model *Model) (nameJson string, prop ModelProperty) {
 	fieldType := field.Type
 	// check for anonymous
@@ -144,7 +176,8 @@ func (b modelBuilder) buildStructTypeProperty(field reflect.StructField, jsonNam
 		prop.Ref = &anonType
 		return jsonName, prop
 	}
-	if field.Name == fieldType.Name() && field.Anonymous {
+
+	if field.Name == fieldType.Name() && field.Anonymous && !hasNamedJSONTag(field) {
 		// embedded struct
 		sub := modelBuilder{map[string]Model{}}
 		sub.addModel(fieldType, "")
@@ -246,8 +279,9 @@ func (b modelBuilder) keyFrom(st reflect.Type) string {
 	return key
 }
 
+// see also https://golang.org/ref/spec#Numeric_types
 func (b modelBuilder) isPrimitiveType(modelName string) bool {
-	return strings.Contains("uint8 int int32 int64 float32 float64 bool string byte time.Time", modelName)
+	return strings.Contains("uint8 uint16 uint32 uint64 int int8 int16 int32 int64 float32 float64 bool string byte rune time.Time", modelName)
 }
 
 // jsonNameOfField returns the name of the field as it should appear in JSON format
@@ -265,25 +299,31 @@ func (b modelBuilder) jsonNameOfField(field reflect.StructField) string {
 	return field.Name
 }
 
+// see also http://json-schema.org/latest/json-schema-core.html#anchor8
 func (b modelBuilder) jsonSchemaType(modelName string) string {
 	schemaMap := map[string]string{
-		"uint8":     "integer",
+		"uint8":  "integer",
-		"int":       "integer",
+		"uint16": "integer",
-		"int32":     "integer",
+		"uint32": "integer",
-		"int64":     "integer",
+		"uint64": "integer",
-                "uint64":    "integer",
+
-		"byte":      "string",
+		"int":   "integer",
+		"int8":  "integer",
+		"int16": "integer",
+		"int32": "integer",
+		"int64": "integer",
+
+		"byte":      "integer",
 		"float64":   "number",
 		"float32":   "number",
 		"bool":      "boolean",
 		"time.Time": "string",
 	}
 	mapped, ok := schemaMap[modelName]
-	if ok {
+	if !ok {
-		return mapped
-	} else {
 		return modelName // use as is (custom or struct)
 	}
+	return mapped
 }
 
 func (b modelBuilder) jsonSchemaFormat(modelName string) string {
@@ -298,9 +338,8 @@ func (b modelBuilder) jsonSchemaFormat(modelName string) string {
 		"time.Time": "date-time",
 	}
 	mapped, ok := schemaMap[modelName]
-	if ok {
+	if !ok {
-		return mapped
-	} else {
 		return "" // no format
 	}
+	return mapped
 }

Godeps/_workspace/src/github.com/emicklei/go-restful/swagger/model_builder_test.go

@@ -647,6 +647,61 @@ func TestStructA3(t *testing.T) {
  }`)
 }
 
+type A4 struct {
+	D "json:,inline"
+}
+
+// clear && go test -v -test.run TestStructA4 ...swagger
+func TestEmbeddedStructA4(t *testing.T) {
+	testJsonFromStruct(t, A4{}, `{
+  "swagger.A4": {
+   "id": "swagger.A4",
+   "required": [
+    "Id"
+   ],
+   "properties": {
+    "Id": {
+     "type": "integer",
+     "format": "int32"
+    }
+   }
+  }
+ }`)
+}
+
+type A5 struct {
+	D `json:"d"`
+}
+
+// clear && go test -v -test.run TestStructA5 ...swagger
+func TestEmbeddedStructA5(t *testing.T) {
+	testJsonFromStruct(t, A5{}, `{
+  "swagger.A5": {
+   "id": "swagger.A5",
+   "required": [
+    "d"
+   ],
+   "properties": {
+    "d": {
+     "$ref": "swagger.D"
+    }
+   }
+  },
+  "swagger.D": {
+   "id": "swagger.D",
+   "required": [
+    "Id"
+   ],
+   "properties": {
+    "Id": {
+     "type": "integer",
+     "format": "int32"
+    }
+   }
+  }
+ }`)
+}
+
 type ObjectId []byte
 
 type Region struct {

Godeps/_workspace/src/github.com/emicklei/go-restful/swagger/postbuild_model_test.go

@@ -0,0 +1,42 @@
+package swagger
+
+import "testing"
+
+type Boat struct {
+	Length int `json:"-"` // on default, this makes the fields not required
+	Weight int `json:"-"`
+}
+
+// PostBuildModel is from swagger.ModelBuildable
+func (b Boat) PostBuildModel(m *Model) *Model {
+	// override required
+	m.Required = []string{"Length", "Weight"}
+
+	// add model property (just to test is can be added; is this a real usecase?)
+	extraType := "string"
+	m.Properties["extra"] = ModelProperty{
+		Description: "extra description",
+		DataTypeFields: DataTypeFields{
+			Type: &extraType,
+		},
+	}
+	return m
+}
+
+func TestCustomPostModelBuilde(t *testing.T) {
+	testJsonFromStruct(t, Boat{}, `{
+  "swagger.Boat": {
+   "id": "swagger.Boat",
+   "required": [
+    "Length",
+    "Weight"
+   ],
+   "properties": {
+    "extra": {
+     "type": "string",
+     "description": "extra description"
+    }
+   }
+  }
+}`)
+}

Godeps/_workspace/src/github.com/emicklei/go-restful/swagger/swagger_webservice.go

@@ -299,7 +299,7 @@ func (sws SwaggerService) addModelFromSampleTo(operation *Operation, isResponse
 		}
 		operation.Type = modelName
 	}
-	modelBuilder{models}.addModel(reflect.TypeOf(sample), "")
+	modelBuilder{models}.addModelFrom(sample)
 }
 
 func asSwaggerParameter(param restful.ParameterData) Parameter {

Godeps/_workspace/src/github.com/emicklei/go-restful/swagger/utils_test.go

@@ -18,7 +18,7 @@ func testJsonFromStruct(t *testing.T, sample interface{}, expectedJson string) b
 func modelsFromStruct(sample interface{}) map[string]Model {
 	models := map[string]Model{}
 	builder := modelBuilder{models}
-	builder.addModel(reflect.TypeOf(sample), "")
+	builder.addModelFrom(sample)
 	return models
 }
 

pkg/apiserver/apiserver.go

@@ -171,13 +171,27 @@ func InstallLogsSupport(mux Mux) {
 	mux.Handle("/logs/", http.StripPrefix("/logs/", http.FileServer(http.Dir("/var/log/"))))
 }
 
-func InstallServiceErrorHandler(container *restful.Container) {
+func InstallServiceErrorHandler(container *restful.Container, requestResolver *APIRequestInfoResolver, apiVersions []string) {
-	container.ServiceErrorHandler(serviceErrorHandler)
+	container.ServiceErrorHandler(func(serviceErr restful.ServiceError, request *restful.Request, response *restful.Response) {
+		serviceErrorHandler(requestResolver, apiVersions, serviceErr, request, response)
+	})
 }
 
-func serviceErrorHandler(serviceErr restful.ServiceError, response *restful.Response) {
+func serviceErrorHandler(requestResolver *APIRequestInfoResolver, apiVersions []string, serviceErr restful.ServiceError, request *restful.Request, response *restful.Response) {
-	// TODO: Update go-restful to return the request as well, so that we can use the appropriate codec rather than using the latest one.
+	requestInfo, err := requestResolver.GetAPIRequestInfo(request.Request)
-	errorJSON(apierrors.NewGenericServerResponse(serviceErr.Code, "", "", "", "", 0, false), latest.Codec, response.ResponseWriter)
+	codec := latest.Codec
+	if err == nil && requestInfo.APIVersion != "" {
+		// check if the api version is valid.
+		for _, version := range apiVersions {
+			if requestInfo.APIVersion == version {
+				// valid api version.
+				codec = runtime.CodecFor(api.Scheme, requestInfo.APIVersion)
+				break
+			}
+		}
+	}
+
+	errorJSON(apierrors.NewGenericServerResponse(serviceErr.Code, "", "", "", "", 0, false), codec, response.ResponseWriter)
 }
 
 // Adds a service to return the supported api versions.

pkg/master/master.go

@@ -449,7 +449,9 @@ func (m *Master) init(c *Config) {
 
 	apiserver.InstallSupport(m.muxHelper, m.rootWebService)
 	apiserver.AddApiWebService(m.handlerContainer, c.APIPrefix, apiVersions)
-	apiserver.InstallServiceErrorHandler(m.handlerContainer)
+	defaultVersion := m.defaultAPIGroupVersion()
+	requestInfoResolver := &apiserver.APIRequestInfoResolver{util.NewStringSet(strings.TrimPrefix(defaultVersion.Root, "/")), defaultVersion.Mapper}
+	apiserver.InstallServiceErrorHandler(m.handlerContainer, requestInfoResolver, apiVersions)
 
 	// Register root handler.
 	// We do not register this using restful Webservice since we do not want to surface this in api docs.