From a34fda984dcee3eed7e2a54eb5849e8d3947616d Mon Sep 17 00:00:00 2001 From: purian23 Date: Wed, 3 Jun 2026 03:17:02 -0400 Subject: [PATCH] fix(Clipboard): stale image entry handling - Resolved random DMS API errors & QML Warnings --- core/internal/server/clipboard/handlers.go | 5 + core/internal/server/clipboard/manager.go | 21 ++- .../internal/server/clipboard/manager_test.go | 141 +++++++++++++++++- .../Modals/Clipboard/ClipboardEditor.qml | 4 + .../Modals/Clipboard/ClipboardThumbnail.qml | 131 ++++++++++++++-- .../ClipboardLauncherPreview.qml | 6 +- 6 files changed, 290 insertions(+), 18 deletions(-) diff --git a/core/internal/server/clipboard/handlers.go b/core/internal/server/clipboard/handlers.go index 1ccc7104..1bfd5994 100644 --- a/core/internal/server/clipboard/handlers.go +++ b/core/internal/server/clipboard/handlers.go @@ -2,6 +2,7 @@ package clipboard import ( "encoding/json" + "errors" "fmt" "net" @@ -73,6 +74,10 @@ func handleGetEntry(conn net.Conn, req models.Request, m *Manager) { entry, err := m.GetEntry(uint64(id)) if err != nil { + if errors.Is(err, errEntryNotFound) { + models.Respond[any](conn, req.ID, nil) + return + } models.RespondError(conn, req.ID, err.Error()) return } diff --git a/core/internal/server/clipboard/manager.go b/core/internal/server/clipboard/manager.go index a07b21c3..69c65bc6 100644 --- a/core/internal/server/clipboard/manager.go +++ b/core/internal/server/clipboard/manager.go @@ -3,6 +3,7 @@ package clipboard import ( "bytes" "encoding/binary" + "errors" "fmt" "image" _ "image/gif" @@ -34,6 +35,8 @@ import ( wlclient "github.com/AvengeMedia/DankMaterialShell/core/pkg/go-wayland/wayland/client" ) +var errEntryNotFound = errors.New("entry not found") + // These mime types won't be stored in history var sensitiveMimeTypes = []string{ "x-kde-passwordManagerHint", @@ -764,9 +767,25 @@ func stateEqual(a, b *State) bool { if len(a.History) != len(b.History) { return false } + for i := range a.History { + if !entryStateEqual(a.History[i], b.History[i]) { + return false + } + } return true } +func entryStateEqual(a, b Entry) bool { + return a.ID == b.ID && + a.Hash == b.Hash && + a.Pinned == b.Pinned && + a.IsImage == b.IsImage && + a.MimeType == b.MimeType && + a.Preview == b.Preview && + a.Size == b.Size && + a.Timestamp.Equal(b.Timestamp) +} + func (m *Manager) GetHistory() []Entry { if m.db == nil { return nil @@ -854,7 +873,7 @@ func (m *Manager) GetEntry(id uint64) (*Entry, error) { return nil, err } if !found { - return nil, fmt.Errorf("entry not found") + return nil, errEntryNotFound } return &entry, nil diff --git a/core/internal/server/clipboard/manager_test.go b/core/internal/server/clipboard/manager_test.go index 9d498df4..372babe6 100644 --- a/core/internal/server/clipboard/manager_test.go +++ b/core/internal/server/clipboard/manager_test.go @@ -1,17 +1,52 @@ package clipboard import ( + "bytes" + "encoding/json" + "net" + "path/filepath" "sync" "sync/atomic" "testing" "time" + "github.com/AvengeMedia/DankMaterialShell/core/internal/server/models" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" mocks_wlcontext "github.com/AvengeMedia/DankMaterialShell/core/internal/mocks/wlcontext" ) +type clipboardTestConn struct { + net.Conn + writeBuf *bytes.Buffer +} + +func newClipboardTestConn() *clipboardTestConn { + return &clipboardTestConn{writeBuf: &bytes.Buffer{}} +} + +func (c *clipboardTestConn) Write(b []byte) (int, error) { + return c.writeBuf.Write(b) +} + +func newTestManagerWithDB(t *testing.T) *Manager { + t.Helper() + + db, err := openDB(filepath.Join(t.TempDir(), "clipboard.db")) + require.NoError(t, err) + + t.Cleanup(func() { + db.Close() + }) + + return &Manager{ + config: DefaultConfig(), + db: db, + } +} + func TestEncodeDecodeEntry_Roundtrip(t *testing.T) { original := Entry{ ID: 12345, @@ -131,11 +166,113 @@ func TestStateEqual_HistoryLengthDiffers(t *testing.T) { } func TestStateEqual_BothEqual(t *testing.T) { - a := &State{Enabled: true, History: []Entry{{ID: 1}, {ID: 2}}} - b := &State{Enabled: true, History: []Entry{{ID: 3}, {ID: 4}}} + ts := time.Now().Truncate(time.Second) + entry := Entry{ + ID: 1, + Hash: 100, + MimeType: "image/png", + Preview: "[[ image 1 KiB png 32x32 ]]", + Size: 1024, + Timestamp: ts, + IsImage: true, + Pinned: true, + } + a := &State{Enabled: true, History: []Entry{entry}} + b := &State{Enabled: true, History: []Entry{entry}} assert.True(t, stateEqual(a, b)) } +func TestStateEqual_SameLengthDifferentIDs(t *testing.T) { + ts := time.Now().Truncate(time.Second) + a := &State{Enabled: true, History: []Entry{{ID: 1, Hash: 100, Timestamp: ts}}} + b := &State{Enabled: true, History: []Entry{{ID: 2, Hash: 100, Timestamp: ts}}} + + assert.False(t, stateEqual(a, b)) +} + +func TestStateEqual_MetadataDiffers(t *testing.T) { + ts := time.Now().Truncate(time.Second) + base := Entry{ + ID: 1, + Hash: 100, + MimeType: "image/png", + Preview: "[[ image 1 KiB png 32x32 ]]", + Size: 1024, + Timestamp: ts, + IsImage: true, + Pinned: false, + } + + tests := []struct { + name string + mutate func(*Entry) + }{ + {name: "hash", mutate: func(e *Entry) { e.Hash = 101 }}, + {name: "pinned", mutate: func(e *Entry) { e.Pinned = true }}, + {name: "is image", mutate: func(e *Entry) { e.IsImage = false }}, + {name: "mime type", mutate: func(e *Entry) { e.MimeType = "image/jpeg" }}, + {name: "preview", mutate: func(e *Entry) { e.Preview = "[[ image 2 KiB jpeg 64x64 ]]" }}, + {name: "size", mutate: func(e *Entry) { e.Size = 2048 }}, + {name: "timestamp", mutate: func(e *Entry) { e.Timestamp = ts.Add(time.Second) }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + changed := base + tt.mutate(&changed) + + a := &State{Enabled: true, History: []Entry{base}} + b := &State{Enabled: true, History: []Entry{changed}} + + assert.False(t, stateEqual(a, b)) + }) + } +} + +func TestHandleGetEntry_ReturnsExistingEntry(t *testing.T) { + m := newTestManagerWithDB(t) + err := m.storeEntry(Entry{ + Data: []byte("hello world"), + MimeType: "text/plain;charset=utf-8", + Preview: "hello world", + Size: len("hello world"), + Timestamp: time.Now().Truncate(time.Second), + IsImage: false, + }) + require.NoError(t, err) + + history := m.GetHistory() + require.Len(t, history, 1) + + conn := newClipboardTestConn() + handleGetEntry(conn, models.Request{ + ID: 1, + Params: map[string]any{"id": float64(history[0].ID)}, + }, m) + + var resp models.Response[Entry] + require.NoError(t, json.NewDecoder(conn.writeBuf).Decode(&resp)) + assert.Empty(t, resp.Error) + require.NotNil(t, resp.Result) + assert.Equal(t, history[0].ID, resp.Result.ID) + assert.Equal(t, []byte("hello world"), resp.Result.Data) +} + +func TestHandleGetEntry_MissingIDReturnsNullResult(t *testing.T) { + m := newTestManagerWithDB(t) + conn := newClipboardTestConn() + + handleGetEntry(conn, models.Request{ + ID: 1, + Params: map[string]any{"id": float64(999)}, + }, m) + + var resp models.Response[any] + require.NoError(t, json.NewDecoder(conn.writeBuf).Decode(&resp)) + assert.Empty(t, resp.Error) + assert.Nil(t, resp.Result) +} + func TestManager_ConcurrentSubscriberAccess(t *testing.T) { m := &Manager{ subscribers: make(map[string]chan State), diff --git a/quickshell/Modals/Clipboard/ClipboardEditor.qml b/quickshell/Modals/Clipboard/ClipboardEditor.qml index 6d387952..37207131 100644 --- a/quickshell/Modals/Clipboard/ClipboardEditor.qml +++ b/quickshell/Modals/Clipboard/ClipboardEditor.qml @@ -89,6 +89,10 @@ Item { if (!root.entry || root.entry.id !== requestedId) { return; } + if (!response.result) { + ClipboardService.refresh(); + return; + } const result = response.result; let fullText = ""; if (result?.data) { diff --git a/quickshell/Modals/Clipboard/ClipboardThumbnail.qml b/quickshell/Modals/Clipboard/ClipboardThumbnail.qml index 10cf2ed3..7e9197b3 100644 --- a/quickshell/Modals/Clipboard/ClipboardThumbnail.qml +++ b/quickshell/Modals/Clipboard/ClipboardThumbnail.qml @@ -13,6 +13,7 @@ Item { required property var modal required property var listView required property int itemIndex + property bool disposed: false Image { id: thumbnailImage @@ -20,6 +21,13 @@ Item { property bool isVisible: false property string cachedImageData: "" property bool loadQueued: false + property bool activeLoad: false + property bool completed: false + property int loadGeneration: 0 + property var activeEntryId: null + property var activeRequest: null + property var currentEntryId: entry && entry.id !== undefined ? entry.id : null + property string currentEntryType: entryType anchors.fill: parent source: cachedImageData ? `data:image/png;base64,${cachedImageData}` : "" @@ -31,29 +39,119 @@ Item { sourceSize.width: 128 sourceSize.height: 128 + onCurrentEntryIdChanged: { + if (thumbnailImage.completed) { + thumbnailImage.resetForEntry(); + } + } + + onCurrentEntryTypeChanged: { + if (thumbnailImage.completed) { + thumbnailImage.resetForEntry(); + } + } + + function hasValidEntryId() { + return entry && entry.id !== undefined && entry.id !== null; + } + + function releaseActiveLoad() { + if (!thumbnailImage.activeLoad) { + return; + } + thumbnailImage.activeLoad = false; + if (modal && modal.activeImageLoads > 0) { + modal.activeImageLoads--; + } + } + + function finishLoad(request) { + thumbnailImage.loadQueued = false; + thumbnailImage.activeEntryId = null; + if (!request || thumbnailImage.activeRequest === request) { + thumbnailImage.activeRequest = null; + } + thumbnailImage.releaseActiveLoad(); + } + + function cancelLoad() { + if (thumbnailImage.activeRequest) { + thumbnailImage.activeRequest.cancelled = true; + thumbnailImage.activeRequest = null; + } + retryTimer.stop(); + visibilityTimer.stop(); + thumbnailImage.loadQueued = false; + thumbnailImage.activeEntryId = null; + thumbnailImage.releaseActiveLoad(); + } + + function resetForEntry() { + thumbnailImage.loadGeneration++; + thumbnailImage.cachedImageData = ""; + thumbnailImage.isVisible = false; + thumbnailImage.cancelLoad(); + Qt.callLater(function () { + if (thumbnail.disposed) { + return; + } + thumbnailImage.checkVisibility(); + }); + } + + function startLoad() { + if (!modal) { + thumbnailImage.loadQueued = false; + return; + } + modal.activeImageLoads++; + thumbnailImage.activeLoad = true; + thumbnailImage.loadImage(); + } + function tryLoadImage() { - if (thumbnailImage.loadQueued || entryType !== "image" || thumbnailImage.cachedImageData) { + if (thumbnail.disposed || thumbnailImage.loadQueued || entryType !== "image" || thumbnailImage.cachedImageData || !thumbnailImage.hasValidEntryId()) { return; } thumbnailImage.loadQueued = true; - if (modal.activeImageLoads < modal.maxConcurrentLoads) { - modal.activeImageLoads++; - thumbnailImage.loadImage(); + if (modal && modal.activeImageLoads < modal.maxConcurrentLoads) { + thumbnailImage.startLoad(); } else { retryTimer.restart(); } } function loadImage() { + if (!thumbnailImage.hasValidEntryId()) { + thumbnailImage.finishLoad(); + return; + } + const requestedId = entry.id; + const generation = thumbnailImage.loadGeneration; + const request = { + "cancelled": false + }; + thumbnailImage.activeEntryId = requestedId; + thumbnailImage.activeRequest = request; DMSService.sendRequest("clipboard.getEntry", { - "id": entry.id + "id": requestedId }, function (response) { - thumbnailImage.loadQueued = false; - if (modal.activeImageLoads > 0) { - modal.activeImageLoads--; + if (request.cancelled) { + return; + } + if (thumbnail.disposed || generation !== thumbnailImage.loadGeneration || thumbnailImage.activeRequest !== request || thumbnailImage.activeEntryId !== requestedId) { + return; + } + thumbnailImage.finishLoad(request); + if (!entry || entry.id !== requestedId || entryType !== "image") { + return; } if (response.error) { - log.warn("Failed to load image:", entry.id); + log.warn("Failed to load image:", requestedId); + return; + } + if (!response.result) { + ClipboardService.refresh(); return; } const data = response.result?.data; @@ -70,9 +168,8 @@ Item { if (!thumbnailImage.loadQueued) { return; } - if (modal.activeImageLoads < modal.maxConcurrentLoads) { - modal.activeImageLoads++; - thumbnailImage.loadImage(); + if (modal && modal.activeImageLoads < modal.maxConcurrentLoads) { + thumbnailImage.startLoad(); } else { retryTimer.restart(); } @@ -80,7 +177,8 @@ Item { } Component.onCompleted: { - if (entryType !== "image" || listView.height <= 0) { + thumbnailImage.completed = true; + if (entryType !== "image" || listView.height <= 0 || !thumbnailImage.hasValidEntryId()) { return; } @@ -94,6 +192,11 @@ Item { } } + Component.onDestruction: { + thumbnail.disposed = true; + thumbnailImage.cancelLoad(); + } + Timer { id: visibilityTimer interval: 100 @@ -101,7 +204,7 @@ Item { } function checkVisibility() { - if (entryType !== "image" || listView.height <= 0 || isVisible) { + if (thumbnail.disposed || entryType !== "image" || listView.height <= 0 || isVisible || !thumbnailImage.hasValidEntryId()) { return; } const itemY = itemIndex * (ClipboardConstants.itemHeight + listView.spacing); diff --git a/quickshell/Modals/DankLauncherV2/ClipboardLauncherPreview.qml b/quickshell/Modals/DankLauncherV2/ClipboardLauncherPreview.qml index da2f9e49..56b9fae8 100644 --- a/quickshell/Modals/DankLauncherV2/ClipboardLauncherPreview.qml +++ b/quickshell/Modals/DankLauncherV2/ClipboardLauncherPreview.qml @@ -57,7 +57,11 @@ Rectangle { return; if (response.error) return; - const result = response.result ?? {}; + if (!response.result) { + ClipboardService.refresh(); + return; + } + const result = response.result; const mimeType = (result.mimeType ?? entry?.mimeType ?? "").toString(); const data = (result.data ?? "").toString(); if (data.length === 0 || !resolvedSourceUrl(data, mimeType))