From ce2a465d22cd2373fde8d94bbbd498859884a6f3 Mon Sep 17 00:00:00 2001 From: Johan Lindh Date: Mon, 23 Oct 2023 09:40:53 +0200 Subject: [PATCH] simplify --- jaws.go | 3 +- request.go | 51 +++----- request_test.go | 300 ++++++++++++++++++----------------------------- testjaws_test.go | 98 ++++++++++++++++ 4 files changed, 227 insertions(+), 225 deletions(-) create mode 100644 testjaws_test.go diff --git a/jaws.go b/jaws.go index 37c168d..878f8ae 100644 --- a/jaws.go +++ b/jaws.go @@ -495,9 +495,8 @@ func (jw *Jaws) ServeWithTimeout(requestTimeout time.Duration) { // could mean nonreproducible and seemingly // random failures in processing logic. mustBroadcast := func(msg Message) { - isCmd := msg.What.IsCommand() for msgCh, rq := range subs { - if isCmd || rq.wantMessage(&msg) { + if msg.Dest == nil || rq.wantMessage(&msg) { select { case msgCh <- msg: default: diff --git a/request.go b/request.go index 11b1b0e..48c2fdd 100644 --- a/request.go +++ b/request.go @@ -302,21 +302,15 @@ func (rq *Request) Dirty(tags ...interface{}) { // wantMessage returns true if the Request want the message. func (rq *Request) wantMessage(msg *Message) (yes bool) { - if rq != nil { - switch dest := msg.Dest.(type) { - case *Request: - yes = dest == rq - case string: // HTML id - yes = true - case *Element: - yes = dest.Request == rq - case jid.Jid: - yes = rq.GetElement(dest) != nil - default: - rq.mu.RLock() - _, yes = rq.tagMap[msg.Dest] - rq.mu.RUnlock() - } + switch dest := msg.Dest.(type) { + case *Request: + yes = dest == rq + case string: // HTML id + yes = true + default: + rq.mu.RLock() + _, yes = rq.tagMap[msg.Dest] + rq.mu.RUnlock() } return } @@ -339,15 +333,14 @@ func (rq *Request) NewElement(ui UI) *Element { return rq.newElementLocked(ui) } -func (rq *Request) getElementLocked(jid Jid) *Element { - if jid > 0 { - for _, elem := range rq.elems { - if elem.jid == jid { - return elem - } +func (rq *Request) getElementLocked(jid Jid) (elem *Element) { + for _, e := range rq.elems { + if e.jid == jid { + elem = e + break } } - return nil + return } func (rq *Request) GetElement(jid Jid) (e *Element) { @@ -511,14 +504,12 @@ func (rq *Request) process(broadcastMsgCh chan Message, incomingMsgCh <-chan wsM // prepare the data to send in the WS message var wsdata string switch data := tagmsg.Data.(type) { - case nil: - // do nothing case string: wsdata = data - case template.HTML: - wsdata = string(data) case []interface{}: // list of tags wsdata = rq.makeIdList(data) + default: + // do nothing } // collect all elements marked with the tag in the message @@ -526,14 +517,6 @@ func (rq *Request) process(broadcastMsgCh chan Message, incomingMsgCh <-chan wsM switch v := tagmsg.Dest.(type) { case nil: // matches no elements - case *Element: - if v.Request == rq { - todo = append(todo, v) - } - case Jid: - if elem := rq.GetElement(v); elem != nil { - todo = append(todo, elem) - } case string: // target is a regular HTML ID wsQueue = append(wsQueue, wsMsg{ diff --git a/request_test.go b/request_test.go index 47bdc72..c256a0a 100644 --- a/request_test.go +++ b/request_test.go @@ -1,11 +1,9 @@ package jaws import ( - "bytes" "context" "errors" "html/template" - "log" "net/http" "net/http/httptest" "strconv" @@ -20,70 +18,6 @@ import ( const testTimeout = time.Second * 3 -type testRequest struct { - jw *Jaws - *Request - log bytes.Buffer - readyCh chan struct{} - doneCh chan struct{} - inCh chan wsMsg - outCh chan string - bcastCh chan Message - ctx context.Context - cancel context.CancelFunc - expectPanic bool - panicked bool - panicVal any -} - -func newTestRequest() (tr *testRequest) { - tr = &testRequest{ - readyCh: make(chan struct{}), - doneCh: make(chan struct{}), - jw: New(), - } - tr.jw.Logger = log.New(&tr.log, "", 0) - tr.jw.Template = template.Must(template.New("testtemplate").Parse(`{{with $.Dot}}
{{.}}
{{end}}`)) - tr.jw.updateTicker = time.NewTicker(time.Millisecond) - tr.ctx, tr.cancel = context.WithTimeout(context.Background(), time.Hour) - hr := httptest.NewRequest(http.MethodGet, "/", nil) - tr.Request = tr.jw.NewRequest(hr) - - tr.jw.UseRequest(tr.JawsKey, hr.WithContext(tr.ctx)) - - go tr.jw.Serve() - - tr.inCh = make(chan wsMsg) - tr.bcastCh = tr.Jaws.subscribe(tr.Request, 64) - tr.outCh = make(chan string, cap(tr.bcastCh)) - - // ensure subscription is processed - for i := 0; i <= cap(tr.Jaws.subCh); i++ { - tr.Jaws.subCh <- subscription{} - } - - go func() { - defer func() { - if tr.expectPanic { - if tr.panicVal = recover(); tr.panicVal != nil { - tr.panicked = true - } - } - close(tr.doneCh) - }() - close(tr.readyCh) - tr.process(tr.bcastCh, tr.inCh, tr.outCh) // usubs from bcase, closes outCh - tr.recycle() - }() - - return -} - -func (tr *testRequest) Close() { - tr.cancel() - tr.jw.Close() -} - func fillWsCh(ch chan string) { for { select { @@ -94,16 +28,6 @@ func fillWsCh(ch chan string) { } } -/*func fillTagCh(ch chan Message) { - for { - select { - case ch <- Message{}: - default: - return - } - } -}*/ - func TestRequest_Registrations(t *testing.T) { is := testHelper{t} rq := newTestRequest() @@ -125,17 +49,7 @@ func TestRequest_Registrations(t *testing.T) { is.True(jid != jid2) } -/*func TestRequest_SendFailsWhenJawsClosed(t *testing.T) { - is := testHelper{t} - jw := New() - rq := jw.NewRequest(nil) - jw.UseRequest(rq.JawsKey, nil) - jw.Close() - is.Equal(rq.send(Message{}), false) -}*/ - func TestRequest_SendPanicsAfterRecycle(t *testing.T) { - // can not run in parallel is := testHelper{t} defer func() { e := recover() @@ -151,22 +65,6 @@ func TestRequest_SendPanicsAfterRecycle(t *testing.T) { rq.Jaws.Broadcast(Message{}) } -/*func TestRequest_SendFailsWhenContextDone(t *testing.T) { - is := testHelper{t} - jw := New() - defer jw.Close() - hr := httptest.NewRequest(http.MethodGet, "/", nil) - rq := jw.NewRequest(hr) - ctx, cancel := context.WithCancel(context.Background()) - jw.UseRequest(rq.JawsKey, hr.WithContext(ctx)) - defer rq.recycle() - if rq.cancelFn == nil { - is.Fail() - } - cancel() - is.Equal(rq.send(Message{}), false) -}*/ - func TestRequest_HeadHTML(t *testing.T) { is := testHelper{t} jw := New() @@ -248,7 +146,6 @@ func TestRequest_OutboundRespectsContextDone(t *testing.T) { } func TestRequest_OutboundOverflowPanicsWithNoLogger(t *testing.T) { - // can not run in parallel is := testHelper{t} rq := newTestRequest() rq.expectPanic = true @@ -484,107 +381,132 @@ func TestRequest_IgnoresIncomingMsgsDuringShutdown(t *testing.T) { } // log data should contain message that we were unable to deliver error - is.True(strings.Contains(rq.log.String(), "outboundMsgCh full sending event")) + is.True(strings.Contains(rq.jw.log.String(), "outboundMsgCh full sending event")) } -func TestRequest_Sends(t *testing.T) { - is := testHelper{t} - rq := newTestRequest() - defer rq.Close() - - rq.Register("SetAttr") - setAttrElement := rq.GetElements(Tag("SetAttr"))[0] - - rq.Register("RemoveAttr") - removeAttrElement := rq.GetElements(Tag("RemoveAttr"))[0] +func TestRequest_Alert(t *testing.T) { + tmr := time.NewTimer(testTimeout) + defer tmr.Stop() + tj := newTestJaws() + defer tj.Close() + rq1 := tj.newRequest(nil) + rq2 := tj.newRequest(nil) - gotSetAttr := "" - gotRemoveAttr := "" - gotInfoAlert := "" - gotDangerAlert := "" - gotRedirect := "" + rq1.Alert("info", "\nnot\tescaped") + select { + case <-tmr.C: + t.Fatal("timeout") + case s := <-rq1.outCh: + if s != "Alert\t\t\"info\\n\\nnot\\tescaped\"\n" { + t.Errorf("%q", s) + } + } + select { + case s := <-rq2.outCh: + t.Errorf("%q", s) + default: + } +} - is.True(cap(rq.outCh)-len(rq.outCh) > 7) +func TestRequest_Redirect(t *testing.T) { + tmr := time.NewTimer(testTimeout) + defer tmr.Stop() + tj := newTestJaws() + defer tj.Close() + rq1 := tj.newRequest(nil) + rq2 := tj.newRequest(nil) + rq1.Redirect("some-url") select { - case <-time.NewTimer(testTimeout).C: - is.Fail() - case <-rq.readyCh: + case <-tmr.C: + t.Fatal("timeout") + case s := <-rq1.outCh: + if s != "Redirect\t\t\"some-url\"\n" { + t.Errorf("%q", s) + } } + select { + case s := <-rq2.outCh: + t.Errorf("%q", s) + default: + } +} - rq.jw.Broadcast(Message{ - Dest: Tag("SetAttr"), - What: what.SAttr, - Data: "bar\nbaz", - }) - rq.jw.Broadcast(Message{ - Dest: Tag("SetAttr"), - What: what.SAttr, - Data: "bar\nbaz", - }) - rq.jw.Broadcast(Message{ - Dest: Tag("RemoveAttr"), - What: what.RAttr, - Data: "bar", - }) - - rq.Alert("info", "\nnot\tescaped") +func TestRequest_AlertError(t *testing.T) { + tmr := time.NewTimer(testTimeout) + defer tmr.Stop() + tj := newTestJaws() + defer tj.Close() + rq := tj.newRequest(nil) rq.AlertError(errors.New("\nshould-be-escaped")) - rq.Redirect("some-url") + select { + case <-tmr.C: + t.Fatal("timeout") + case s := <-rq.outCh: + if s != "Alert\t\t\"danger\\n<html>\\nshould-be-escaped\"\n" { + t.Errorf("%q", s) + } + } +} - done := false - for !done { - select { - case <-time.NewTimer(testTimeout).C: - t.Log("timeout") - t.FailNow() - done = true - case msgstr, ok := <-rq.outCh: - if ok { - msg, parseok := wsParse([]byte(msgstr)) - if !parseok { - t.Log(strconv.Quote(msgstr), msg) - is.Fail() - } - switch rq.GetElement(msg.Jid) { - case setAttrElement: - gotSetAttr = msg.Format() - case removeAttrElement: - gotRemoveAttr = msg.Format() - default: - switch msg.What { - case what.Alert: - if strings.HasPrefix(msg.Data, "info\n") { - gotInfoAlert = msg.Format() - } - if strings.HasPrefix(msg.Data, "danger\n") { - gotDangerAlert = msg.Format() - } - case what.Redirect: - gotRedirect = msg.Format() - rq.cancel() - default: - t.Log(msg) - t.FailNow() - } - } - } - case <-rq.doneCh: - done = true +func TestRequest_TagBroadcast(t *testing.T) { + tmr := time.NewTimer(testTimeout) + defer tmr.Stop() + tj := newTestJaws() + defer tj.Close() + rq1 := tj.newRequest(nil) + rq1.Register("fooTag") + rq2 := tj.newRequest(nil) + + tj.Broadcast(Message{ + Dest: Tag("fooTag"), + What: what.Inner, + Data: "inner", + }) + select { + case <-tmr.C: + t.Fatal("timeout") + case s := <-rq1.outCh: + if s != "Inner\tJid.32\t\"inner\"\n" { + t.Errorf("%q", s) } } + select { + case s := <-rq2.outCh: + t.Errorf("%q", s) + default: + } +} - if !strings.HasSuffix(gotSetAttr, "\t\"bar\\nbaz\"\n") { - t.Log(strconv.Quote(gotSetAttr)) - is.Fail() +func TestRequest_HtmlIdBroadcast(t *testing.T) { + tmr := time.NewTimer(testTimeout) + defer tmr.Stop() + tj := newTestJaws() + defer tj.Close() + rq1 := tj.newRequest(nil) + rq2 := tj.newRequest(nil) + + tj.Broadcast(Message{ + Dest: "fooId", + What: what.Inner, + Data: "inner", + }) + select { + case <-tmr.C: + t.Fatal("timeout") + case s := <-rq1.outCh: + if s != "Inner\tfooId\t\"inner\"\n" { + t.Errorf("%q", s) + } } - if !(strings.HasPrefix(gotRemoveAttr, "RAttr\t") && strings.HasSuffix(gotRemoveAttr, "\t\"bar\"\n")) { - t.Log(strconv.Quote(gotRemoveAttr)) - is.Fail() + select { + case <-tmr.C: + t.Fatal("timeout") + case s := <-rq2.outCh: + if s != "Inner\tfooId\t\"inner\"\n" { + t.Errorf("%q", s) + } } - is.Equal(gotRedirect, "Redirect\t\t\"some-url\"\n") - is.Equal(gotInfoAlert, "Alert\t\t\"info\\n\\nnot\\tescaped\"\n") - is.Equal(gotDangerAlert, "Alert\t\t\"danger\\n<html>\\nshould-be-escaped\"\n") } func jidForTag(rq *Request, tag interface{}) jid.Jid { diff --git a/testjaws_test.go b/testjaws_test.go new file mode 100644 index 0000000..de3c63e --- /dev/null +++ b/testjaws_test.go @@ -0,0 +1,98 @@ +package jaws + +import ( + "bytes" + "context" + "html/template" + "log" + "net/http" + "net/http/httptest" + "time" +) + +type testJaws struct { + *Jaws + log bytes.Buffer +} + +func newTestJaws() (tj *testJaws) { + tj = &testJaws{ + Jaws: New(), + } + tj.Jaws.Logger = log.New(&tj.log, "", 0) + tj.Template = template.Must(template.New("testtemplate").Parse(`{{with $.Dot}}
{{.}}
{{end}}`)) + tj.Jaws.updateTicker = time.NewTicker(time.Millisecond) + go tj.Serve() + return +} + +type testRequest struct { + hr *http.Request + jw *testJaws + readyCh chan struct{} + doneCh chan struct{} + inCh chan wsMsg + outCh chan string + bcastCh chan Message + ctx context.Context + cancel context.CancelFunc + expectPanic bool + panicked bool + panicVal any + *Request +} + +func (tj *testJaws) newRequest(hr *http.Request) (tr *testRequest) { + if hr == nil { + hr = httptest.NewRequest(http.MethodGet, "/", nil) + } + ctx, cancel := context.WithTimeout(context.Background(), time.Hour) + hr = hr.WithContext(ctx) + rq := tj.NewRequest(hr) + if rq == nil || tj.UseRequest(rq.JawsKey, hr) != rq { + panic("failed to create or use jaws.Request") + } + bcastCh := tj.subscribe(rq, 64) + for i := 0; i <= cap(tj.subCh); i++ { + tj.subCh <- subscription{} // ensure subscription is processed + } + + tr = &testRequest{ + hr: hr, + jw: tj, + readyCh: make(chan struct{}), + doneCh: make(chan struct{}), + inCh: make(chan wsMsg), + outCh: make(chan string, cap(bcastCh)), + bcastCh: bcastCh, + ctx: ctx, + cancel: cancel, + Request: rq, + } + + go func() { + defer func() { + if tr.expectPanic { + if tr.panicVal = recover(); tr.panicVal != nil { + tr.panicked = true + } + } + close(tr.doneCh) + }() + close(tr.readyCh) + tr.process(tr.bcastCh, tr.inCh, tr.outCh) // usubs from bcase, closes outCh + tr.recycle() + }() + + return +} + +func (tr *testRequest) Close() { + tr.cancel() + tr.jw.Close() +} + +func newTestRequest() (tr *testRequest) { + tj := newTestJaws() + return tj.newRequest(nil) +}