From baec67a6e089cec17bc5b0f20fbe840a26eebcf5 Mon Sep 17 00:00:00 2001 From: John W Higgins Date: Tue, 12 May 2020 23:45:22 -0700 Subject: [PATCH 1/3] Export and refactor har.Logger.Entries --- har/entrylist.go | 132 ++++++++++++++++++++++++++++++++++++++++++ har/entrylist_test.go | 83 ++++++++++++++++++++++++++ har/har.go | 80 ++++--------------------- 3 files changed, 226 insertions(+), 69 deletions(-) create mode 100644 har/entrylist.go create mode 100644 har/entrylist_test.go diff --git a/har/entrylist.go b/har/entrylist.go new file mode 100644 index 000000000..59d3efdec --- /dev/null +++ b/har/entrylist.go @@ -0,0 +1,132 @@ +// Copyright 2020 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package har + +import ( + "container/list" + "sync" +) + +type EntryList struct { + lock sync.Mutex + Items *list.List +} + +func NewEntryList() *EntryList { + return &EntryList{ + Items: list.New(), + } +} + +func (el *EntryList) Lock() { + el.lock.Lock() +} + +func (el *EntryList) Unlock() { + el.lock.Unlock() +} + +// AddEntry adds an entry to the entry list +func (el *EntryList) AddEntry(entry *Entry) { + el.Lock() + defer el.Unlock() + + el.Items.PushBack(entry) +} + +// Entries returns a slice containing all entries +func (el *EntryList) Entries() []*Entry { + el.Lock() + defer el.Unlock() + + es := make([]*Entry, 0, el.Items.Len()) + + for e := el.Items.Front(); e != nil; e = e.Next() { + es = append(es, e.Value.(*Entry)) + } + + return es +} + +// RemoveMatches takes a matcher function and returns all entries that return true from the function +func (el *EntryList) RemoveMatches(matcher func(*Entry) bool) []*Entry { + el.Lock() + defer el.Unlock() + + es := make([]*Entry, 0, el.Items.Len()) + var next *list.Element + + for e := el.Items.Front(); e != nil; e = next { + next = e.Next() + + entry := getEntry(e) + if matcher(entry) { + es = append(es, entry) + el.Items.Remove(e) + } + } + + return es +} + +// RemoveEntry removes and entry from the entry list via the entry's id +func (el *EntryList) RemoveEntry(id string) *Entry { + el.Lock() + defer el.Unlock() + + if e, en := el.retrieveElementEntry(id); e != nil { + el.Items.Remove(e) + + return en + } + + return nil +} + +// Reset reinitializes the entrylist +func (el *EntryList) Reset() { + el.Lock() + defer el.Unlock() + + el.Items.Init() +} + +// RetrieveEntry returns an entry from the entrylist via the entry's id +func (el *EntryList) RetrieveEntry(id string) *Entry { + el.Lock() + defer el.Unlock() + + _, en := el.retrieveElementEntry(id) + + return en +} + +func getEntry(e *list.Element) *Entry { + if e != nil { + return e.Value.(*Entry) + } + + return nil +} + +func (el *EntryList) retrieveElementEntry(id string) (*list.Element, *Entry) { + for e := el.Items.Front(); e != nil; e = e.Next() { + if en := getEntry(e); en.ID == id { + return e, en + } + } + + return nil, nil +} diff --git a/har/entrylist_test.go b/har/entrylist_test.go new file mode 100644 index 000000000..6c9048b33 --- /dev/null +++ b/har/entrylist_test.go @@ -0,0 +1,83 @@ +// Copyright 2020 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package har + +import ( + "net/http" + "strings" + "testing" + + "github.com/google/martian/v3" +) + +func TestEntryList(t *testing.T) { + ids := make([]string, 3) + urls := make([]string, 3) + + logger := NewLogger() + + urls[0] = "http://0.example.com/path" + urls[1] = "http://1.example.com/path" + urls[2] = "http://2.example.com/path" + + for idx, url := range urls { + req, err := http.NewRequest("GET", url, nil) + if err != nil { + t.Fatalf("http.NewRequest(): got %v, want no error", err) + } + + _, remove, err := martian.TestContext(req, nil, nil) + if err != nil { + t.Fatalf("martian.TestContext(): got %v, want no error", err) + } + defer remove() + + if err := logger.ModifyRequest(req); err != nil { + t.Fatalf("ModifyRequest(): got %v, want no error", err) + } + + ids[idx] = logger.Entries.Entries()[idx].ID + } + + for idx, url := range urls { + if got, want := logger.Entries.RetrieveEntry(ids[idx]).Request.URL, url; got != want { + t.Errorf("RetrieveEntry(): got %q, want %q", got, want) + } + } + + matcher := func(e *Entry) bool { + return strings.Contains(e.Request.URL, "1.example.com") + } + + if got, want := logger.Entries.RemoveEntry(ids[0]).Request.URL, urls[0]; got != want { + t.Errorf("RemoveEntry: got %q, want %q", got, want) + } + + if got := logger.Entries.RemoveEntry(ids[0]); got != nil { + t.Errorf("RemoveEntry: should not have retrieve an entry") + } + + if got, want := logger.Entries.RemoveMatches(matcher)[0].Request.URL, urls[1]; got != want { + t.Errorf("RemoveMatches: got %q, want %q", got, want) + } + + if got, want := logger.Entries.RetrieveEntry(ids[2]).Request.URL, urls[2]; got != want { + t.Errorf("RemoveEntry got %q, want %q", got, want) + } + + if got := logger.Entries.RetrieveEntry(""); got != nil { + t.Errorf("RetrieveEntry: should not have retrieve an entry") + } +} diff --git a/har/har.go b/har/har.go index 7f38ddac9..bc476b6c4 100644 --- a/har/har.go +++ b/har/har.go @@ -29,7 +29,6 @@ import ( "net/http" "net/url" "strings" - "sync" "time" "unicode/utf8" @@ -46,9 +45,7 @@ type Logger struct { creator *Creator - mu sync.Mutex - entries map[string]*Entry - tail *Entry + Entries *EntryList } // HAR is the top level object of a HAR log. @@ -91,7 +88,6 @@ type Entry struct { // Timings describes various phases within request-response round trip. All // times are specified in milliseconds. Timings *Timings `json:"timings"` - next *Entry } // Request holds data about an individual HTTP request. @@ -392,7 +388,7 @@ func NewLogger() *Logger { Name: "martian proxy", Version: "2.0.0", }, - entries: make(map[string]*Entry), + Entries: NewEntryList(), } l.SetOption(BodyLogging(true)) l.SetOption(PostDataLogging(true)) @@ -434,19 +430,11 @@ func (l *Logger) RecordRequest(id string, req *http.Request) error { Timings: &Timings{}, } - l.mu.Lock() - defer l.mu.Unlock() - - if _, exists := l.entries[id]; exists { + if l.Entries.RetrieveEntry(id) != nil { return fmt.Errorf("Duplicate request ID: %s", id) } - l.entries[id] = entry - if l.tail == nil { - l.tail = entry - } - entry.next = l.tail.next - l.tail.next = entry - l.tail = entry + + l.Entries.AddEntry(entry) return nil } @@ -504,10 +492,7 @@ func (l *Logger) RecordResponse(id string, res *http.Response) error { return err } - l.mu.Lock() - defer l.mu.Unlock() - - if e, ok := l.entries[id]; ok { + if e := l.Entries.RetrieveEntry(id); e != nil { e.Response = hres e.Time = time.Since(e.StartedDateTime).Nanoseconds() / 1000000 } @@ -563,53 +548,14 @@ func NewResponse(res *http.Response, withBody bool) (*Response, error) { // Export returns the in-memory log. func (l *Logger) Export() *HAR { - l.mu.Lock() - defer l.mu.Unlock() - - es := make([]*Entry, 0, len(l.entries)) - curr := l.tail - for curr != nil { - curr = curr.next - es = append(es, curr) - if curr == l.tail { - break - } - } - - return l.makeHAR(es) + return l.makeHAR(l.Entries.Entries()) } // ExportAndReset returns the in-memory log for completed requests, clearing them. func (l *Logger) ExportAndReset() *HAR { - l.mu.Lock() - defer l.mu.Unlock() - - es := make([]*Entry, 0, len(l.entries)) - curr := l.tail - prev := l.tail - var first *Entry - for curr != nil { - curr = curr.next - if curr.Response != nil { - es = append(es, curr) - delete(l.entries, curr.ID) - } else { - if first == nil { - first = curr - } - prev.next = curr - prev = curr - } - if curr == l.tail { - break - } - } - if len(l.entries) == 0 { - l.tail = nil - } else { - l.tail = prev - l.tail.next = first - } + matcher := func(e *Entry) bool { return e.Response != nil } + + es := l.Entries.RemoveMatches(matcher) return l.makeHAR(es) } @@ -626,11 +572,7 @@ func (l *Logger) makeHAR(es []*Entry) *HAR { // Reset clears the in-memory log of entries. func (l *Logger) Reset() { - l.mu.Lock() - defer l.mu.Unlock() - - l.entries = make(map[string]*Entry) - l.tail = nil + l.Entries.Reset() } func cookies(cs []*http.Cookie) []Cookie { From 9ec09ea53ac7b947eb166737896d61f40a6790fb Mon Sep 17 00:00:00 2001 From: John W Higgins Date: Wed, 13 May 2020 16:50:57 -0700 Subject: [PATCH 2/3] Switch to EntryContainer interface --- har/entrylist.go | 59 +++++++++++++++++++------------------------ har/entrylist_test.go | 9 ------- har/har.go | 26 +++++++++++++++---- 3 files changed, 47 insertions(+), 47 deletions(-) diff --git a/har/entrylist.go b/har/entrylist.go index 59d3efdec..9dfef81a0 100644 --- a/har/entrylist.go +++ b/har/entrylist.go @@ -19,41 +19,34 @@ import ( "sync" ) +// EntryList implements the har.EntryContainer interface for the storage of har.Entry type EntryList struct { lock sync.Mutex - Items *list.List + items *list.List } func NewEntryList() *EntryList { return &EntryList{ - Items: list.New(), + items: list.New(), } } -func (el *EntryList) Lock() { - el.lock.Lock() -} - -func (el *EntryList) Unlock() { - el.lock.Unlock() -} - // AddEntry adds an entry to the entry list func (el *EntryList) AddEntry(entry *Entry) { - el.Lock() - defer el.Unlock() + el.lock.Lock() + defer el.lock.Unlock() - el.Items.PushBack(entry) + el.items.PushBack(entry) } // Entries returns a slice containing all entries func (el *EntryList) Entries() []*Entry { - el.Lock() - defer el.Unlock() + el.lock.Lock() + defer el.lock.Unlock() - es := make([]*Entry, 0, el.Items.Len()) + es := make([]*Entry, 0, el.items.Len()) - for e := el.Items.Front(); e != nil; e = e.Next() { + for e := el.items.Front(); e != nil; e = e.Next() { es = append(es, e.Value.(*Entry)) } @@ -61,20 +54,20 @@ func (el *EntryList) Entries() []*Entry { } // RemoveMatches takes a matcher function and returns all entries that return true from the function -func (el *EntryList) RemoveMatches(matcher func(*Entry) bool) []*Entry { - el.Lock() - defer el.Unlock() +func (el *EntryList) RemoveCompleted() []*Entry { + el.lock.Lock() + defer el.lock.Unlock() - es := make([]*Entry, 0, el.Items.Len()) + es := make([]*Entry, 0, el.items.Len()) var next *list.Element - for e := el.Items.Front(); e != nil; e = next { + for e := el.items.Front(); e != nil; e = next { next = e.Next() entry := getEntry(e) - if matcher(entry) { + if entry.Response != nil { es = append(es, entry) - el.Items.Remove(e) + el.items.Remove(e) } } @@ -83,11 +76,11 @@ func (el *EntryList) RemoveMatches(matcher func(*Entry) bool) []*Entry { // RemoveEntry removes and entry from the entry list via the entry's id func (el *EntryList) RemoveEntry(id string) *Entry { - el.Lock() - defer el.Unlock() + el.lock.Lock() + defer el.lock.Unlock() if e, en := el.retrieveElementEntry(id); e != nil { - el.Items.Remove(e) + el.items.Remove(e) return en } @@ -97,16 +90,16 @@ func (el *EntryList) RemoveEntry(id string) *Entry { // Reset reinitializes the entrylist func (el *EntryList) Reset() { - el.Lock() - defer el.Unlock() + el.lock.Lock() + defer el.lock.Unlock() - el.Items.Init() + el.items.Init() } // RetrieveEntry returns an entry from the entrylist via the entry's id func (el *EntryList) RetrieveEntry(id string) *Entry { - el.Lock() - defer el.Unlock() + el.lock.Lock() + defer el.lock.Unlock() _, en := el.retrieveElementEntry(id) @@ -122,7 +115,7 @@ func getEntry(e *list.Element) *Entry { } func (el *EntryList) retrieveElementEntry(id string) (*list.Element, *Entry) { - for e := el.Items.Front(); e != nil; e = e.Next() { + for e := el.items.Front(); e != nil; e = e.Next() { if en := getEntry(e); en.ID == id { return e, en } diff --git a/har/entrylist_test.go b/har/entrylist_test.go index 6c9048b33..ce9eadebd 100644 --- a/har/entrylist_test.go +++ b/har/entrylist_test.go @@ -16,7 +16,6 @@ package har import ( "net/http" - "strings" "testing" "github.com/google/martian/v3" @@ -57,10 +56,6 @@ func TestEntryList(t *testing.T) { } } - matcher := func(e *Entry) bool { - return strings.Contains(e.Request.URL, "1.example.com") - } - if got, want := logger.Entries.RemoveEntry(ids[0]).Request.URL, urls[0]; got != want { t.Errorf("RemoveEntry: got %q, want %q", got, want) } @@ -69,10 +64,6 @@ func TestEntryList(t *testing.T) { t.Errorf("RemoveEntry: should not have retrieve an entry") } - if got, want := logger.Entries.RemoveMatches(matcher)[0].Request.URL, urls[1]; got != want { - t.Errorf("RemoveMatches: got %q, want %q", got, want) - } - if got, want := logger.Entries.RetrieveEntry(ids[2]).Request.URL, urls[2]; got != want { t.Errorf("RemoveEntry got %q, want %q", got, want) } diff --git a/har/har.go b/har/har.go index bc476b6c4..d626f021f 100644 --- a/har/har.go +++ b/har/har.go @@ -38,6 +38,16 @@ import ( "github.com/google/martian/v3/proxyutil" ) +// EntryContainer is an interface for the storage of the har entries +type EntryContainer interface { + AddEntry(entry *Entry) + Entries() []*Entry + RemoveCompleted() []*Entry + RemoveEntry(id string) *Entry + Reset() + RetrieveEntry(id string) *Entry +} + // Logger maintains request and response log entries. type Logger struct { bodyLogging func(*http.Response) bool @@ -45,7 +55,7 @@ type Logger struct { creator *Creator - Entries *EntryList + Entries EntryContainer } // HAR is the top level object of a HAR log. @@ -395,6 +405,12 @@ func NewLogger() *Logger { return l } +// SetEntries allows the changing of the entry container to another struct which +// implements the EntryContainer interface +func (l *Logger) SetEntries(ec EntryContainer) { + l.Entries = ec +} + // SetOption sets configurable options on the logger. func (l *Logger) SetOption(opts ...Option) { for _, opt := range opts { @@ -548,14 +564,14 @@ func NewResponse(res *http.Response, withBody bool) (*Response, error) { // Export returns the in-memory log. func (l *Logger) Export() *HAR { - return l.makeHAR(l.Entries.Entries()) + es := l.Entries.Entries() + + return l.makeHAR(es) } // ExportAndReset returns the in-memory log for completed requests, clearing them. func (l *Logger) ExportAndReset() *HAR { - matcher := func(e *Entry) bool { return e.Response != nil } - - es := l.Entries.RemoveMatches(matcher) + es := l.Entries.RemoveCompleted() return l.makeHAR(es) } From becf132c463cfdc70c41b678827087da8379edc5 Mon Sep 17 00:00:00 2001 From: John W Higgins Date: Wed, 13 May 2020 19:37:06 -0700 Subject: [PATCH 3/3] Name mutexes consistently --- har/entrylist.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/har/entrylist.go b/har/entrylist.go index 9dfef81a0..b9ea73c3a 100644 --- a/har/entrylist.go +++ b/har/entrylist.go @@ -21,7 +21,7 @@ import ( // EntryList implements the har.EntryContainer interface for the storage of har.Entry type EntryList struct { - lock sync.Mutex + mu sync.Mutex items *list.List } @@ -33,16 +33,16 @@ func NewEntryList() *EntryList { // AddEntry adds an entry to the entry list func (el *EntryList) AddEntry(entry *Entry) { - el.lock.Lock() - defer el.lock.Unlock() + el.mu.Lock() + defer el.mu.Unlock() el.items.PushBack(entry) } // Entries returns a slice containing all entries func (el *EntryList) Entries() []*Entry { - el.lock.Lock() - defer el.lock.Unlock() + el.mu.Lock() + defer el.mu.Unlock() es := make([]*Entry, 0, el.items.Len()) @@ -55,8 +55,8 @@ func (el *EntryList) Entries() []*Entry { // RemoveMatches takes a matcher function and returns all entries that return true from the function func (el *EntryList) RemoveCompleted() []*Entry { - el.lock.Lock() - defer el.lock.Unlock() + el.mu.Lock() + defer el.mu.Unlock() es := make([]*Entry, 0, el.items.Len()) var next *list.Element @@ -76,8 +76,8 @@ func (el *EntryList) RemoveCompleted() []*Entry { // RemoveEntry removes and entry from the entry list via the entry's id func (el *EntryList) RemoveEntry(id string) *Entry { - el.lock.Lock() - defer el.lock.Unlock() + el.mu.Lock() + defer el.mu.Unlock() if e, en := el.retrieveElementEntry(id); e != nil { el.items.Remove(e) @@ -90,16 +90,16 @@ func (el *EntryList) RemoveEntry(id string) *Entry { // Reset reinitializes the entrylist func (el *EntryList) Reset() { - el.lock.Lock() - defer el.lock.Unlock() + el.mu.Lock() + defer el.mu.Unlock() el.items.Init() } // RetrieveEntry returns an entry from the entrylist via the entry's id func (el *EntryList) RetrieveEntry(id string) *Entry { - el.lock.Lock() - defer el.lock.Unlock() + el.mu.Lock() + defer el.mu.Unlock() _, en := el.retrieveElementEntry(id)