Skip to content

Commit

Permalink
feat(service): support ErrorProduces (#197)
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue.

 feat(service): support ErrorProduces

**What this PR does / why we need it**:

Add `ErrorProduces` into `Definition`. `ErrorProduces` is used to generate data for error. If this field is empty, it stands for this field equals to Produces. In some cases, succeessful data and error data should be generated in different ways.

e.g.
An API  returns a file (`Content-Type` is `application/java-archive`), but it may returns an error with `NotFound`. You can set `Definition` as:
```go
Produces: []string{"application/java-archive"},
ErrorProduces: []string{"application/json"},
```
If a file existing, the API returns a file with `Content-Type: application/java-archive`, and carries data by response body. If an error occurs, it returns an error json with `Content-Type: application/json`.

**Which issue(s) this PR fixes** *(optional, close the issue(s) when PR gets merged)*:

Fixes #169 

**Special notes for your reviewer**:

/cc @ddysher 

**Release note**:


```release-note
NONE
```

<!--  Thanks for sending a pull request! Here are some tips:

1. https://github.com/caicloud/engineering/blob/master/docs/review_conventions.md  <-- what is the review process looks like
2. https://github.com/caicloud/engineering/blob/master/docs/commit_conventions.md  <-- how to structure your git commit
3. https://github.com/caicloud/engineering/blob/master/docs/caicloud_bot.md        <-- how to work with caicloud bot

Other tips from Kubernetes cmomunity:

1. If this is your first time, read our contributor guidelines https://git.k8s.io/community/contributors/devel/pull-requests.md#the-pr-submit-process and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/devel/pull-requests.md#best-practices-for-faster-reviews
3. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/devel/pull-requests.md#write-release-notes-if-needed
4. If the PR is unfinished, see how to mark it: https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md#marking-unfinished-pull-requests
-->
  • Loading branch information
kdada authored and caicloud-bot committed Apr 9, 2018
1 parent f9ea845 commit 21dc908
Show file tree
Hide file tree
Showing 6 changed files with 214 additions and 63 deletions.
5 changes: 5 additions & 0 deletions definition/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ type Definition struct {
// Produces indicates how many content types the handler can produce.
// It will override parent descriptor's produces.
Produces []string
// ErrorProduces is used to generate data for error. If this field is empty,
// it means that this field equals to Produces.
// In some cases, succeessful data and error data should be generated in
// different ways.
ErrorProduces []string
// Function is a function handler. It must be func type.
Function interface{}
// Parameters describes function parameters.
Expand Down
7 changes: 6 additions & 1 deletion service/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ func (b *builder) copyDefinition(d *definition.Definition, consumes []string, pr
r.Description = ""
newOne.Results[i] = r
}
if len(newOne.ErrorProduces) <= 0 {
newOne.ErrorProduces = newOne.Produces
}
return newOne
}

Expand Down Expand Up @@ -176,7 +179,9 @@ func (b *builder) Build() (Service, error) {
for _, d := range bd.definitions {
b.logger.V(log.LevelDebug).Infof(" Method: %s Consumes: %v Produces: %v",
d.Method, d.Consumes, d.Produces)
b.modifier(&d)
if b.modifier != nil {
b.modifier(&d)
}
if err := inspector.addDefinition(d); err != nil {
return nil, err
}
Expand Down
83 changes: 81 additions & 2 deletions service/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"net/http"
"net/url"
"testing"

"github.com/caicloud/nirvana/definition"
"github.com/caicloud/nirvana/errors"
)

type responseWriter struct {
Expand Down Expand Up @@ -368,7 +368,7 @@ func defaultParamsHandler(ctx context.Context, q1, q2, tenant, tenant2 string, u
if q1 == "q1" && q2 == "" && tenant == "" && tenant2 == "tenant2" && u.Username == "name" && u.Password == "pwd" {
return "match", nil
}
return "", errors.New("not match params")
return "", errors.NotFound.Error("not match params")

}

Expand Down Expand Up @@ -405,6 +405,85 @@ func TestDefaultParams(t *testing.T) {
}
}

var errDesc = definition.Descriptor{
Path: "/api/v1/",
Definitions: []definition.Definition{},
Consumes: []string{definition.MIMEAll},
Produces: []string{definition.MIMEJSON},
Children: []definition.Descriptor{
{
Path: "/{err}",
Definitions: []definition.Definition{
{
Method: definition.Get,
ErrorProduces: []string{definition.MIMEXML},
Function: func(err bool) (*Application, error) {
if err {
return nil, errors.NotFound.Error("error for test ${test}", err)
}
return &Application{
Name: "test",
}, nil
},
Parameters: []definition.Parameter{
{
Source: definition.Path,
Name: "err",
},
},
Results: definition.DataErrorResults(""),
},
},
},
},
}

func TestErrorProduces(t *testing.T) {
builder := NewBuilder()
err := builder.AddDescriptor(errDesc)
if err != nil {
t.Fatal(err)
}
s, err := builder.Build()
if err != nil {
t.Fatal(err)
}

req := &http.Request{
Method: "GET",
Header: http.Header{
"Accept": []string{"application/json, application/xml"},
},
}
req = req.WithContext(context.Background())

u, _ := url.Parse("/api/v1/true")
req.URL = u
resp := newRW()
s.ServeHTTP(resp, req)

if resp.code != 404 {
t.Fatalf("Response code should be 404, but got: %d", resp.code)
}
desired := `<message><Message>error for test true</Message><Data><test>true</test></Data></message>`
if resp.buf.String() != desired {
t.Fatalf("Response data is not desired: %s", resp.buf.String())
}

u, _ = url.Parse("/api/v1/false")
req.URL = u
resp = newRW()
s.ServeHTTP(resp, req)

if resp.code != 200 {
t.Fatalf("Response code should be 200, but got: %d", resp.code)
}
desired = `{"name":"test","namespace":"","target":"","target2":0,"target1":false}` + "\n"
if resp.buf.String() != desired {
t.Fatalf("Response data is not desired: %s", resp.buf.String())
}
}

func BenchmarkServer(b *testing.B) {
u, _ := url.Parse("/api/v1/1222/false?target1=1&target2=false")
data := []byte(`{
Expand Down
63 changes: 49 additions & 14 deletions service/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ func (i *inspector) addDefinition(d definition.Definition) error {
if len(d.Produces) <= 0 {
return definitionNoProduces.Error(d.Method, i.path)
}
if len(d.ErrorProduces) <= 0 {
return definitionNoErrorProduces.Error(d.Method, i.path)
}
if d.Function == nil {
return definitionNoFunction.Error(d.Method, i.path)
}
Expand Down Expand Up @@ -112,6 +115,28 @@ func (i *inspector) addDefinition(d definition.Definition) error {
}
}
}
errorProduceAll := false
errorProduces := map[string]bool{}
for _, ct := range d.ErrorProduces {
if ct == definition.MIMEAll {
errorProduceAll = true
continue
}
if producer := ProducerFor(ct); producer != nil {
c.errorProducers = append(c.errorProducers, producer)
errorProduces[producer.ContentType()] = true
} else {
return definitionNoProducer.Error(ct, d.Method, i.path)
}
}
if errorProduceAll {
// Add remaining producers to executor.
for _, producer := range AllProducers() {
if !errorProduces[producer.ContentType()] {
c.errorProducers = append(c.errorProducers, producer)
}
}
}
// Get func name and file position.
f := runtime.FuncForPC(value.Pointer())
file, line := f.FileLine(value.Pointer())
Expand Down Expand Up @@ -337,14 +362,15 @@ func (i *inspector) Inspect(ctx context.Context) (router.Executor, error) {
}

type executor struct {
logger log.Logger
method string
code int
consumers []Consumer
producers []Producer
parameters []parameter
results []result
function reflect.Value
logger log.Logger
method string
code int
consumers []Consumer
producers []Producer
errorProducers []Producer
parameters []parameter
results []result
function reflect.Value
}

type parameter struct {
Expand Down Expand Up @@ -381,9 +407,9 @@ func (e *executor) acceptable(ct string) bool {
return false
}

func (e *executor) producible(ats []string) bool {
func (e *executor) check(producers []Producer, ats []string) bool {
for _, at := range ats {
for _, c := range e.producers {
for _, c := range producers {
if c.ContentType() == at {
return true
}
Expand All @@ -392,6 +418,10 @@ func (e *executor) producible(ats []string) bool {
return false
}

func (e *executor) producible(ats []string) bool {
return e.check(e.producers, ats) && e.check(e.errorProducers, ats)
}

// Execute executes with context.
func (e *executor) Execute(ctx context.Context) (err error) {
c := HTTPContextFrom(ctx)
Expand All @@ -402,7 +432,7 @@ func (e *executor) Execute(ctx context.Context) (err error) {
for _, p := range e.parameters {
result, err := p.generator.Generate(ctx, c.ValueContainer(), e.consumers, p.name, p.targetType)
if err != nil {
return writeError(ctx, e.producers, err)
return writeError(ctx, e.errorProducers, err)
}
if result == nil {
if p.defaultValue != nil {
Expand All @@ -414,11 +444,11 @@ func (e *executor) Execute(ctx context.Context) (err error) {
for _, operator := range p.operators {
result, err = operator.Operate(ctx, p.name, result)
if err != nil {
return writeError(ctx, e.producers, err)
return writeError(ctx, e.errorProducers, err)
}
}
if result == nil {
return writeError(ctx, e.producers, requiredField.Error(p.name, p.generator.Source()))
return writeError(ctx, e.errorProducers, requiredField.Error(p.name, p.generator.Source()))
} else if closer, ok := result.(io.Closer); ok {
defer func() {
if e := closer.Close(); e != nil && err == nil {
Expand Down Expand Up @@ -451,7 +481,12 @@ func (e *executor) Execute(ctx context.Context) (err error) {
}()
}
}
goon, err := r.handler.Handle(ctx, e.producers, e.code, data)
producers := e.producers
if r.handler.Destination() == definition.Error {
// Select correct producers to produce error.
producers = e.errorProducers
}
goon, err := r.handler.Handle(ctx, producers, e.code, data)
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 21dc908

Please sign in to comment.