~emersion/soju-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
2

[PATCH v2] Fix clearing webpush targets after any MARKREAD

Details
Message ID
<20221024125635.6893-1-delthas@dille.cc>
DKIM signature
missing
Download raw message
Patch: +19 -7
Previously, we would clear webpush targets after any MARKREAD.

Consider the following scenario (ignore any typos, this is crafted by
hand):

    <<< @time=2020-01-01T00:00:00Z PRIVMSG #foo :hi mark!
    <<< @time=2020-01-02T00:00:00Z PRIVMSG #foo :hi again mark!
    >>> MARKREAD #foo timestamp=2020-01-01T00:00:00Z
    >>> MARKREAD #foo timestamp=2020-01-02T00:00:00Z

The push target was previously cleared on the first MARKREAD, which
means that the second MARKREAD was never broadcast to Firebase, and all
devices would keep the "hi again mark!" notification indefinitely.

This changes the webpush target map so that we store a timestamp of the
last highlight we sent. We only clear the push target when sending a
MARKREAD that is at or after the last message.
---
Addressed the forEach comment.
As for reading msg.Tags["time"], we always set it if it is not already 
set, at the start of the handleMessageRegistered function; so this 
should work even when the upstream doesn't support server-time.

 downstream.go |  4 +++-
 irc.go        | 14 +++++++++++---
 upstream.go   |  4 +++-
 user.go       |  4 ++--
 4 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/downstream.go b/downstream.go
index 12dcaff..d8b9c4f 100644
--- a/downstream.go
+++ b/downstream.go
@@ -2818,7 +2818,9 @@ func (dc *downstreamConn) handleMessageRegistered(ctx context.Context, msg *irc.

		if broadcast && network.pushTargets.Has(target) {
			// TODO: only broadcast if draft/read-marker has been negotiated
			network.pushTargets.Del(target)
			if !r.Timestamp.Before(network.pushTargets.Get(target)) {
				network.pushTargets.Del(target)
			}
			go network.broadcastWebPush(&irc.Message{
				Command: "MARKREAD",
				Params:  []string{target, timestampStr},
diff --git a/irc.go b/irc.go
index bb17d64..33bd192 100644
--- a/irc.go
+++ b/irc.go
@@ -459,10 +459,18 @@ func (cm *monitorCasemapMap) ForEach(f func(name string, online bool)) {
	}
}

type casemapSet struct{ casemapMap }
type pushTargetCasemapMap struct{ casemapMap }

func (cs *casemapSet) Add(name string) {
	cs.set(name, nil)
func (cm *pushTargetCasemapMap) Get(name string) (last time.Time) {
	if v := cm.get(name); v == nil {
		return time.Time{}
	} else {
		return v.(time.Time)
	}
}

func (cm *pushTargetCasemapMap) Set(name string, last time.Time) {
	cm.set(name, last)
}

func isWordBoundary(r rune) bool {
diff --git a/upstream.go b/upstream.go
index e1030a2..180da98 100644
--- a/upstream.go
+++ b/upstream.go
@@ -551,7 +551,9 @@ func (uc *upstreamConn) handleMessage(ctx context.Context, msg *irc.Message) err

		if highlight || uc.isOurNick(target) {
			go uc.network.broadcastWebPush(msg)
			uc.network.pushTargets.Add(bufferName)
			if timestamp, err := time.Parse(xirc.ServerTimeLayout, string(msg.Tags["time"])); err == nil {
				uc.network.pushTargets.Set(bufferName, timestamp)
			}
		}

		uc.produce(bufferName, msg, downstreamID)
diff --git a/user.go b/user.go
index 7468483..3c8dd9a 100644
--- a/user.go
+++ b/user.go
@@ -139,7 +139,7 @@ type network struct {
	conn        *upstreamConn
	channels    channelCasemapMap
	delivered   deliveredStore
	pushTargets casemapSet
	pushTargets pushTargetCasemapMap
	lastError   error
	casemap     casemapping
}
@@ -160,7 +160,7 @@ func newNetwork(user *user, record *database.Network, channels []database.Channe
		stopped:     make(chan struct{}),
		channels:    m,
		delivered:   newDeliveredStore(),
		pushTargets: casemapSet{newCasemapMap()},
		pushTargets: pushTargetCasemapMap{newCasemapMap()},
		casemap:     casemapRFC1459,
	}
}

base-commit: 926dcb37ac688171edd2a18cf77b3b1f64a541ef
-- 
2.17.1
Details
Message ID
<ZwabRQq4jJdqMUn3ZEYl7jWYGrVX5QNtUtlFsRz_Iz5FSQCYmdIX2OFdMdFVno4ndblWXZvVa1MJmPp2tMM-jno0Yguwuq7uhv48d8BvMLk=@emersion.fr>
In-Reply-To
<20221024125635.6893-1-delthas@dille.cc> (view parent)
DKIM signature
missing
Download raw message
This comment still hasn't been addressed:

> Hm, what happens if upstream doesn't support server-time? We should probably
> fall back to time.Time or something right?
Details
Message ID
<NzuJBWC2ysq2zbjSvanRVWihXveujKI6f8k0fIebMXiRNb1AOCnHenqQVZd6BJC_C_zDwzz49FfatNlAlORiErBCBwemmRnA3pVgv0b9jEE=@emersion.fr>
In-Reply-To
<ZwabRQq4jJdqMUn3ZEYl7jWYGrVX5QNtUtlFsRz_Iz5FSQCYmdIX2OFdMdFVno4ndblWXZvVa1MJmPp2tMM-jno0Yguwuq7uhv48d8BvMLk=@emersion.fr> (view parent)
DKIM signature
missing
Download raw message
On Monday, October 24th, 2022 at 15:37, Simon Ser <contact@emersion.fr> wrote:

> This comment still hasn't been addressed:
> 
> > Hm, what happens if upstream doesn't support server-time? We should probably
> > fall back to time.Time or something right?

This is addressed by "Truncate message times to the second when using
the FS message store".
Reply to thread Export thread (mbox)