diff --git a/go/chat/convsource.go b/go/chat/convsource.go index 4b038f70f4c6..4b8d0c9b066e 100644 --- a/go/chat/convsource.go +++ b/go/chat/convsource.go @@ -133,8 +133,11 @@ func (s *baseConversationSource) postProcessThread(ctx context.Context, uid greg // Resolve supersedes if q == nil || !q.DisableResolveSupersedes { + deletePlaceholders := q != nil && q.EnableDeletePlaceholders if superXform == nil { - superXform = newBasicSupersedesTransform(s.G()) + superXform = newBasicSupersedesTransform(s.G(), basicSupersedesTransformOpts{ + UseDeletePlaceholders: deletePlaceholders, + }) } if thread.Messages, err = superXform.Run(ctx, conv, uid, thread.Messages); err != nil { return err @@ -165,7 +168,7 @@ func (s *baseConversationSource) postProcessThread(ctx context.Context, uid greg func (s *baseConversationSource) TransformSupersedes(ctx context.Context, unboxInfo types.UnboxConversationInfo, uid gregor1.UID, msgs []chat1.MessageUnboxed) ([]chat1.MessageUnboxed, error) { - transform := newBasicSupersedesTransform(s.G()) + transform := newBasicSupersedesTransform(s.G(), basicSupersedesTransformOpts{}) return transform.Run(ctx, unboxInfo, uid, msgs) } @@ -660,7 +663,7 @@ func (s *HybridConversationSource) identifyTLF(ctx context.Context, conv types.U } func (s *HybridConversationSource) resolveHoles(ctx context.Context, uid gregor1.UID, - thread *chat1.ThreadView, conv chat1.Conversation, reason chat1.GetThreadReason) (err error) { + thread *chat1.ThreadView, conv chat1.Conversation, reason chat1.GetThreadReason) (holesFilled int, err error) { defer s.Trace(ctx, func() error { return err }, "resolveHoles")() var msgIDs []chat1.MessageID // Gather all placeholder messages so we can go fetch them @@ -673,47 +676,28 @@ func (s *HybridConversationSource) resolveHoles(ctx context.Context, uid gregor1 if index == len(thread.Messages)-1 { // If the last message is a hole, we might not have fetched everything, // so fail this case like a normal miss - return storage.MissError{} + return 0, storage.MissError{} } msgIDs = append(msgIDs, msg.GetMessageID()) } } if len(msgIDs) == 0 { // Nothing to do - return nil + return 0, nil } if s.IsOffline(ctx) { // Don't attempt if we are offline - return OfflineError{} + return 0, OfflineError{} } // Fetch all missing messages from server, and sub in the real ones into the placeholder slots msgs, err := s.GetMessages(ctx, conv, uid, msgIDs, &reason) if err != nil { s.Debug(ctx, "resolveHoles: failed to get missing messages: %s", err.Error()) - return err - } - msgLookup := make(map[chat1.MessageID]chat1.MessageUnboxed) - for _, msg := range msgs { - msgLookup[msg.GetMessageID()] = msg - } - for i, threadMsg := range thread.Messages { - state, err := threadMsg.State() - if err != nil { - continue - } - if state == chat1.MessageUnboxedState_PLACEHOLDER { - if msg, ok := msgLookup[threadMsg.GetMessageID()]; ok { - thread.Messages[i] = msg - } else { - s.Debug(ctx, "resolveHoles: did not fetch all placeholder messages, missing msgID: %d", - threadMsg.GetMessageID()) - return fmt.Errorf("did not fetch all placeholder messages") - } - } + return 0, err } s.Debug(ctx, "resolveHoles: success: filled %d holes", len(msgs)) - return nil + return len(msgs), nil } // maxHolesForPull is the number of misses in the body storage cache we will tolerate missing. A good @@ -738,6 +722,7 @@ func (s *HybridConversationSource) Pull(ctx context.Context, convID chat1.Conver if err == nil { unboxConv = conv // Try locally first + var holesFilled int rc := storage.NewHoleyResultCollector(maxHolesForPull, s.storage.ResultCollectorFromQuery(ctx, query, pagination)) thread, err = s.fetchMaybeNotify(ctx, conv.GetConvID(), uid, rc, conv.ReaderInfo.MaxMsgid, @@ -745,9 +730,15 @@ func (s *HybridConversationSource) Pull(ctx context.Context, convID chat1.Conver if err == nil { // Since we are using the "holey" collector, we need to resolve any placeholder // messages that may have been fetched. - s.Debug(ctx, "Pull: cache hit: convID: %s uid: %s holes: %d msgs: %d", unboxConv.GetConvID(), uid, - rc.Holes(), len(thread.Messages)) - err = s.resolveHoles(ctx, uid, &thread, conv, reason) + s.Debug(ctx, "Pull: (holey) cache hit: convID: %s uid: %s holes: %d msgs: %d", + unboxConv.GetConvID(), uid, rc.Holes(), len(thread.Messages)) + holesFilled, err = s.resolveHoles(ctx, uid, &thread, conv, reason) + } + if err == nil && holesFilled > 0 { + s.Debug(ctx, "Pull: %d holes filled, refetching from storage") + rc := s.storage.ResultCollectorFromQuery(ctx, query, pagination) + thread, err = s.fetchMaybeNotify(ctx, conv.GetConvID(), uid, rc, conv.ReaderInfo.MaxMsgid, + query, pagination) } if err == nil { // Do online only things @@ -830,8 +821,7 @@ func (s *HybridConversationSource) Pull(ctx context.Context, convID chat1.Conver } type pullLocalResultCollector struct { - *storage.SimpleResultCollector - num int + storage.ResultCollector } func (p *pullLocalResultCollector) Name() string { @@ -839,7 +829,7 @@ func (p *pullLocalResultCollector) Name() string { } func (p *pullLocalResultCollector) String() string { - return fmt.Sprintf("[ %s: t: %d ]", p.Name(), p.num) + return fmt.Sprintf("[ %s: base: %s ]", p.Name(), p.ResultCollector) } func (p *pullLocalResultCollector) hasRealResults() bool { @@ -867,10 +857,9 @@ func (p *pullLocalResultCollector) Error(err storage.Error) storage.Error { return err } -func newPullLocalResultCollector(num int) *pullLocalResultCollector { +func newPullLocalResultCollector(baseRC storage.ResultCollector) *pullLocalResultCollector { return &pullLocalResultCollector{ - num: num, - SimpleResultCollector: storage.NewSimpleResultCollector(num), + ResultCollector: baseRC, } } @@ -885,7 +874,7 @@ func (s *HybridConversationSource) PullLocalOnly(ctx context.Context, convID cha // Post process thread before returning defer func() { if err == nil { - superXform := newBasicSupersedesTransform(s.G()) + superXform := newBasicSupersedesTransform(s.G(), basicSupersedesTransformOpts{}) superXform.SetMessagesFunc(func(ctx context.Context, conv types.UnboxConversationInfo, uid gregor1.UID, msgIDs []chat1.MessageID, _ *chat1.GetThreadReason) (res []chat1.MessageUnboxed, err error) { @@ -928,7 +917,9 @@ func (s *HybridConversationSource) PullLocalOnly(ctx context.Context, convID cha if pagination != nil { num = pagination.Num } - rc := storage.NewHoleyResultCollector(maxPlaceholders, newPullLocalResultCollector(num)) + baseRC := s.storage.ResultCollectorFromQuery(ctx, query, pagination) + baseRC.SetTarget(num) + rc := storage.NewHoleyResultCollector(maxPlaceholders, newPullLocalResultCollector(baseRC)) tv, err = s.fetchMaybeNotify(ctx, convID, uid, rc, iboxMaxMsgID, query, pagination) if err != nil { s.Debug(ctx, "PullLocalOnly: failed to fetch local messages: %s", err.Error()) diff --git a/go/chat/helper.go b/go/chat/helper.go index f8d0298b6802..a5c6489c6c0d 100644 --- a/go/chat/helper.go +++ b/go/chat/helper.go @@ -1232,7 +1232,6 @@ func (n *newConversationHelper) makeFirstMessage(ctx context.Context, triple cha } func CreateNameInfoSource(ctx context.Context, g *globals.Context, membersType chat1.ConversationMembersType) types.NameInfoSource { - g.GetLog().CDebugf(ctx, "createNameInfoSource: hi") if override := ctx.Value(nameInfoOverrideKey); override != nil { g.GetLog().CDebugf(ctx, "createNameInfoSource: using override: %T", override) return override.(types.NameInfoSource) diff --git a/go/chat/inboxsource.go b/go/chat/inboxsource.go index eb43c68e423d..57ee084bccdc 100644 --- a/go/chat/inboxsource.go +++ b/go/chat/inboxsource.go @@ -78,7 +78,8 @@ func NewBlockingLocalizer(g *globals.Context) *BlockingLocalizer { return &BlockingLocalizer{ Contextified: globals.NewContextified(g), baseLocalizer: newBaseLocalizer(g), - pipeline: newLocalizerPipeline(g, newBasicSupersedesTransform(g)), + pipeline: newLocalizerPipeline(g, + newBasicSupersedesTransform(g, basicSupersedesTransformOpts{})), } } @@ -124,9 +125,10 @@ func NewNonblockingLocalizer(g *globals.Context, localizeCb chan NonblockInboxRe Contextified: globals.NewContextified(g), DebugLabeler: utils.NewDebugLabeler(g.GetLog(), "NonblockingLocalizer", false), baseLocalizer: newBaseLocalizer(g), - pipeline: newLocalizerPipeline(g, newBasicSupersedesTransform(g)), - localizeCb: localizeCb, - maxUnbox: maxUnbox, + pipeline: newLocalizerPipeline(g, + newBasicSupersedesTransform(g, basicSupersedesTransformOpts{})), + localizeCb: localizeCb, + maxUnbox: maxUnbox, } } diff --git a/go/chat/server.go b/go/chat/server.go index e14b1f1104e2..4dac83aca0ff 100644 --- a/go/chat/server.go +++ b/go/chat/server.go @@ -482,28 +482,28 @@ func (h *Server) mergeLocalRemoteThread(ctx context.Context, remoteThread, local if state == chat1.MessageUnboxedState_PLACEHOLDER && !rm[m.GetMessageID()] { h.Debug(ctx, "mergeLocalRemoteThread: subbing in dead placeholder: msgID: %d", m.GetMessageID()) - res.Messages = append(res.Messages, chat1.NewMessageUnboxedWithPlaceholder( - chat1.MessageUnboxedPlaceholder{ - MessageID: m.GetMessageID(), - Hidden: true, - }, - )) + res.Messages = append(res.Messages, utils.CreateHiddenPlaceholder(m.GetMessageID())) } } sort.Sort(utils.ByMsgUnboxedMsgID(res.Messages)) }() - shouldAppend := func(oldMsg chat1.MessageUnboxed) bool { - state, err := oldMsg.State() - if err != nil { + shouldAppend := func(newMsg chat1.MessageUnboxed, oldMsgs map[chat1.MessageID]chat1.MessageUnboxed) bool { + oldMsg, ok := oldMsgs[newMsg.GetMessageID()] + if !ok { return true } - switch state { - case chat1.MessageUnboxedState_VALID: - return false - default: + // If either message is not valid, return the new one, something weird might be going on + if !oldMsg.IsValid() || !newMsg.IsValid() { + return true + } + // If newMsg is now superseded by something different than what we sent, then let's include it + if newMsg.Valid().ServerHeader.SupersededBy != oldMsg.Valid().ServerHeader.SupersededBy { + h.Debug(ctx, "mergeLocalRemoteThread: including supersededBy change: msgID: %d", + newMsg.GetMessageID()) return true } + return false } switch mode { case chat1.GetThreadNonblockCbMode_FULL: @@ -516,8 +516,7 @@ func (h *Server) mergeLocalRemoteThread(ctx context.Context, remoteThread, local } res.Pagination = remoteThread.Pagination for _, m := range remoteThread.Messages { - oldMsg, ok := lm[m.GetMessageID()] - if !ok || shouldAppend(oldMsg) { + if shouldAppend(m, lm) { res.Messages = append(res.Messages, m) } } @@ -659,6 +658,12 @@ func (h *Server) GetThreadNonblock(ctx context.Context, arg chat1.GetThreadNonbl return res, err } + // Enable delete placeholders for supersede transform + if arg.Query == nil { + arg.Query = new(chat1.GetThreadQuery) + } + arg.Query.EnableDeletePlaceholders = true + // Xlate pager control into pagination if given if arg.Query != nil && arg.Query.MessageIDControl != nil { pagination = utils.XlateMessageIDControlToPagination(arg.Query.MessageIDControl) diff --git a/go/chat/server_test.go b/go/chat/server_test.go index 4d3a589fcc92..61077c34e97a 100644 --- a/go/chat/server_test.go +++ b/go/chat/server_test.go @@ -2667,31 +2667,185 @@ func confirmIsText(t *testing.T, msgID chat1.MessageID, msg chat1.UIMessage, tex require.Equal(t, text, msg.Valid().MessageBody.Text().Body) } -func TestChatSrvGetThreadNonblockPlaceholders(t *testing.T) { +func TestChatSrvGetThreadNonblockSupersedes(t *testing.T) { runWithMemberTypes(t, func(mt chat1.ConversationMembersType) { - ctc := makeChatTestContext(t, "GetThreadNonblockPlaceholders", 1) + ctc := makeChatTestContext(t, "GetThreadNonblockSupersedes", 1) defer ctc.cleanup() users := ctc.users() - editMsg := func(ctx context.Context, conv chat1.ConversationInfoLocal, msgID chat1.MessageID) chat1.MessageID { - postRes, err := ctc.as(t, users[0]).chatLocalHandler().PostLocal(ctx, chat1.PostLocalArg{ - ConversationID: conv.Id, - Msg: chat1.MessagePlaintext{ - ClientHeader: chat1.MessageClientHeader{ - Conv: conv.Triple, - MessageType: chat1.MessageType_EDIT, - TlfName: conv.TlfName, - Supersedes: msgID, - }, - MessageBody: chat1.NewMessageBodyWithEdit(chat1.MessageEdit{ - MessageID: msgID, - Body: "HI", - }), + uid := gregor1.UID(users[0].GetUID().ToBytes()) + inboxCb := make(chan kbtest.NonblockInboxResult, 100) + threadCb := make(chan kbtest.NonblockThreadResult, 100) + ui := kbtest.NewChatUI(inboxCb, threadCb, nil, nil) + ctc.as(t, users[0]).h.mockChatUI = ui + ctx := ctc.as(t, users[0]).startCtx + <-ctc.as(t, users[0]).h.G().ConvLoader.Stop(ctx) + listener := newServerChatListener() + ctc.as(t, users[0]).h.G().NotifyRouter.SetListener(listener) + + conv := mustCreateConversationForTest(t, ctc, users[0], chat1.TopicType_CHAT, mt) + cs := ctc.world.Tcs[users[0].Username].ChatG.ConvSource + msg := chat1.NewMessageBodyWithText(chat1.MessageText{Body: "hi"}) + msgID1 := mustPostLocalForTest(t, ctc, users[0], conv, msg) + consumeNewMsgRemote(t, listener, chat1.MessageType_TEXT) + msgRes, err := ctc.as(t, users[0]).chatLocalHandler().GetMessagesLocal(ctx, chat1.GetMessagesLocalArg{ + ConversationID: conv.Id, + MessageIDs: []chat1.MessageID{msgID1}, + DisableResolveSupersedes: true, + }) + require.NoError(t, err) + require.Equal(t, 1, len(msgRes.Messages)) + msg1 := msgRes.Messages[0] + editMsgID1 := mustEditMsg(ctx, t, ctc, users[0], conv, msgID1) + consumeNewMsgRemote(t, listener, chat1.MessageType_EDIT) + + msgIDs := []chat1.MessageID{editMsgID1, msgID1, 1} + require.NoError(t, cs.Clear(context.TODO(), conv.Id, uid)) + _, err = cs.PushUnboxed(ctx, conv.Id, uid, msg1) + require.NoError(t, err) + + delay := 10 * time.Minute + clock := clockwork.NewFakeClock() + ctc.as(t, users[0]).h.clock = clock + ctc.as(t, users[0]).h.remoteThreadDelay = &delay + cb := make(chan struct{}) + query := chat1.GetThreadQuery{ + MessageTypes: []chat1.MessageType{chat1.MessageType_TEXT}, + } + go func() { + _, err := ctc.as(t, users[0]).chatLocalHandler().GetThreadNonblock(ctx, + chat1.GetThreadNonblockArg{ + ConversationID: conv.Id, + Query: &query, + CbMode: chat1.GetThreadNonblockCbMode_INCREMENTAL, }, - }) + ) + require.NoError(t, err) + close(cb) + }() + select { + case res := <-threadCb: + require.False(t, res.Full) + require.Equal(t, len(msgIDs), len(res.Thread.Messages)) + require.Equal(t, msgIDs, utils.PluckUIMessageIDs(res.Thread.Messages)) + confirmIsText(t, msgID1, res.Thread.Messages[1], "hi") + require.False(t, res.Thread.Messages[1].Valid().Superseded) + confirmIsPlaceholder(t, editMsgID1, res.Thread.Messages[0], false) + confirmIsPlaceholder(t, 1, res.Thread.Messages[2], false) + case <-time.After(20 * time.Second): + require.Fail(t, "no thread cb") + } + clock.Advance(20 * time.Minute) + select { + case res := <-threadCb: + require.True(t, res.Full) + require.Equal(t, len(msgIDs), len(res.Thread.Messages)) + confirmIsPlaceholder(t, editMsgID1, res.Thread.Messages[0], true) + confirmIsText(t, msgID1, res.Thread.Messages[1], "HI") + confirmIsPlaceholder(t, 1, res.Thread.Messages[2], true) + case <-time.After(20 * time.Second): + require.Fail(t, "no thread cb") + } + select { + case <-cb: + case <-time.After(20 * time.Second): + require.Fail(t, "GetThread never finished") + } + + deleteMsgID := mustDeleteMsg(ctx, t, ctc, users[0], conv, msgID1) + consumeNewMsgRemote(t, listener, chat1.MessageType_DELETE) + msgIDs = []chat1.MessageID{deleteMsgID, editMsgID1, msgID1, 1} + require.NoError(t, cs.Clear(context.TODO(), conv.Id, uid)) + _, err = cs.PushUnboxed(ctx, conv.Id, uid, msg1) + require.NoError(t, err) + cb = make(chan struct{}) + go func() { + _, err := ctc.as(t, users[0]).chatLocalHandler().GetThreadNonblock(ctx, + chat1.GetThreadNonblockArg{ + ConversationID: conv.Id, + Query: &query, + CbMode: chat1.GetThreadNonblockCbMode_INCREMENTAL, + }, + ) require.NoError(t, err) - return postRes.MessageID + close(cb) + }() + select { + case res := <-threadCb: + require.False(t, res.Full) + require.Equal(t, len(msgIDs), len(res.Thread.Messages)) + require.Equal(t, msgIDs, utils.PluckUIMessageIDs(res.Thread.Messages)) + confirmIsPlaceholder(t, deleteMsgID, res.Thread.Messages[0], false) + confirmIsPlaceholder(t, editMsgID1, res.Thread.Messages[1], false) + confirmIsText(t, msgID1, res.Thread.Messages[2], "hi") + require.False(t, res.Thread.Messages[2].Valid().Superseded) + confirmIsPlaceholder(t, 1, res.Thread.Messages[3], false) + case <-time.After(20 * time.Second): + require.Fail(t, "no thread cb") + } + clock.Advance(20 * time.Minute) + select { + case res := <-threadCb: + require.True(t, res.Full) + require.Equal(t, len(msgIDs), len(res.Thread.Messages)) + confirmIsPlaceholder(t, deleteMsgID, res.Thread.Messages[0], true) + confirmIsPlaceholder(t, editMsgID1, res.Thread.Messages[1], true) + confirmIsPlaceholder(t, msgID1, res.Thread.Messages[2], true) + confirmIsPlaceholder(t, 1, res.Thread.Messages[3], true) + case <-time.After(20 * time.Second): + require.Fail(t, "no thread cb") } + select { + case <-cb: + case <-time.After(20 * time.Second): + require.Fail(t, "GetThread never finished") + } + }) +} + +func mustDeleteMsg(ctx context.Context, t *testing.T, ctc *chatTestContext, user *kbtest.FakeUser, + conv chat1.ConversationInfoLocal, msgID chat1.MessageID) chat1.MessageID { + postRes, err := ctc.as(t, user).chatLocalHandler().PostLocal(ctx, chat1.PostLocalArg{ + ConversationID: conv.Id, + Msg: chat1.MessagePlaintext{ + ClientHeader: chat1.MessageClientHeader{ + Conv: conv.Triple, + MessageType: chat1.MessageType_DELETE, + TlfName: conv.TlfName, + Supersedes: msgID, + }, + }, + }) + require.NoError(t, err) + return postRes.MessageID +} + +func mustEditMsg(ctx context.Context, t *testing.T, ctc *chatTestContext, user *kbtest.FakeUser, + conv chat1.ConversationInfoLocal, msgID chat1.MessageID) chat1.MessageID { + postRes, err := ctc.as(t, user).chatLocalHandler().PostLocal(ctx, chat1.PostLocalArg{ + ConversationID: conv.Id, + Msg: chat1.MessagePlaintext{ + ClientHeader: chat1.MessageClientHeader{ + Conv: conv.Triple, + MessageType: chat1.MessageType_EDIT, + TlfName: conv.TlfName, + Supersedes: msgID, + }, + MessageBody: chat1.NewMessageBodyWithEdit(chat1.MessageEdit{ + MessageID: msgID, + Body: "HI", + }), + }, + }) + require.NoError(t, err) + return postRes.MessageID +} + +func TestChatSrvGetThreadNonblockPlaceholders(t *testing.T) { + runWithMemberTypes(t, func(mt chat1.ConversationMembersType) { + ctc := makeChatTestContext(t, "GetThreadNonblockPlaceholders", 1) + defer ctc.cleanup() + users := ctc.users() uid := gregor1.UID(users[0].GetUID().ToBytes()) inboxCb := make(chan kbtest.NonblockInboxResult, 100) @@ -2708,11 +2862,11 @@ func TestChatSrvGetThreadNonblockPlaceholders(t *testing.T) { msg := chat1.NewMessageBodyWithText(chat1.MessageText{Body: "hi"}) msgID1 := mustPostLocalForTest(t, ctc, users[0], conv, msg) consumeNewMsgRemote(t, listener, chat1.MessageType_TEXT) - editMsgID1 := editMsg(ctx, conv, msgID1) + editMsgID1 := mustEditMsg(ctx, t, ctc, users[0], conv, msgID1) consumeNewMsgRemote(t, listener, chat1.MessageType_EDIT) msgID2 := mustPostLocalForTest(t, ctc, users[0], conv, msg) consumeNewMsgRemote(t, listener, chat1.MessageType_TEXT) - editMsgID2 := editMsg(ctx, conv, msgID2) + editMsgID2 := mustEditMsg(ctx, t, ctc, users[0], conv, msgID2) consumeNewMsgRemote(t, listener, chat1.MessageType_EDIT) msgID3 := mustPostLocalForTest(t, ctc, users[0], conv, msg) consumeNewMsgRemote(t, listener, chat1.MessageType_TEXT) diff --git a/go/chat/storage/storage.go b/go/chat/storage/storage.go index b144cbe5e689..61aed0d6b6d5 100644 --- a/go/chat/storage/storage.go +++ b/go/chat/storage/storage.go @@ -24,6 +24,7 @@ type ResultCollector interface { Result() []chat1.MessageUnboxed Error(err Error) Error Name() string + SetTarget(num int) String() string } @@ -161,6 +162,10 @@ func (s *SimpleResultCollector) PushPlaceholder(chat1.MessageID) bool { return false } +func (s *SimpleResultCollector) SetTarget(num int) { + s.target = num +} + func NewSimpleResultCollector(num int) *SimpleResultCollector { return &SimpleResultCollector{ target: num, @@ -203,6 +208,8 @@ func (s *InsatiableResultCollector) Error(err Error) Error { return err } +func (s *InsatiableResultCollector) SetTarget(num int) {} + func (s *InsatiableResultCollector) PushPlaceholder(chat1.MessageID) bool { // Missing messages are a-ok return true @@ -268,6 +275,10 @@ func (t *TypedResultCollector) PushPlaceholder(msgID chat1.MessageID) bool { return false } +func (t *TypedResultCollector) SetTarget(num int) { + t.target = num +} + type HoleyResultCollector struct { ResultCollector diff --git a/go/chat/supersedes.go b/go/chat/supersedes.go index aa424a35c932..45b258bc77d2 100644 --- a/go/chat/supersedes.go +++ b/go/chat/supersedes.go @@ -20,20 +20,26 @@ type supersedesTransform interface { type getMessagesFunc func(context.Context, types.UnboxConversationInfo, gregor1.UID, []chat1.MessageID, *chat1.GetThreadReason) ([]chat1.MessageUnboxed, error) +type basicSupersedesTransformOpts struct { + UseDeletePlaceholders bool +} + type basicSupersedesTransform struct { globals.Contextified utils.DebugLabeler messagesFunc getMessagesFunc + opts basicSupersedesTransformOpts } var _ supersedesTransform = (*basicSupersedesTransform)(nil) -func newBasicSupersedesTransform(g *globals.Context) *basicSupersedesTransform { +func newBasicSupersedesTransform(g *globals.Context, opts basicSupersedesTransformOpts) *basicSupersedesTransform { return &basicSupersedesTransform{ Contextified: globals.NewContextified(g), DebugLabeler: utils.NewDebugLabeler(g.GetLog(), "supersedesTransform", false), messagesFunc: g.ConvSource.GetMessages, + opts: opts, } } @@ -220,6 +226,11 @@ func (t *basicSupersedesTransform) Run(ctx context.Context, // Run through all messages and transform superseded messages into final state var newMsgs []chat1.MessageUnboxed + xformDelete := func(msgID chat1.MessageID) { + if t.opts.UseDeletePlaceholders { + newMsgs = append(newMsgs, utils.CreateHiddenPlaceholder(msgID)) + } + } for i, msg := range originalMsgs { if msg.IsValid() { newMsg := &originalMsgs[i] @@ -230,9 +241,12 @@ func (t *basicSupersedesTransform) Run(ctx context.Context, if newMsg == nil { // Transform might return nil in case of a delete. t.Debug(ctx, "skipping: %d because it was deleted", msg.GetMessageID()) + xformDelete(msg.GetMessageID()) continue } - if newMsg.GetMessageID() < deleteHistoryUpto && chat1.IsDeletableByDeleteHistory(newMsg.GetMessageType()) { + if newMsg.GetMessageID() < deleteHistoryUpto && + chat1.IsDeletableByDeleteHistory(newMsg.GetMessageType()) { + xformDelete(msg.GetMessageID()) continue } if !newMsg.IsValidFull() { @@ -243,6 +257,7 @@ func (t *basicSupersedesTransform) Run(ctx context.Context, mvalid := newMsg.Valid() if !mvalid.IsEphemeral() || mvalid.HideExplosion(conv.GetExpunge(), time.Now()) { t.Debug(ctx, "skipping: %d because not valid full", msg.GetMessageID()) + xformDelete(msg.GetMessageID()) continue } } diff --git a/go/chat/utils/utils.go b/go/chat/utils/utils.go index 90dcbca1cd47..ec3a6e3dbca8 100644 --- a/go/chat/utils/utils.go +++ b/go/chat/utils/utils.go @@ -1395,3 +1395,11 @@ func ForceReloadUPAKsForUIDs(ctx context.Context, g *globals.Context, uids []key } return g.GetUPAKLoader().Batcher(ctx, getArg, nil, 0) } + +func CreateHiddenPlaceholder(msgID chat1.MessageID) chat1.MessageUnboxed { + return chat1.NewMessageUnboxedWithPlaceholder( + chat1.MessageUnboxedPlaceholder{ + MessageID: msgID, + Hidden: true, + }) +} diff --git a/go/protocol/chat1/local.go b/go/protocol/chat1/local.go index c12da466d8ff..7ce7bbd28b7e 100644 --- a/go/protocol/chat1/local.go +++ b/go/protocol/chat1/local.go @@ -3074,6 +3074,7 @@ type GetThreadQuery struct { MarkAsRead bool `codec:"markAsRead" json:"markAsRead"` MessageTypes []MessageType `codec:"messageTypes" json:"messageTypes"` DisableResolveSupersedes bool `codec:"disableResolveSupersedes" json:"disableResolveSupersedes"` + EnableDeletePlaceholders bool `codec:"enableDeletePlaceholders" json:"enableDeletePlaceholders"` Before *gregor1.Time `codec:"before,omitempty" json:"before,omitempty"` After *gregor1.Time `codec:"after,omitempty" json:"after,omitempty"` MessageIDControl *MessageIDControl `codec:"messageIDControl,omitempty" json:"messageIDControl,omitempty"` @@ -3094,6 +3095,7 @@ func (o GetThreadQuery) DeepCopy() GetThreadQuery { return ret })(o.MessageTypes), DisableResolveSupersedes: o.DisableResolveSupersedes, + EnableDeletePlaceholders: o.EnableDeletePlaceholders, Before: (func(x *gregor1.Time) *gregor1.Time { if x == nil { return nil diff --git a/protocol/avdl/chat1/local.avdl b/protocol/avdl/chat1/local.avdl index 66dc10e08566..30d7db6d7322 100644 --- a/protocol/avdl/chat1/local.avdl +++ b/protocol/avdl/chat1/local.avdl @@ -574,6 +574,7 @@ protocol local { boolean markAsRead; array messageTypes; boolean disableResolveSupersedes; + boolean enableDeletePlaceholders; union { null, gregor1.Time } before; union { null, gregor1.Time } after; diff --git a/protocol/js/rpc-chat-gen.js b/protocol/js/rpc-chat-gen.js index c0d253138cf3..2d0a0a8efb46 100644 --- a/protocol/js/rpc-chat-gen.js +++ b/protocol/js/rpc-chat-gen.js @@ -475,7 +475,7 @@ export type GetThreadNonblockPgMode = | 0 // DEFAULT_0 | 1 // SERVER_1 -export type GetThreadQuery = $ReadOnly<{markAsRead: Boolean, messageTypes?: ?Array, disableResolveSupersedes: Boolean, before?: ?Gregor1.Time, after?: ?Gregor1.Time, messageIDControl?: ?MessageIDControl}> +export type GetThreadQuery = $ReadOnly<{markAsRead: Boolean, messageTypes?: ?Array, disableResolveSupersedes: Boolean, enableDeletePlaceholders: Boolean, before?: ?Gregor1.Time, after?: ?Gregor1.Time, messageIDControl?: ?MessageIDControl}> export type GetThreadReason = | 0 // GENERAL_0 | 1 // PUSH_1 diff --git a/protocol/json/chat1/local.json b/protocol/json/chat1/local.json index b9c0135e3a42..799e55645110 100644 --- a/protocol/json/chat1/local.json +++ b/protocol/json/chat1/local.json @@ -1727,6 +1727,10 @@ "type": "boolean", "name": "disableResolveSupersedes" }, + { + "type": "boolean", + "name": "enableDeletePlaceholders" + }, { "type": [ null, diff --git a/shared/actions/chat2/index.js b/shared/actions/chat2/index.js index ed26c5d3b401..6535286048a0 100644 --- a/shared/actions/chat2/index.js +++ b/shared/actions/chat2/index.js @@ -933,6 +933,7 @@ const loadMoreMessages = ( }, pgmode: RPCChatTypes.localGetThreadNonblockPgMode.server, query: { + enableDeletePlaceholders: true, disableResolveSupersedes: false, markAsRead: false, messageTypes: loadThreadMessageTypes, diff --git a/shared/constants/types/rpc-chat-gen.js b/shared/constants/types/rpc-chat-gen.js index c0d253138cf3..2d0a0a8efb46 100644 --- a/shared/constants/types/rpc-chat-gen.js +++ b/shared/constants/types/rpc-chat-gen.js @@ -475,7 +475,7 @@ export type GetThreadNonblockPgMode = | 0 // DEFAULT_0 | 1 // SERVER_1 -export type GetThreadQuery = $ReadOnly<{markAsRead: Boolean, messageTypes?: ?Array, disableResolveSupersedes: Boolean, before?: ?Gregor1.Time, after?: ?Gregor1.Time, messageIDControl?: ?MessageIDControl}> +export type GetThreadQuery = $ReadOnly<{markAsRead: Boolean, messageTypes?: ?Array, disableResolveSupersedes: Boolean, enableDeletePlaceholders: Boolean, before?: ?Gregor1.Time, after?: ?Gregor1.Time, messageIDControl?: ?MessageIDControl}> export type GetThreadReason = | 0 // GENERAL_0 | 1 // PUSH_1