diff --git a/server/topic.go b/server/topic.go index 476f28a3..32b0d4b6 100644 --- a/server/topic.go +++ b/server/topic.go @@ -45,24 +45,16 @@ func newTopic(id string) *topic { } // Subscribe subscribes to this topic -func (t *topic) Subscribe(s subscriber, userID string, cancel func()) int { - max_retries := 5 - retries := 1 +func (t *topic) Subscribe(s subscriber, userID string, cancel func()) (subscriberID int) { t.mu.Lock() defer t.mu.Unlock() - - subscriberID := rand.Int() - // simple check for existing id in maps - for { - _, ok := t.subscribers[subscriberID] - if ok && retries <= max_retries { - subscriberID = rand.Int() - retries++ - } else { + for i := 0; i < 5; i++ { // Best effort retry + subscriberID = rand.Int() + _, exists := t.subscribers[subscriberID] + if !exists { break } } - t.subscribers[subscriberID] = &topicSubscriber{ userID: userID, // May be empty subscriber: s, diff --git a/server/topic_test.go b/server/topic_test.go index 400f4b6a..3fed46c2 100644 --- a/server/topic_test.go +++ b/server/topic_test.go @@ -42,12 +42,11 @@ func TestTopic_Keepalive(t *testing.T) { require.True(t, to.LastAccess().Unix() <= time.Now().Unix()+2) } -func TestTopic_Subscribe_duplicateID(t *testing.T) { +func TestTopic_Subscribe_DuplicateID(t *testing.T) { t.Parallel() - to := newTopic("mytopic") - // fix random seed to force same number generation + // Fix random seed to force same number generation rand.Seed(1) a := rand.Int() to.subscribers[a] = &topicSubscriber{ @@ -60,11 +59,11 @@ func TestTopic_Subscribe_duplicateID(t *testing.T) { return nil } - // force rand.Int to generate the same id once more + // Force rand.Int to generate the same id once more rand.Seed(1) id := to.Subscribe(subFn, "b", func() {}) res := to.subscribers[id] - require.False(t, id == a) - require.True(t, res.userID == "b") + require.NotEqual(t, id, a) + require.Equal(t, "b", res.userID, "b") }