Skip to content

Commit

Permalink
Fixed (*Sublist).HasInterest() (#5353)
Browse files Browse the repository at this point in the history
I made an incorrect comment previously about not needing to check the
cache's result to know if there is a match. I added a test for that, but
also discovered that there was an issue where `HasInterest()` would
incorrectly return true if checking for `foo` when there is only a
subscription on `foo.*`. Added more tests.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
  • Loading branch information
derekcollison committed Apr 25, 2024
2 parents df067f2 + 51ba21e commit bb9bf95
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 3 deletions.
8 changes: 5 additions & 3 deletions server/sublist.go
Expand Up @@ -621,7 +621,9 @@ func (s *Sublist) hasInterest(subject string, doLock bool) bool {
}
var matched bool
if s.cache != nil {
_, matched = s.cache[subject]
if r, ok := s.cache[subject]; ok {
matched = len(r.psubs)+len(r.qsubs) > 0
}
}
if doLock {
s.RUnlock()
Expand Down Expand Up @@ -789,10 +791,10 @@ func matchLevelForAny(l *level, toks []string) bool {
}
}
if n != nil {
return true
return len(n.plist) > 0 || len(n.psubs) > 0 || len(n.qsubs) > 0
}
if pwc != nil {
return true
return len(pwc.plist) > 0 || len(pwc.psubs) > 0 || len(pwc.qsubs) > 0
}
return false
}
Expand Down
144 changes: 144 additions & 0 deletions server/sublist_test.go
Expand Up @@ -1622,6 +1622,150 @@ func TestSublistHasInterest(t *testing.T) {
require_True(t, sl.HasInterest("foo"))
require_Equal(t, sl.cacheHits, i)
}

// Call Match on a subject we know there is no match.
sl.Match("bar")
require_False(t, sl.HasInterest("bar"))

// Remove fooSub and check interest again
sl.Remove(fooSub)
require_False(t, sl.HasInterest("foo"))

// Try with some wildcards
sub := newSub("foo.*")
sl.Insert(sub)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_False(t, sl.HasInterest("foo.bar.baz"))

// Remove sub, there should be no interest
sl.Remove(sub)
require_False(t, sl.HasInterest("foo"))
require_False(t, sl.HasInterest("foo.bar"))
require_False(t, sl.HasInterest("foo.bar.baz"))

sub = newSub("foo.>")
sl.Insert(sub)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_True(t, sl.HasInterest("foo.bar.baz"))

sl.Remove(sub)
require_False(t, sl.HasInterest("foo"))
require_False(t, sl.HasInterest("foo.bar"))
require_False(t, sl.HasInterest("foo.bar.baz"))

sub = newSub("*.>")
sl.Insert(sub)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_True(t, sl.HasInterest("foo.baz"))
sl.Remove(sub)

sub = newSub("*.bar")
sl.Insert(sub)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_False(t, sl.HasInterest("foo.baz"))
sl.Remove(sub)

sub = newSub("*")
sl.Insert(sub)
require_True(t, sl.HasInterest("foo"))
require_False(t, sl.HasInterest("foo.bar"))
sl.Remove(sub)

// Try with queues now.
qsub := newQSub("foo", "bar")
sl.Insert(qsub)
require_True(t, sl.HasInterest("foo"))
require_False(t, sl.HasInterest("foo.bar"))

qsub2 := newQSub("foo", "baz")
sl.Insert(qsub2)
require_True(t, sl.HasInterest("foo"))
require_False(t, sl.HasInterest("foo.bar"))

// Remove first queue
sl.Remove(qsub)
require_True(t, sl.HasInterest("foo"))
require_False(t, sl.HasInterest("foo.bar"))

// Remove last.
sl.Remove(qsub2)
require_False(t, sl.HasInterest("foo"))
require_False(t, sl.HasInterest("foo.bar"))

// With wildcards now
qsub = newQSub("foo.*", "bar")
sl.Insert(qsub)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_False(t, sl.HasInterest("foo.bar.baz"))

// Add another queue to the group
qsub2 = newQSub("foo.*", "baz")
sl.Insert(qsub2)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_False(t, sl.HasInterest("foo.bar.baz"))

// Remove first queue
sl.Remove(qsub)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_False(t, sl.HasInterest("foo.bar.baz"))

// Remove last
sl.Remove(qsub2)
require_False(t, sl.HasInterest("foo"))
require_False(t, sl.HasInterest("foo.bar"))
require_False(t, sl.HasInterest("foo.bar.baz"))

qsub = newQSub("foo.>", "bar")
sl.Insert(qsub)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_True(t, sl.HasInterest("foo.bar.baz"))

// Add another queue to the group
qsub2 = newQSub("foo.>", "baz")
sl.Insert(qsub2)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_True(t, sl.HasInterest("foo.bar.baz"))

// Remove first queue
sl.Remove(qsub)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_True(t, sl.HasInterest("foo.bar.baz"))

// Remove last
sl.Remove(qsub2)
require_False(t, sl.HasInterest("foo"))
require_False(t, sl.HasInterest("foo.bar"))
require_False(t, sl.HasInterest("foo.bar.baz"))

qsub = newQSub("*.>", "bar")
sl.Insert(qsub)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_True(t, sl.HasInterest("foo.baz"))
sl.Remove(qsub)

qsub = newQSub("*.bar", "bar")
sl.Insert(qsub)
require_False(t, sl.HasInterest("foo"))
require_True(t, sl.HasInterest("foo.bar"))
require_False(t, sl.HasInterest("foo.baz"))
sl.Remove(qsub)

qsub = newQSub("*", "bar")
sl.Insert(qsub)
require_True(t, sl.HasInterest("foo"))
require_False(t, sl.HasInterest("foo.bar"))
sl.Remove(qsub)
}

func subsInit(pre string, toks []string) {
Expand Down

0 comments on commit bb9bf95

Please sign in to comment.