From 9aeb0aa2c790fc3e968974b459c4fd3eb54e96c5 Mon Sep 17 00:00:00 2001 From: Bill Thiede Date: Tue, 31 Dec 2013 19:33:56 -0800 Subject: [PATCH] pkg/client: Search fix zero-value Describe queries. Adds support for zero value blob.Ref {Unm,M}arshalJSON. Fix for: https://camlistore.org/issue/308 Change-Id: I910fc4f05015b0c3ddc57eba9d1d8fe1bfe992bf --- pkg/blob/ref.go | 14 +++++++--- pkg/blob/ref_test.go | 27 ++++++++++++++++++- pkg/search/handler.go | 14 +++++----- pkg/search/query_test.go | 56 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 100 insertions(+), 11 deletions(-) diff --git a/pkg/blob/ref.go b/pkg/blob/ref.go index e2cea0620..65a3f787f 100644 --- a/pkg/blob/ref.go +++ b/pkg/blob/ref.go @@ -464,23 +464,31 @@ func ValidRefString(s string) bool { return ParseOrZero(s).Valid() } +var null = []byte(`null`) + func (r *Ref) UnmarshalJSON(d []byte) error { if r.digest != nil { return errors.New("Can't UnmarshalJSON into a non-zero Ref") } + if len(d) == 0 || bytes.Equal(d, null) { + return nil + } if len(d) < 2 || d[0] != '"' || d[len(d)-1] != '"' { return fmt.Errorf("blob: expecting a JSON string to unmarshal, got %q", d) } - refStr := string(d[1 : len(d)-1]) - p, ok := Parse(refStr) + d = d[1 : len(d)-1] + p, ok := ParseBytes(d) if !ok { - return fmt.Errorf("blobref: invalid blobref %q (%d)", refStr, len(refStr)) + return fmt.Errorf("blobref: invalid blobref %q (%d)", d, len(d)) } *r = p return nil } func (r Ref) MarshalJSON() ([]byte, error) { + if !r.Valid() { + return null, nil + } dname := r.digest.digestName() bs := r.digest.bytes() buf := make([]byte, 0, 3+len(dname)+len(bs)*2) diff --git a/pkg/blob/ref_test.go b/pkg/blob/ref_test.go index 8bb537863..1bfa1d84e 100644 --- a/pkg/blob/ref_test.go +++ b/pkg/blob/ref_test.go @@ -126,14 +126,39 @@ func TestJSONUnmarshal(t *testing.T) { if g, e := f.B.String(), "abc-def123"; g != e { t.Errorf("got %q, want %q", g, e) } + + f = Foo{} + if err := json.Unmarshal([]byte(`{}`), &f); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if f.B.Valid() { + t.Fatal("blobref is valid and shouldn't be") + } + + f = Foo{} + if err := json.Unmarshal([]byte(`{"foo":null}`), &f); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if f.B.Valid() { + t.Fatal("blobref is valid and shouldn't be") + } } func TestJSONMarshal(t *testing.T) { - f := &Foo{B: MustParse("def-1234abcd")} + f := &Foo{} bs, err := json.Marshal(f) if err != nil { t.Fatalf("Marshal: %v", err) } + if g, e := string(bs), `{"foo":null}`; g != e { + t.Errorf("got %q, want %q", g, e) + } + + f = &Foo{B: MustParse("def-1234abcd")} + bs, err = json.Marshal(f) + if err != nil { + t.Fatalf("Marshal: %v", err) + } if g, e := string(bs), `{"foo":"def-1234abcd"}`; g != e { t.Errorf("got %q, want %q", g, e) } diff --git a/pkg/search/handler.go b/pkg/search/handler.go index 388fe571d..8626de4dd 100644 --- a/pkg/search/handler.go +++ b/pkg/search/handler.go @@ -642,26 +642,28 @@ func (sh *Handler) serveClaims(rw http.ResponseWriter, req *http.Request) { type DescribeRequest struct { // BlobRefs are the blobs to describe. If length zero, BlobRef // is used. - BlobRefs []blob.Ref + BlobRefs []blob.Ref `json:"blobrefs,omitempty"` // BlobRef is the blob to describe. - BlobRef blob.Ref + BlobRef blob.Ref `json:"blobref,omitempty"` // Depth is the optional traversal depth to describe from the // root BlobRef. If zero, a default is used. - Depth int + Depth int `json:"depth,omitempty"` // MaxDirChildren is the requested optional limit to the number // of children that should be fetched when describing a static // directory. If zero, a default is used. - MaxDirChildren int + MaxDirChildren int `json:"maxDirChildren,omitempty"` // At specifies the time which we wish to see the state of // this blob. If zero (unspecified), all claims will be // considered, otherwise, any claims after this date will not // be considered. - At types.Time3339 + At types.Time3339 `json:"at"` - ThumbnailSize int // or zero for none + // ThumbnailSize sets the max dimension for the thumbnail ULR generated, + // or zero for none + ThumbnailSize int `json:"thumbnailSize,omitempty"` // Internal details, used while loading. // Initialized by sh.initDescribeRequest. diff --git a/pkg/search/query_test.go b/pkg/search/query_test.go index 611cbd850..95b6502f0 100644 --- a/pkg/search/query_test.go +++ b/pkg/search/query_test.go @@ -895,6 +895,60 @@ func TestPlannedQuery(t *testing.T) { } } +func TestDescribeMarshal(t *testing.T) { + // Empty Describe + q := &SearchQuery{ + Describe: &DescribeRequest{}, + } + enc, err := json.Marshal(q) + if err != nil { + t.Fatal(err) + } + if got, want := string(enc), `{"describe":{"blobref":null,"at":null}}`; got != want { + t.Errorf("JSON: %s; want %s", got, want) + } + back := &SearchQuery{} + err = json.Unmarshal(enc, back) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(q, back) { + t.Errorf("Didn't round-trip. Got %#v; want %#v", back, q) + } + + // DescribeRequest with multiple blobref + q = &SearchQuery{ + Describe: &DescribeRequest{ + BlobRefs: []blob.Ref{blob.MustParse("sha-1234"), blob.MustParse("sha-abcd")}, + }, + } + enc, err = json.Marshal(q) + if err != nil { + t.Fatal(err) + } + if got, want := string(enc), `{"describe":{"blobrefs":["sha-1234","sha-abcd"],"blobref":null,"at":null}}`; got != want { + t.Errorf("JSON: %s; want %s", got, want) + } + back = &SearchQuery{} + err = json.Unmarshal(enc, back) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(q, back) { + t.Errorf("Didn't round-trip. Got %#v; want %#v", back, q) + } + + // and the zero value + q = &SearchQuery{} + enc, err = json.Marshal(q) + if err != nil { + t.Fatal(err) + } + if string(enc) != "{}" { + t.Errorf(`Zero value: %q; want null`, enc) + } +} + func TestSortMarshal(t *testing.T) { q := &SearchQuery{ Sort: CreatedDesc, @@ -904,7 +958,7 @@ func TestSortMarshal(t *testing.T) { t.Fatal(err) } if got, want := string(enc), `{"sort":"-created"}`; got != want { - t.Logf("JSON: %s; want %s", got, want) + t.Errorf("JSON: %s; want %s", got, want) } back := &SearchQuery{} err = json.Unmarshal(enc, back)