Merge pull request #327 from bnema/refactor/reactive-history-sidebar
refactor(history): refresh sidebar from history changes
26 files · +3353 -691
M .mockery.yaml +2 -0
| @@ -113,6 +113,8 @@ packages: | |||
| 113 | 113 | EntryInputTarget: {} | |
| 114 | 114 | HomepageFavorites: {} | |
| 115 | 115 | HomepageHistory: {} | |
| 116 | + | HistoryChangeSink: {} | |
| 117 | + | HistoryMutationCoordinator: {} | |
| 116 | 118 | HistorySidebarHistory: {} | |
| 117 | 119 | AllKeybindingsResetter: {} | |
| 118 | 120 | KeybindingResetter: {} | |
M cmd/dumber/main.go +47 -30
| @@ -259,6 +259,7 @@ func runGUI(cfg *config.Config) int { | |||
| 259 | 259 | engine.SetHandlerContext(ctx) | |
| 260 | 260 | ||
| 261 | 261 | useCases := createUseCases(repos, cfg) | |
| 262 | + | defer useCases.Close() | |
| 262 | 263 | if needsEagerDB { | |
| 263 | 264 | handleAutoRestore(ctx, cfg, useCases, browserSession.Session.ID) | |
| 264 | 265 | } | |
| @@ -322,6 +323,7 @@ func runStandaloneOmnibox() int { | |||
| 322 | 323 | } | |
| 323 | 324 | ||
| 324 | 325 | useCases := createUseCases(repos, cfg) | |
| 326 | + | defer useCases.Close() | |
| 325 | 327 | uiDeps, err := buildUIDependencies( | |
| 326 | 328 | ctx, | |
| 327 | 329 | cfg, | |
| @@ -728,21 +730,29 @@ func createLazyRepositories(provider port.DatabaseProvider) *repositories { | |||
| 728 | 730 | ||
| 729 | 731 | // useCases groups application layer use case implementations. | |
| 730 | 732 | type useCases struct { | |
| 731 | - | tabs *usecase.ManageTabsUseCase | |
| 732 | - | panes *usecase.ManagePanesUseCase | |
| 733 | - | history *usecase.SearchHistoryUseCase | |
| 734 | - | favorites *usecase.ManageFavoritesUseCase | |
| 735 | - | zoom *usecase.ManageZoomUseCase | |
| 736 | - | permission *usecase.HandlePermissionUseCase | |
| 737 | - | navigate *usecase.NavigateUseCase | |
| 738 | - | copyURL *usecase.CopyURLUseCase | |
| 739 | - | snapshot *usecase.SnapshotSessionUseCase | |
| 740 | - | lastRestorable *usecase.GetLastRestorableSessionUseCase | |
| 741 | - | checkUpdate *usecase.CheckUpdateUseCase | |
| 742 | - | applyUpdate *usecase.ApplyUpdateUseCase | |
| 743 | - | clipboard port.Clipboard | |
| 744 | - | favicon *infrafavicon.Service | |
| 745 | - | faviconUC *usecase.FaviconUseCase | |
| 733 | + | tabs *usecase.ManageTabsUseCase | |
| 734 | + | panes *usecase.ManagePanesUseCase | |
| 735 | + | history *usecase.SearchHistoryUseCase | |
| 736 | + | favorites *usecase.ManageFavoritesUseCase | |
| 737 | + | zoom *usecase.ManageZoomUseCase | |
| 738 | + | permission *usecase.HandlePermissionUseCase | |
| 739 | + | navigate *usecase.NavigateUseCase | |
| 740 | + | historyRecorder *usecase.HistoryRecorderUseCase | |
| 741 | + | copyURL *usecase.CopyURLUseCase | |
| 742 | + | snapshot *usecase.SnapshotSessionUseCase | |
| 743 | + | lastRestorable *usecase.GetLastRestorableSessionUseCase | |
| 744 | + | checkUpdate *usecase.CheckUpdateUseCase | |
| 745 | + | applyUpdate *usecase.ApplyUpdateUseCase | |
| 746 | + | clipboard port.Clipboard | |
| 747 | + | favicon *infrafavicon.Service | |
| 748 | + | faviconUC *usecase.FaviconUseCase | |
| 749 | + | } | |
| 750 | + | ||
| 751 | + | func (uc *useCases) Close() { | |
| 752 | + | if uc == nil || uc.historyRecorder == nil { | |
| 753 | + | return | |
| 754 | + | } | |
| 755 | + | uc.historyRecorder.Close() | |
| 746 | 756 | } | |
| 747 | 757 | ||
| 748 | 758 | func createUseCases(repos *repositories, cfg *config.Config) *useCases { | |
| @@ -783,23 +793,29 @@ func createUseCases(repos *repositories, cfg *config.Config) *useCases { | |||
| 783 | 793 | ||
| 784 | 794 | // Permission use case will be initialized later with dialog presenter | |
| 785 | 795 | permissionUC := usecase.NewHandlePermissionUseCase(repos.permission, nil, logging.FromContext) | |
| 796 | + | historyUC := usecase.NewSearchHistoryUseCase(repos.history) | |
| 797 | + | historyRecorderUC := usecase.NewHistoryRecorderUseCase(repos.history, nil) | |
| 798 | + | // App setup passes HistoryRecorderUC into NavigationCoordinatorWithHistoryRecorder | |
| 799 | + | // so LoadCommitted callbacks record history through NavigationCoordinator.RecordHistory. | |
| 800 | + | historyUC.SetHistoryMutationCoordinator(historyRecorderUC) | |
| 786 | 801 | ||
| 787 | 802 | return &useCases{ | |
| 788 | - | tabs: usecase.NewManageTabsUseCase(idGenerator), | |
| 789 | - | panes: usecase.NewManagePanesUseCase(idGenerator), | |
| 790 | - | history: usecase.NewSearchHistoryUseCase(repos.history), | |
| 791 | - | favorites: usecase.NewManageFavoritesUseCase(repos.favorite, repos.folder, repos.tag), | |
| 792 | - | zoom: usecase.NewManageZoomUseCase(repos.zoom, defaultZoom, zoomCache), | |
| 793 | - | permission: permissionUC, | |
| 794 | - | navigate: usecase.NewNavigateUseCase(repos.history, repos.zoom, defaultZoom), | |
| 795 | - | copyURL: usecase.NewCopyURLUseCase(clipboardAdapter), | |
| 796 | - | snapshot: usecase.NewSnapshotSessionUseCase(repos.sessionState), | |
| 797 | - | lastRestorable: usecase.NewGetLastRestorableSessionUseCase(repos.session, repos.sessionState), | |
| 798 | - | checkUpdate: usecase.NewCheckUpdateUseCase(updateChecker, updateApplier, buildInfo), | |
| 799 | - | applyUpdate: usecase.NewApplyUpdateUseCase(updateDownloader, updateApplier, xdgDirs.CacheHome), | |
| 800 | - | clipboard: clipboardAdapter, | |
| 801 | - | favicon: faviconService, | |
| 802 | - | faviconUC: faviconUC, | |
| 803 | + | tabs: usecase.NewManageTabsUseCase(idGenerator), | |
| 804 | + | panes: usecase.NewManagePanesUseCase(idGenerator), | |
| 805 | + | history: historyUC, | |
| 806 | + | favorites: usecase.NewManageFavoritesUseCase(repos.favorite, repos.folder, repos.tag), | |
| 807 | + | zoom: usecase.NewManageZoomUseCase(repos.zoom, defaultZoom, zoomCache), | |
| 808 | + | permission: permissionUC, | |
| 809 | + | navigate: usecase.NewNavigateUseCase(defaultZoom), | |
| 810 | + | historyRecorder: historyRecorderUC, | |
| 811 | + | copyURL: usecase.NewCopyURLUseCase(clipboardAdapter), | |
| 812 | + | snapshot: usecase.NewSnapshotSessionUseCase(repos.sessionState), | |
| 813 | + | lastRestorable: usecase.NewGetLastRestorableSessionUseCase(repos.session, repos.sessionState), | |
| 814 | + | checkUpdate: usecase.NewCheckUpdateUseCase(updateChecker, updateApplier, buildInfo), | |
| 815 | + | applyUpdate: usecase.NewApplyUpdateUseCase(updateDownloader, updateApplier, xdgDirs.CacheHome), | |
| 816 | + | clipboard: clipboardAdapter, | |
| 817 | + | favicon: faviconService, | |
| 818 | + | faviconUC: faviconUC, | |
| 803 | 819 | } | |
| 804 | 820 | } | |
| 805 | 821 | ||
| @@ -896,6 +912,7 @@ func buildUIDependencies( | |||
| 896 | 912 | ZoomUC: uc.zoom, | |
| 897 | 913 | PermissionUC: uc.permission, | |
| 898 | 914 | NavigateUC: uc.navigate, | |
| 915 | + | HistoryRecorderUC: uc.historyRecorder, | |
| 899 | 916 | CopyURLUC: uc.copyURL, | |
| 900 | 917 | Clipboard: uc.clipboard, | |
| 901 | 918 | FaviconService: legacyFaviconService(uc), | |
A internal/application/dto/history_change.go +28 -0
| @@ -0,0 +1,28 @@ | |||
| 1 | + | package dto | |
| 2 | + | ||
| 3 | + | import "time" | |
| 4 | + | ||
| 5 | + | // HistoryChangeReason describes the category of a persisted history mutation. | |
| 6 | + | type HistoryChangeReason string | |
| 7 | + | ||
| 8 | + | const ( | |
| 9 | + | // HistoryChangeReasonVisit indicates one or more history visits were persisted. | |
| 10 | + | HistoryChangeReasonVisit HistoryChangeReason = "visit" | |
| 11 | + | // HistoryChangeReasonTitle indicates one or more history titles were updated. | |
| 12 | + | HistoryChangeReasonTitle HistoryChangeReason = "title" | |
| 13 | + | // HistoryChangeReasonDelete indicates one or more history entries were deleted. | |
| 14 | + | HistoryChangeReasonDelete HistoryChangeReason = "delete" | |
| 15 | + | // HistoryChangeReasonClear indicates a history clear operation was persisted. | |
| 16 | + | HistoryChangeReasonClear HistoryChangeReason = "clear" | |
| 17 | + | ) | |
| 18 | + | ||
| 19 | + | // HistoryChange reports aggregate facts about persisted history mutations. | |
| 20 | + | // It intentionally omits URLs so UI adapters only know that history changed. | |
| 21 | + | type HistoryChange struct { | |
| 22 | + | Reasons []HistoryChangeReason | |
| 23 | + | VisitCount int | |
| 24 | + | TitleCount int | |
| 25 | + | DeleteCount int | |
| 26 | + | DeleteCountKnown bool | |
| 27 | + | ChangedAt time.Time | |
| 28 | + | } | |
A internal/application/port/history_changes.go +17 -0
| @@ -0,0 +1,17 @@ | |||
| 1 | + | package port | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/application/dto" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | // HistoryChangeSink receives persisted history change notifications. | |
| 10 | + | type HistoryChangeSink interface { | |
| 11 | + | OnHistoryChanged(ctx context.Context, change dto.HistoryChange) | |
| 12 | + | } | |
| 13 | + | ||
| 14 | + | // HistoryMutationCoordinator serializes destructive history mutations with pending history writes. | |
| 15 | + | type HistoryMutationCoordinator interface { | |
| 16 | + | BeginHistoryMutation(ctx context.Context) (release func(), err error) | |
| 17 | + | } | |
A internal/application/port/mocks/history_changes.go +174 -0
| @@ -0,0 +1,174 @@ | |||
| 1 | + | // Code generated by mockery; DO NOT EDIT. | |
| 2 | + | // github.com/vektra/mockery | |
| 3 | + | // template: testify | |
| 4 | + | ||
| 5 | + | package mocks | |
| 6 | + | ||
| 7 | + | import ( | |
| 8 | + | "context" | |
| 9 | + | ||
| 10 | + | "github.com/bnema/dumber/internal/application/dto" | |
| 11 | + | mock "github.com/stretchr/testify/mock" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // NewMockHistoryChangeSink creates a new instance of MockHistoryChangeSink. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |
| 15 | + | // The first argument is typically a *testing.T value. | |
| 16 | + | func NewMockHistoryChangeSink(t interface { | |
| 17 | + | mock.TestingT | |
| 18 | + | Cleanup(func()) | |
| 19 | + | }) *MockHistoryChangeSink { | |
| 20 | + | mock := &MockHistoryChangeSink{} | |
| 21 | + | mock.Mock.Test(t) | |
| 22 | + | ||
| 23 | + | t.Cleanup(func() { mock.AssertExpectations(t) }) | |
| 24 | + | ||
| 25 | + | return mock | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | // MockHistoryChangeSink is an autogenerated mock type for the HistoryChangeSink type | |
| 29 | + | type MockHistoryChangeSink struct { | |
| 30 | + | mock.Mock | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | type MockHistoryChangeSink_Expecter struct { | |
| 34 | + | mock *mock.Mock | |
| 35 | + | } | |
| 36 | + | ||
| 37 | + | func (_m *MockHistoryChangeSink) EXPECT() *MockHistoryChangeSink_Expecter { | |
| 38 | + | return &MockHistoryChangeSink_Expecter{mock: &_m.Mock} | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | // OnHistoryChanged provides a mock function for the type MockHistoryChangeSink | |
| 42 | + | func (_mock *MockHistoryChangeSink) OnHistoryChanged(ctx context.Context, change dto.HistoryChange) { | |
| 43 | + | _mock.Called(ctx, change) | |
| 44 | + | return | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | // MockHistoryChangeSink_OnHistoryChanged_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'OnHistoryChanged' | |
| 48 | + | type MockHistoryChangeSink_OnHistoryChanged_Call struct { | |
| 49 | + | *mock.Call | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | // OnHistoryChanged is a helper method to define mock.On call | |
| 53 | + | // - ctx context.Context | |
| 54 | + | // - change dto.HistoryChange | |
| 55 | + | func (_e *MockHistoryChangeSink_Expecter) OnHistoryChanged(ctx any, change any) *MockHistoryChangeSink_OnHistoryChanged_Call { | |
| 56 | + | return &MockHistoryChangeSink_OnHistoryChanged_Call{Call: _e.mock.On("OnHistoryChanged", ctx, change)} | |
| 57 | + | } | |
| 58 | + | ||
| 59 | + | func (_c *MockHistoryChangeSink_OnHistoryChanged_Call) Run(run func(ctx context.Context, change dto.HistoryChange)) *MockHistoryChangeSink_OnHistoryChanged_Call { | |
| 60 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 61 | + | var arg0 context.Context | |
| 62 | + | if args[0] != nil { | |
| 63 | + | arg0 = args[0].(context.Context) | |
| 64 | + | } | |
| 65 | + | var arg1 dto.HistoryChange | |
| 66 | + | if args[1] != nil { | |
| 67 | + | arg1 = args[1].(dto.HistoryChange) | |
| 68 | + | } | |
| 69 | + | run( | |
| 70 | + | arg0, | |
| 71 | + | arg1, | |
| 72 | + | ) | |
| 73 | + | }) | |
| 74 | + | return _c | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | func (_c *MockHistoryChangeSink_OnHistoryChanged_Call) Return() *MockHistoryChangeSink_OnHistoryChanged_Call { | |
| 78 | + | _c.Call.Return() | |
| 79 | + | return _c | |
| 80 | + | } | |
| 81 | + | ||
| 82 | + | func (_c *MockHistoryChangeSink_OnHistoryChanged_Call) RunAndReturn(run func(ctx context.Context, change dto.HistoryChange)) *MockHistoryChangeSink_OnHistoryChanged_Call { | |
| 83 | + | _c.Run(run) | |
| 84 | + | return _c | |
| 85 | + | } | |
| 86 | + | ||
| 87 | + | // NewMockHistoryMutationCoordinator creates a new instance of MockHistoryMutationCoordinator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |
| 88 | + | // The first argument is typically a *testing.T value. | |
| 89 | + | func NewMockHistoryMutationCoordinator(t interface { | |
| 90 | + | mock.TestingT | |
| 91 | + | Cleanup(func()) | |
| 92 | + | }) *MockHistoryMutationCoordinator { | |
| 93 | + | mock := &MockHistoryMutationCoordinator{} | |
| 94 | + | mock.Mock.Test(t) | |
| 95 | + | ||
| 96 | + | t.Cleanup(func() { mock.AssertExpectations(t) }) | |
| 97 | + | ||
| 98 | + | return mock | |
| 99 | + | } | |
| 100 | + | ||
| 101 | + | // MockHistoryMutationCoordinator is an autogenerated mock type for the HistoryMutationCoordinator type | |
| 102 | + | type MockHistoryMutationCoordinator struct { | |
| 103 | + | mock.Mock | |
| 104 | + | } | |
| 105 | + | ||
| 106 | + | type MockHistoryMutationCoordinator_Expecter struct { | |
| 107 | + | mock *mock.Mock | |
| 108 | + | } | |
| 109 | + | ||
| 110 | + | func (_m *MockHistoryMutationCoordinator) EXPECT() *MockHistoryMutationCoordinator_Expecter { | |
| 111 | + | return &MockHistoryMutationCoordinator_Expecter{mock: &_m.Mock} | |
| 112 | + | } | |
| 113 | + | ||
| 114 | + | // BeginHistoryMutation provides a mock function for the type MockHistoryMutationCoordinator | |
| 115 | + | func (_mock *MockHistoryMutationCoordinator) BeginHistoryMutation(ctx context.Context) (func(), error) { | |
| 116 | + | ret := _mock.Called(ctx) | |
| 117 | + | ||
| 118 | + | if len(ret) == 0 { | |
| 119 | + | panic("no return value specified for BeginHistoryMutation") | |
| 120 | + | } | |
| 121 | + | ||
| 122 | + | var r0 func() | |
| 123 | + | var r1 error | |
| 124 | + | if returnFunc, ok := ret.Get(0).(func(context.Context) (func(), error)); ok { | |
| 125 | + | return returnFunc(ctx) | |
| 126 | + | } | |
| 127 | + | if returnFunc, ok := ret.Get(0).(func(context.Context) func()); ok { | |
| 128 | + | r0 = returnFunc(ctx) | |
| 129 | + | } else { | |
| 130 | + | if ret.Get(0) != nil { | |
| 131 | + | r0 = ret.Get(0).(func()) | |
| 132 | + | } | |
| 133 | + | } | |
| 134 | + | if returnFunc, ok := ret.Get(1).(func(context.Context) error); ok { | |
| 135 | + | r1 = returnFunc(ctx) | |
| 136 | + | } else { | |
| 137 | + | r1 = ret.Error(1) | |
| 138 | + | } | |
| 139 | + | return r0, r1 | |
| 140 | + | } | |
| 141 | + | ||
| 142 | + | // MockHistoryMutationCoordinator_BeginHistoryMutation_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'BeginHistoryMutation' | |
| 143 | + | type MockHistoryMutationCoordinator_BeginHistoryMutation_Call struct { | |
| 144 | + | *mock.Call | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | // BeginHistoryMutation is a helper method to define mock.On call | |
| 148 | + | // - ctx context.Context | |
| 149 | + | func (_e *MockHistoryMutationCoordinator_Expecter) BeginHistoryMutation(ctx any) *MockHistoryMutationCoordinator_BeginHistoryMutation_Call { | |
| 150 | + | return &MockHistoryMutationCoordinator_BeginHistoryMutation_Call{Call: _e.mock.On("BeginHistoryMutation", ctx)} | |
| 151 | + | } | |
| 152 | + | ||
| 153 | + | func (_c *MockHistoryMutationCoordinator_BeginHistoryMutation_Call) Run(run func(ctx context.Context)) *MockHistoryMutationCoordinator_BeginHistoryMutation_Call { | |
| 154 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 155 | + | var arg0 context.Context | |
| 156 | + | if args[0] != nil { | |
| 157 | + | arg0 = args[0].(context.Context) | |
| 158 | + | } | |
| 159 | + | run( | |
| 160 | + | arg0, | |
| 161 | + | ) | |
| 162 | + | }) | |
| 163 | + | return _c | |
| 164 | + | } | |
| 165 | + | ||
| 166 | + | func (_c *MockHistoryMutationCoordinator_BeginHistoryMutation_Call) Return(release func(), err error) *MockHistoryMutationCoordinator_BeginHistoryMutation_Call { | |
| 167 | + | _c.Call.Return(release, err) | |
| 168 | + | return _c | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | func (_c *MockHistoryMutationCoordinator_BeginHistoryMutation_Call) RunAndReturn(run func(ctx context.Context) (func(), error)) *MockHistoryMutationCoordinator_BeginHistoryMutation_Call { | |
| 172 | + | _c.Call.Return(run) | |
| 173 | + | return _c | |
| 174 | + | } | |
A internal/application/usecase/history_change_sink.go +37 -0
| @@ -0,0 +1,37 @@ | |||
| 1 | + | package usecase | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "reflect" | |
| 6 | + | ||
| 7 | + | "github.com/bnema/dumber/internal/application/dto" | |
| 8 | + | "github.com/bnema/dumber/internal/application/port" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | type noopHistoryChangeSink struct{} | |
| 12 | + | ||
| 13 | + | func (noopHistoryChangeSink) OnHistoryChanged(_ context.Context, _ dto.HistoryChange) {} | |
| 14 | + | ||
| 15 | + | func normalizeHistoryChangeSink(sink port.HistoryChangeSink) port.HistoryChangeSink { | |
| 16 | + | if isNilHistoryChangeSink(sink) { | |
| 17 | + | return noopHistoryChangeSink{} | |
| 18 | + | } | |
| 19 | + | return sink | |
| 20 | + | } | |
| 21 | + | ||
| 22 | + | func isNilHistoryChangeSink(sink port.HistoryChangeSink) bool { | |
| 23 | + | return isNilInterface(sink) | |
| 24 | + | } | |
| 25 | + | ||
| 26 | + | func isNilInterface(value any) bool { | |
| 27 | + | if value == nil { | |
| 28 | + | return true | |
| 29 | + | } | |
| 30 | + | reflected := reflect.ValueOf(value) | |
| 31 | + | switch reflected.Kind() { | |
| 32 | + | case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Pointer, reflect.Slice: | |
| 33 | + | return reflected.IsNil() | |
| 34 | + | default: | |
| 35 | + | return false | |
| 36 | + | } | |
| 37 | + | } | |
A internal/application/usecase/history_recorder.go +798 -0
| @@ -0,0 +1,798 @@ | |||
| 1 | + | package usecase | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "errors" | |
| 6 | + | "net" | |
| 7 | + | "net/url" | |
| 8 | + | "strings" | |
| 9 | + | "sync" | |
| 10 | + | "time" | |
| 11 | + | ||
| 12 | + | "github.com/bnema/dumber/internal/application/dto" | |
| 13 | + | "github.com/bnema/dumber/internal/application/port" | |
| 14 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 15 | + | "github.com/bnema/dumber/internal/domain/repository" | |
| 16 | + | "github.com/bnema/dumber/internal/logging" | |
| 17 | + | "github.com/rs/zerolog" | |
| 18 | + | ) | |
| 19 | + | ||
| 20 | + | const ( | |
| 21 | + | // historyQueueSize is the buffer size for the async history queue. | |
| 22 | + | // If the queue is full, new records are dropped with a warning. | |
| 23 | + | historyQueueSize = 100 | |
| 24 | + | ||
| 25 | + | // historyWorkerFlushInterval coalesces bursts into fewer persistence writes. | |
| 26 | + | historyWorkerFlushInterval = 100 * time.Millisecond | |
| 27 | + | ||
| 28 | + | // historyWorkerShutdownFlushTimeout bounds final persistence attempts during shutdown. | |
| 29 | + | historyWorkerShutdownFlushTimeout = 2 * time.Second | |
| 30 | + | ||
| 31 | + | // historyWorkerShutdownWaitTimeout bounds Close even if a repository call ignores context cancellation. | |
| 32 | + | historyWorkerShutdownWaitTimeout = historyWorkerShutdownFlushTimeout + 500*time.Millisecond | |
| 33 | + | ) | |
| 34 | + | ||
| 35 | + | // historyDeduplicationWindow is the time window for deduplicating history visits. | |
| 36 | + | // Visits to the same URL within this window count as a single visit. | |
| 37 | + | // This prevents inflation from redirects and rapid navigation. | |
| 38 | + | const historyDeduplicationWindow = 2 * time.Second | |
| 39 | + | ||
| 40 | + | // stripTrackingParamsForHistoryDedup controls whether known tracking parameters | |
| 41 | + | // are removed while canonicalizing URLs for history deduplication/storage. | |
| 42 | + | const stripTrackingParamsForHistoryDedup = true | |
| 43 | + | ||
| 44 | + | // historyRecord holds data for async history recording. | |
| 45 | + | type historyRecord struct { | |
| 46 | + | url string | |
| 47 | + | title string // non-empty for title-update-only records | |
| 48 | + | visits int | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | type historyVisitDeltaIncrementer interface { | |
| 52 | + | IncrementVisitCountBy(ctx context.Context, url string, delta int) error | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | type historyMetadataUpdater interface { | |
| 56 | + | UpdateMetadata(ctx context.Context, entry *entity.HistoryEntry) error | |
| 57 | + | } | |
| 58 | + | ||
| 59 | + | type paneHistoryState struct { | |
| 60 | + | lastRawURL string | |
| 61 | + | lastCanonicalURL string | |
| 62 | + | lastRecordedAt time.Time | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | type historyCanonicalizationOptions struct { | |
| 66 | + | StripTrackingParams bool | |
| 67 | + | } | |
| 68 | + | ||
| 69 | + | type historyControlRequest struct { | |
| 70 | + | kind historyControlKind | |
| 71 | + | ctx context.Context | |
| 72 | + | done chan error | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | type historyControlKind int | |
| 76 | + | ||
| 77 | + | type titlePersistenceResult int | |
| 78 | + | ||
| 79 | + | const historyControlFlushAndReset historyControlKind = iota | |
| 80 | + | ||
| 81 | + | const ( | |
| 82 | + | titlePersistenceRetry titlePersistenceResult = iota | |
| 83 | + | titlePersistenceSaved | |
| 84 | + | titlePersistenceDiscard | |
| 85 | + | ) | |
| 86 | + | ||
| 87 | + | var ( | |
| 88 | + | errHistoryRecorderClosing = errors.New("history recorder closing") | |
| 89 | + | errHistoryPersistenceIncomplete = errors.New("history persistence incomplete") | |
| 90 | + | ) | |
| 91 | + | ||
| 92 | + | type pendingHistoryRecords struct { | |
| 93 | + | visits map[string]int | |
| 94 | + | titles map[string]string | |
| 95 | + | } | |
| 96 | + | ||
| 97 | + | func newPendingHistoryRecords() *pendingHistoryRecords { | |
| 98 | + | return &pendingHistoryRecords{ | |
| 99 | + | visits: make(map[string]int), | |
| 100 | + | titles: make(map[string]string), | |
| 101 | + | } | |
| 102 | + | } | |
| 103 | + | ||
| 104 | + | func (p *pendingHistoryRecords) add(record historyRecord) { | |
| 105 | + | if record.title != "" { | |
| 106 | + | p.titles[record.url] = record.title | |
| 107 | + | } | |
| 108 | + | if record.visits > 0 { | |
| 109 | + | p.visits[record.url] += record.visits | |
| 110 | + | } | |
| 111 | + | } | |
| 112 | + | ||
| 113 | + | func (p *pendingHistoryRecords) empty() bool { | |
| 114 | + | return len(p.visits) == 0 && len(p.titles) == 0 | |
| 115 | + | } | |
| 116 | + | ||
| 117 | + | func (p *pendingHistoryRecords) reset() { | |
| 118 | + | p.visits = make(map[string]int) | |
| 119 | + | p.titles = make(map[string]string) | |
| 120 | + | } | |
| 121 | + | ||
| 122 | + | // HistoryRecorderUseCase records persisted history visits and title updates asynchronously. | |
| 123 | + | type HistoryRecorderUseCase struct { | |
| 124 | + | historyRepo repository.HistoryRepository | |
| 125 | + | changeSinkMu sync.RWMutex | |
| 126 | + | changeSink port.HistoryChangeSink | |
| 127 | + | recentMu sync.Mutex | |
| 128 | + | recentVisits map[string]paneHistoryState // key: pane ID, value: per-pane history state | |
| 129 | + | mutationMu sync.Mutex | |
| 130 | + | closeOnce sync.Once | |
| 131 | + | enqueueMu sync.Mutex | |
| 132 | + | closing bool | |
| 133 | + | ||
| 134 | + | historyQueue chan historyRecord | |
| 135 | + | controlQueue chan historyControlRequest | |
| 136 | + | done chan struct{} | |
| 137 | + | wg sync.WaitGroup | |
| 138 | + | ctx context.Context // Base context for background worker | |
| 139 | + | cancel context.CancelFunc | |
| 140 | + | } | |
| 141 | + | ||
| 142 | + | // NewHistoryRecorderUseCase creates a new history recorder use case. | |
| 143 | + | func NewHistoryRecorderUseCase(historyRepo repository.HistoryRepository, changeSink port.HistoryChangeSink) *HistoryRecorderUseCase { | |
| 144 | + | changeSink = normalizeHistoryChangeSink(changeSink) | |
| 145 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 146 | + | ||
| 147 | + | uc := &HistoryRecorderUseCase{ | |
| 148 | + | historyRepo: historyRepo, | |
| 149 | + | changeSink: changeSink, | |
| 150 | + | recentVisits: make(map[string]paneHistoryState), | |
| 151 | + | historyQueue: make(chan historyRecord, historyQueueSize), | |
| 152 | + | controlQueue: make(chan historyControlRequest), | |
| 153 | + | done: make(chan struct{}), | |
| 154 | + | ctx: ctx, | |
| 155 | + | cancel: cancel, | |
| 156 | + | } | |
| 157 | + | ||
| 158 | + | uc.wg.Add(1) | |
| 159 | + | go uc.historyWorker() | |
| 160 | + | ||
| 161 | + | return uc | |
| 162 | + | } | |
| 163 | + | ||
| 164 | + | // SetHistoryChangeSink sets the sink for persisted history change notifications. | |
| 165 | + | func (uc *HistoryRecorderUseCase) SetHistoryChangeSink(changeSink port.HistoryChangeSink) { | |
| 166 | + | if uc == nil { | |
| 167 | + | return | |
| 168 | + | } | |
| 169 | + | changeSink = normalizeHistoryChangeSink(changeSink) | |
| 170 | + | uc.changeSinkMu.Lock() | |
| 171 | + | uc.changeSink = changeSink | |
| 172 | + | uc.changeSinkMu.Unlock() | |
| 173 | + | } | |
| 174 | + | ||
| 175 | + | // Close shuts down the background history worker and drains any pending records. | |
| 176 | + | func (uc *HistoryRecorderUseCase) Close() { | |
| 177 | + | if uc == nil { | |
| 178 | + | return | |
| 179 | + | } | |
| 180 | + | uc.closeOnce.Do(func() { | |
| 181 | + | uc.enqueueMu.Lock() | |
| 182 | + | uc.closing = true | |
| 183 | + | uc.enqueueMu.Unlock() | |
| 184 | + | uc.cancelWorkerContext() | |
| 185 | + | if uc.done != nil { | |
| 186 | + | close(uc.done) | |
| 187 | + | } | |
| 188 | + | }) | |
| 189 | + | if uc.waitForHistoryWorker(historyWorkerShutdownWaitTimeout) { | |
| 190 | + | return | |
| 191 | + | } | |
| 192 | + | logging.FromContext(uc.workerContext()).Warn(). | |
| 193 | + | Dur("timeout", historyWorkerShutdownWaitTimeout). | |
| 194 | + | Msg("history worker shutdown timed out") | |
| 195 | + | } | |
| 196 | + | ||
| 197 | + | func (uc *HistoryRecorderUseCase) workerContext() context.Context { | |
| 198 | + | if uc == nil || uc.ctx == nil { | |
| 199 | + | return context.Background() | |
| 200 | + | } | |
| 201 | + | return uc.ctx | |
| 202 | + | } | |
| 203 | + | ||
| 204 | + | func (uc *HistoryRecorderUseCase) cancelWorkerContext() { | |
| 205 | + | if uc != nil && uc.cancel != nil { | |
| 206 | + | uc.cancel() | |
| 207 | + | } | |
| 208 | + | } | |
| 209 | + | ||
| 210 | + | func (uc *HistoryRecorderUseCase) waitForHistoryWorker(timeout time.Duration) bool { | |
| 211 | + | done := make(chan struct{}) | |
| 212 | + | go func() { | |
| 213 | + | uc.wg.Wait() | |
| 214 | + | close(done) | |
| 215 | + | }() | |
| 216 | + | if timeout <= 0 { | |
| 217 | + | <-done | |
| 218 | + | return true | |
| 219 | + | } | |
| 220 | + | timer := time.NewTimer(timeout) | |
| 221 | + | defer timer.Stop() | |
| 222 | + | select { | |
| 223 | + | case <-done: | |
| 224 | + | return true | |
| 225 | + | case <-timer.C: | |
| 226 | + | return false | |
| 227 | + | } | |
| 228 | + | } | |
| 229 | + | ||
| 230 | + | // RecordHistory queues a history entry for async recording. | |
| 231 | + | // This is non-blocking to avoid SQLite I/O on the GTK main thread. | |
| 232 | + | // Should be called on LoadCommitted when URI is guaranteed correct. | |
| 233 | + | func (uc *HistoryRecorderUseCase) RecordHistory(ctx context.Context, paneID, rawURL string) { | |
| 234 | + | if uc == nil { | |
| 235 | + | return | |
| 236 | + | } | |
| 237 | + | log := logging.FromContext(ctx) | |
| 238 | + | rawURL = strings.TrimSpace(rawURL) | |
| 239 | + | if rawURL == "" { | |
| 240 | + | return | |
| 241 | + | } | |
| 242 | + | ||
| 243 | + | if !isRecordableHistoryURL(rawURL) { | |
| 244 | + | return | |
| 245 | + | } | |
| 246 | + | ||
| 247 | + | canonicalURL := canonicalizeURLForHistory(rawURL, historyCanonicalizationOptions{ | |
| 248 | + | StripTrackingParams: stripTrackingParamsForHistoryDedup, | |
| 249 | + | }) | |
| 250 | + | if canonicalURL == "" { | |
| 251 | + | return | |
| 252 | + | } | |
| 253 | + | ||
| 254 | + | paneID = normalizePaneID(paneID) | |
| 255 | + | ||
| 256 | + | now := time.Now() | |
| 257 | + | ||
| 258 | + | uc.mutationMu.Lock() | |
| 259 | + | defer uc.mutationMu.Unlock() | |
| 260 | + | ||
| 261 | + | uc.recentMu.Lock() | |
| 262 | + | state := uc.recentVisits[paneID] | |
| 263 | + | ||
| 264 | + | // Ignore in-page hash transitions for history persistence. | |
| 265 | + | if isHashOnlyTransition(state.lastRawURL, rawURL) { | |
| 266 | + | state.lastRawURL = rawURL | |
| 267 | + | uc.recentVisits[paneID] = state | |
| 268 | + | uc.recentMu.Unlock() | |
| 269 | + | return | |
| 270 | + | } | |
| 271 | + | ||
| 272 | + | // Per-pane short-window deduplication by canonical URL. | |
| 273 | + | if state.lastCanonicalURL == canonicalURL && now.Sub(state.lastRecordedAt) < historyDeduplicationWindow { | |
| 274 | + | state.lastRawURL = rawURL | |
| 275 | + | uc.recentVisits[paneID] = state | |
| 276 | + | uc.recentMu.Unlock() | |
| 277 | + | return | |
| 278 | + | } | |
| 279 | + | ||
| 280 | + | state.lastRawURL = rawURL | |
| 281 | + | state.lastCanonicalURL = canonicalURL | |
| 282 | + | state.lastRecordedAt = now | |
| 283 | + | enqueued := uc.enqueueHistoryRecord(historyRecord{url: canonicalURL, visits: 1}) | |
| 284 | + | if enqueued { | |
| 285 | + | uc.recentVisits[paneID] = state | |
| 286 | + | } | |
| 287 | + | uc.recentMu.Unlock() | |
| 288 | + | ||
| 289 | + | if !enqueued { | |
| 290 | + | log.Warn().Str("url", logging.RedactURL(canonicalURL)).Msg("history queue full or recorder closed, dropping record") | |
| 291 | + | } | |
| 292 | + | } | |
| 293 | + | ||
| 294 | + | // ClearPaneHistory removes per-pane deduplication state to prevent unbounded growth. | |
| 295 | + | // Callers should invoke this when a pane is closed. | |
| 296 | + | func (uc *HistoryRecorderUseCase) ClearPaneHistory(paneID string) { | |
| 297 | + | if uc == nil { | |
| 298 | + | return | |
| 299 | + | } | |
| 300 | + | paneID = normalizePaneID(paneID) | |
| 301 | + | uc.mutationMu.Lock() | |
| 302 | + | defer uc.mutationMu.Unlock() | |
| 303 | + | uc.recentMu.Lock() | |
| 304 | + | delete(uc.recentVisits, paneID) | |
| 305 | + | uc.recentMu.Unlock() | |
| 306 | + | } | |
| 307 | + | ||
| 308 | + | // UpdateHistoryTitle queues a title update for a history entry. The actual | |
| 309 | + | // DB write happens in the background historyWorker to avoid concurrent | |
| 310 | + | // repo access from the GTK main thread and the worker goroutine. | |
| 311 | + | func (uc *HistoryRecorderUseCase) UpdateHistoryTitle(_ context.Context, historyURL, title string) { | |
| 312 | + | if uc == nil { | |
| 313 | + | return | |
| 314 | + | } | |
| 315 | + | historyURL = strings.TrimSpace(historyURL) | |
| 316 | + | if historyURL == "" || !isRecordableHistoryURL(historyURL) { | |
| 317 | + | return | |
| 318 | + | } | |
| 319 | + | historyURL = canonicalizeURLForHistory(historyURL, historyCanonicalizationOptions{ | |
| 320 | + | StripTrackingParams: stripTrackingParamsForHistoryDedup, | |
| 321 | + | }) | |
| 322 | + | if historyURL == "" { | |
| 323 | + | return | |
| 324 | + | } | |
| 325 | + | ||
| 326 | + | uc.mutationMu.Lock() | |
| 327 | + | defer uc.mutationMu.Unlock() | |
| 328 | + | ||
| 329 | + | if !uc.enqueueHistoryRecord(historyRecord{url: historyURL, title: title}) { | |
| 330 | + | logging.FromContext(uc.workerContext()).Warn(). | |
| 331 | + | Str("url", logging.RedactURL(historyURL)). | |
| 332 | + | Msg("history queue full or recorder closed, title update dropped") | |
| 333 | + | } | |
| 334 | + | } | |
| 335 | + | ||
| 336 | + | func (uc *HistoryRecorderUseCase) enqueueHistoryRecord(record historyRecord) bool { | |
| 337 | + | uc.enqueueMu.Lock() | |
| 338 | + | defer uc.enqueueMu.Unlock() | |
| 339 | + | if uc.closing { | |
| 340 | + | return false | |
| 341 | + | } | |
| 342 | + | select { | |
| 343 | + | case uc.historyQueue <- record: | |
| 344 | + | return true | |
| 345 | + | default: | |
| 346 | + | return false | |
| 347 | + | } | |
| 348 | + | } | |
| 349 | + | ||
| 350 | + | // historyWorker is a background goroutine that processes history records. | |
| 351 | + | // It drains the queue and persists records to the database without blocking the UI. | |
| 352 | + | func (uc *HistoryRecorderUseCase) historyWorker() { | |
| 353 | + | defer uc.wg.Done() | |
| 354 | + | ||
| 355 | + | ticker := time.NewTicker(historyWorkerFlushInterval) | |
| 356 | + | defer ticker.Stop() | |
| 357 | + | workerCtx := uc.workerContext() | |
| 358 | + | ||
| 359 | + | pending := newPendingHistoryRecords() | |
| 360 | + | for { | |
| 361 | + | select { | |
| 362 | + | case record := <-uc.historyQueue: | |
| 363 | + | pending.add(record) | |
| 364 | + | case <-ticker.C: | |
| 365 | + | _ = uc.flushPendingHistory(workerCtx, pending) | |
| 366 | + | case req := <-uc.controlQueue: | |
| 367 | + | uc.handleHistoryControl(pending, req) | |
| 368 | + | case <-uc.done: | |
| 369 | + | uc.shutdownHistoryWorker(pending) | |
| 370 | + | return | |
| 371 | + | case <-workerCtx.Done(): | |
| 372 | + | uc.shutdownHistoryWorker(pending) | |
| 373 | + | return | |
| 374 | + | } | |
| 375 | + | } | |
| 376 | + | } | |
| 377 | + | ||
| 378 | + | func (uc *HistoryRecorderUseCase) flushPendingHistory(ctx context.Context, pending *pendingHistoryRecords) error { | |
| 379 | + | if ctx == nil { | |
| 380 | + | ctx = uc.workerContext() | |
| 381 | + | } | |
| 382 | + | if pending.empty() { | |
| 383 | + | return nil | |
| 384 | + | } | |
| 385 | + | ||
| 386 | + | visitCount := len(pending.visits) | |
| 387 | + | titleCount := len(pending.titles) | |
| 388 | + | logging.FromContext(ctx).Debug(). | |
| 389 | + | Int("visits", visitCount). | |
| 390 | + | Int("titles", titleCount). | |
| 391 | + | Msg("flushing pending history") | |
| 392 | + | ||
| 393 | + | var flushErr error | |
| 394 | + | successfulVisits := 0 | |
| 395 | + | for historyURL, visits := range pending.visits { | |
| 396 | + | if err := ctx.Err(); err != nil { | |
| 397 | + | uc.publishHistoryChange(successfulVisits, 0) | |
| 398 | + | return err | |
| 399 | + | } | |
| 400 | + | record := historyRecord{url: historyURL, visits: visits} | |
| 401 | + | writtenVisits := uc.persistHistory(ctx, record) | |
| 402 | + | if writtenVisits > 0 { | |
| 403 | + | successfulVisits += writtenVisits | |
| 404 | + | if writtenVisits >= visits { | |
| 405 | + | delete(pending.visits, historyURL) | |
| 406 | + | } else { | |
| 407 | + | pending.visits[historyURL] = visits - writtenVisits | |
| 408 | + | } | |
| 409 | + | } | |
| 410 | + | if writtenVisits < visits { | |
| 411 | + | flushErr = errHistoryPersistenceIncomplete | |
| 412 | + | } | |
| 413 | + | } | |
| 414 | + | ||
| 415 | + | successfulTitles := 0 | |
| 416 | + | for historyURL, title := range pending.titles { | |
| 417 | + | if err := ctx.Err(); err != nil { | |
| 418 | + | uc.publishHistoryChange(successfulVisits, successfulTitles) | |
| 419 | + | return err | |
| 420 | + | } | |
| 421 | + | if _, visitStillPending := pending.visits[historyURL]; visitStillPending { | |
| 422 | + | flushErr = errHistoryPersistenceIncomplete | |
| 423 | + | continue | |
| 424 | + | } | |
| 425 | + | switch uc.persistTitleUpdate(ctx, historyURL, title) { | |
| 426 | + | case titlePersistenceSaved: | |
| 427 | + | successfulTitles++ | |
| 428 | + | delete(pending.titles, historyURL) | |
| 429 | + | case titlePersistenceDiscard: | |
| 430 | + | delete(pending.titles, historyURL) | |
| 431 | + | case titlePersistenceRetry: | |
| 432 | + | flushErr = errHistoryPersistenceIncomplete | |
| 433 | + | } | |
| 434 | + | } | |
| 435 | + | uc.publishHistoryChange(successfulVisits, successfulTitles) | |
| 436 | + | if flushErr != nil { | |
| 437 | + | return flushErr | |
| 438 | + | } | |
| 439 | + | return ctx.Err() | |
| 440 | + | } | |
| 441 | + | ||
| 442 | + | func (uc *HistoryRecorderUseCase) handleHistoryControl(pending *pendingHistoryRecords, req historyControlRequest) { | |
| 443 | + | var err error | |
| 444 | + | switch req.kind { | |
| 445 | + | case historyControlFlushAndReset: | |
| 446 | + | uc.drainHistoryQueue(pending) | |
| 447 | + | err = uc.flushPendingHistory(req.ctx, pending) | |
| 448 | + | if err == nil { | |
| 449 | + | uc.resetRecentVisits() | |
| 450 | + | } | |
| 451 | + | } | |
| 452 | + | req.done <- err | |
| 453 | + | close(req.done) | |
| 454 | + | } | |
| 455 | + | ||
| 456 | + | func (uc *HistoryRecorderUseCase) drainHistoryQueue(pending *pendingHistoryRecords) { | |
| 457 | + | for { | |
| 458 | + | select { | |
| 459 | + | case record := <-uc.historyQueue: | |
| 460 | + | pending.add(record) | |
| 461 | + | default: | |
| 462 | + | return | |
| 463 | + | } | |
| 464 | + | } | |
| 465 | + | } | |
| 466 | + | ||
| 467 | + | func (uc *HistoryRecorderUseCase) shutdownHistoryWorker(pending *pendingHistoryRecords) { | |
| 468 | + | log := logging.FromContext(uc.workerContext()).With(). | |
| 469 | + | Str("component", "history-worker"). | |
| 470 | + | Logger() | |
| 471 | + | shutdownCtx, cancel := context.WithTimeout(context.Background(), historyWorkerShutdownFlushTimeout) | |
| 472 | + | defer cancel() | |
| 473 | + | ||
| 474 | + | for { | |
| 475 | + | if err := shutdownCtx.Err(); err != nil { | |
| 476 | + | if !pending.empty() { | |
| 477 | + | log.Warn().Err(err).Msg("dropping unflushed history during shutdown after timeout") | |
| 478 | + | pending.reset() | |
| 479 | + | } | |
| 480 | + | return | |
| 481 | + | } | |
| 482 | + | log.Debug().Int("remaining", len(uc.historyQueue)).Msg("draining history queue") | |
| 483 | + | uc.drainHistoryQueue(pending) | |
| 484 | + | uc.flushPendingHistoryForShutdown(shutdownCtx, log, pending) | |
| 485 | + | ||
| 486 | + | uc.enqueueMu.Lock() | |
| 487 | + | uc.drainHistoryQueue(pending) | |
| 488 | + | if pending.empty() && len(uc.historyQueue) == 0 { | |
| 489 | + | uc.closing = true | |
| 490 | + | uc.enqueueMu.Unlock() | |
| 491 | + | log.Debug().Msg("history worker shutdown complete") | |
| 492 | + | return | |
| 493 | + | } | |
| 494 | + | uc.enqueueMu.Unlock() | |
| 495 | + | } | |
| 496 | + | } | |
| 497 | + | ||
| 498 | + | func (uc *HistoryRecorderUseCase) flushPendingHistoryForShutdown(ctx context.Context, log zerolog.Logger, pending *pendingHistoryRecords) { | |
| 499 | + | const shutdownFlushAttempts = 3 | |
| 500 | + | for attempt := 1; attempt <= shutdownFlushAttempts; attempt++ { | |
| 501 | + | err := uc.flushPendingHistory(ctx, pending) | |
| 502 | + | if err == nil || pending.empty() { | |
| 503 | + | return | |
| 504 | + | } | |
| 505 | + | if attempt == shutdownFlushAttempts { | |
| 506 | + | log.Warn().Err(err).Msg("dropping unflushed history during shutdown after retries") | |
| 507 | + | pending.reset() | |
| 508 | + | return | |
| 509 | + | } | |
| 510 | + | log.Warn().Err(err).Int("attempt", attempt).Msg("retrying unflushed history during shutdown") | |
| 511 | + | select { | |
| 512 | + | case <-ctx.Done(): | |
| 513 | + | log.Warn().Err(ctx.Err()).Msg("dropping unflushed history during shutdown after timeout") | |
| 514 | + | pending.reset() | |
| 515 | + | return | |
| 516 | + | case <-time.After(historyWorkerFlushInterval): | |
| 517 | + | } | |
| 518 | + | } | |
| 519 | + | } | |
| 520 | + | ||
| 521 | + | // BeginHistoryMutation flushes pending recorder writes and blocks new recorder enqueues until release is called. | |
| 522 | + | func (uc *HistoryRecorderUseCase) BeginHistoryMutation(ctx context.Context) (func(), error) { | |
| 523 | + | if uc == nil { | |
| 524 | + | return func() {}, nil | |
| 525 | + | } | |
| 526 | + | if ctx == nil { | |
| 527 | + | ctx = context.Background() | |
| 528 | + | } | |
| 529 | + | uc.mutationMu.Lock() | |
| 530 | + | if err := uc.flushAndResetHistory(ctx); err != nil { | |
| 531 | + | uc.mutationMu.Unlock() | |
| 532 | + | if errors.Is(err, errHistoryRecorderClosing) { | |
| 533 | + | uc.wg.Wait() | |
| 534 | + | uc.resetRecentVisits() | |
| 535 | + | return func() {}, nil | |
| 536 | + | } | |
| 537 | + | return nil, err | |
| 538 | + | } | |
| 539 | + | return func() { uc.mutationMu.Unlock() }, nil | |
| 540 | + | } | |
| 541 | + | ||
| 542 | + | func (uc *HistoryRecorderUseCase) flushAndResetHistory(ctx context.Context) error { | |
| 543 | + | req := historyControlRequest{ | |
| 544 | + | kind: historyControlFlushAndReset, | |
| 545 | + | ctx: ctx, | |
| 546 | + | done: make(chan error, 1), | |
| 547 | + | } | |
| 548 | + | select { | |
| 549 | + | case uc.controlQueue <- req: | |
| 550 | + | select { | |
| 551 | + | case err := <-req.done: | |
| 552 | + | return err | |
| 553 | + | case <-uc.done: | |
| 554 | + | return uc.waitForAcceptedFlushOrShutdown(ctx, req.done) | |
| 555 | + | case <-ctx.Done(): | |
| 556 | + | return ctx.Err() | |
| 557 | + | } | |
| 558 | + | case <-uc.done: | |
| 559 | + | return errHistoryRecorderClosing | |
| 560 | + | case <-ctx.Done(): | |
| 561 | + | return ctx.Err() | |
| 562 | + | } | |
| 563 | + | } | |
| 564 | + | ||
| 565 | + | func (uc *HistoryRecorderUseCase) waitForAcceptedFlushOrShutdown(ctx context.Context, reqDone <-chan error) error { | |
| 566 | + | wgDone := make(chan struct{}) | |
| 567 | + | go func() { | |
| 568 | + | uc.wg.Wait() | |
| 569 | + | close(wgDone) | |
| 570 | + | }() | |
| 571 | + | ||
| 572 | + | select { | |
| 573 | + | case err := <-reqDone: | |
| 574 | + | return err | |
| 575 | + | case <-wgDone: | |
| 576 | + | uc.resetRecentVisits() | |
| 577 | + | return errHistoryRecorderClosing | |
| 578 | + | case <-ctx.Done(): | |
| 579 | + | return ctx.Err() | |
| 580 | + | } | |
| 581 | + | } | |
| 582 | + | ||
| 583 | + | func (uc *HistoryRecorderUseCase) resetRecentVisits() { | |
| 584 | + | uc.recentMu.Lock() | |
| 585 | + | uc.recentVisits = make(map[string]paneHistoryState) | |
| 586 | + | uc.recentMu.Unlock() | |
| 587 | + | } | |
| 588 | + | ||
| 589 | + | func (uc *HistoryRecorderUseCase) publishHistoryChange(successfulVisits, successfulTitles int) { | |
| 590 | + | if successfulVisits == 0 && successfulTitles == 0 { | |
| 591 | + | return | |
| 592 | + | } | |
| 593 | + | reasons := make([]dto.HistoryChangeReason, 0, 2) | |
| 594 | + | if successfulVisits > 0 { | |
| 595 | + | reasons = append(reasons, dto.HistoryChangeReasonVisit) | |
| 596 | + | } | |
| 597 | + | if successfulTitles > 0 { | |
| 598 | + | reasons = append(reasons, dto.HistoryChangeReasonTitle) | |
| 599 | + | } | |
| 600 | + | sink := uc.historyChangeSink() | |
| 601 | + | sink.OnHistoryChanged(uc.workerContext(), dto.HistoryChange{ | |
| 602 | + | Reasons: reasons, | |
| 603 | + | VisitCount: successfulVisits, | |
| 604 | + | TitleCount: successfulTitles, | |
| 605 | + | ChangedAt: time.Now(), | |
| 606 | + | }) | |
| 607 | + | } | |
| 608 | + | ||
| 609 | + | func (uc *HistoryRecorderUseCase) historyChangeSink() port.HistoryChangeSink { | |
| 610 | + | uc.changeSinkMu.RLock() | |
| 611 | + | sink := uc.changeSink | |
| 612 | + | uc.changeSinkMu.RUnlock() | |
| 613 | + | return normalizeHistoryChangeSink(sink) | |
| 614 | + | } | |
| 615 | + | ||
| 616 | + | // persistHistory writes a history record to the database. | |
| 617 | + | // Called from the background worker goroutine. | |
| 618 | + | func (uc *HistoryRecorderUseCase) persistHistory(ctx context.Context, record historyRecord) int { | |
| 619 | + | log := logging.FromContext(ctx) | |
| 620 | + | ||
| 621 | + | existing, err := uc.historyRepo.FindByURL(ctx, record.url) | |
| 622 | + | if err != nil { | |
| 623 | + | log.Warn().Err(err).Str("url", logging.RedactURL(record.url)).Msg("failed to check history") | |
| 624 | + | return 0 | |
| 625 | + | } | |
| 626 | + | ||
| 627 | + | delta := max(1, record.visits) | |
| 628 | + | if existing != nil { | |
| 629 | + | if deltaWriter, ok := uc.historyRepo.(historyVisitDeltaIncrementer); ok { | |
| 630 | + | if err := deltaWriter.IncrementVisitCountBy(ctx, record.url, delta); err != nil { | |
| 631 | + | log.Warn().Err(err).Str("url", logging.RedactURL(record.url)).Int("delta", delta).Msg("failed to increment visit count by delta") | |
| 632 | + | return 0 | |
| 633 | + | } | |
| 634 | + | return delta | |
| 635 | + | } | |
| 636 | + | written := 0 | |
| 637 | + | for i := 0; i < delta; i++ { | |
| 638 | + | if err := uc.historyRepo.IncrementVisitCount(ctx, record.url); err != nil { | |
| 639 | + | log.Warn().Err(err).Str("url", logging.RedactURL(record.url)).Msg("failed to increment visit count") | |
| 640 | + | return written | |
| 641 | + | } | |
| 642 | + | written++ | |
| 643 | + | } | |
| 644 | + | return written | |
| 645 | + | } | |
| 646 | + | ||
| 647 | + | entry := entity.NewHistoryEntry(record.url, "") | |
| 648 | + | entry.VisitCount = int64(delta) | |
| 649 | + | if err := uc.historyRepo.Save(ctx, entry); err != nil { | |
| 650 | + | log.Warn().Err(err).Str("url", logging.RedactURL(record.url)).Msg("failed to save history") | |
| 651 | + | return 0 | |
| 652 | + | } | |
| 653 | + | return delta | |
| 654 | + | } | |
| 655 | + | ||
| 656 | + | // persistTitleUpdate writes a title update to the database. | |
| 657 | + | // Called from the background worker goroutine. | |
| 658 | + | func (uc *HistoryRecorderUseCase) persistTitleUpdate(ctx context.Context, historyURL, title string) titlePersistenceResult { | |
| 659 | + | log := logging.FromContext(ctx) | |
| 660 | + | ||
| 661 | + | if title == "" { | |
| 662 | + | return titlePersistenceDiscard | |
| 663 | + | } | |
| 664 | + | ||
| 665 | + | entry, err := uc.historyRepo.FindByURL(ctx, historyURL) | |
| 666 | + | if err != nil { | |
| 667 | + | log.Warn().Err(err).Str("url", logging.RedactURL(historyURL)).Msg("failed to find history entry for title update") | |
| 668 | + | return titlePersistenceRetry | |
| 669 | + | } | |
| 670 | + | if entry == nil { | |
| 671 | + | return titlePersistenceDiscard | |
| 672 | + | } | |
| 673 | + | ||
| 674 | + | entry.Title = title | |
| 675 | + | if updater, ok := uc.historyRepo.(historyMetadataUpdater); ok { | |
| 676 | + | if err := updater.UpdateMetadata(ctx, entry); err != nil { | |
| 677 | + | log.Warn().Err(err).Str("url", logging.RedactURL(historyURL)).Msg("failed to update history metadata") | |
| 678 | + | return titlePersistenceRetry | |
| 679 | + | } | |
| 680 | + | return titlePersistenceSaved | |
| 681 | + | } | |
| 682 | + | if err := uc.historyRepo.Save(ctx, entry); err != nil { | |
| 683 | + | log.Warn().Err(err).Str("url", logging.RedactURL(historyURL)).Msg("failed to update history title") | |
| 684 | + | return titlePersistenceRetry | |
| 685 | + | } | |
| 686 | + | return titlePersistenceSaved | |
| 687 | + | } | |
| 688 | + | ||
| 689 | + | func isRecordableHistoryURL(raw string) bool { | |
| 690 | + | parsed, err := url.Parse(strings.TrimSpace(raw)) | |
| 691 | + | if err != nil { | |
| 692 | + | return false | |
| 693 | + | } | |
| 694 | + | if strings.TrimSpace(parsed.Host) == "" { | |
| 695 | + | return false | |
| 696 | + | } | |
| 697 | + | switch strings.ToLower(parsed.Scheme) { | |
| 698 | + | case "http", "https": | |
| 699 | + | return true | |
| 700 | + | default: | |
| 701 | + | return false | |
| 702 | + | } | |
| 703 | + | } | |
| 704 | + | ||
| 705 | + | func canonicalizeURLForHistory(raw string, opts historyCanonicalizationOptions) string { | |
| 706 | + | raw = strings.TrimSpace(raw) | |
| 707 | + | if raw == "" { | |
| 708 | + | return "" | |
| 709 | + | } | |
| 710 | + | ||
| 711 | + | parsed, err := url.Parse(raw) | |
| 712 | + | if err != nil { | |
| 713 | + | return strings.TrimSuffix(raw, "/") | |
| 714 | + | } | |
| 715 | + | ||
| 716 | + | parsed.Scheme = strings.ToLower(parsed.Scheme) | |
| 717 | + | parsed.Host = canonicalHistoryHost(parsed) | |
| 718 | + | parsed.User = nil | |
| 719 | + | parsed.Fragment = "" | |
| 720 | + | parsed.Path = normalizePathForHistory(parsed.Path) | |
| 721 | + | ||
| 722 | + | query := parsed.Query() | |
| 723 | + | if opts.StripTrackingParams { | |
| 724 | + | for key := range query { | |
| 725 | + | if isTrackingQueryParam(key) { | |
| 726 | + | query.Del(key) | |
| 727 | + | } | |
| 728 | + | } | |
| 729 | + | } | |
| 730 | + | parsed.RawQuery = query.Encode() | |
| 731 | + | ||
| 732 | + | return parsed.String() | |
| 733 | + | } | |
| 734 | + | ||
| 735 | + | func canonicalHistoryHost(parsed *url.URL) string { | |
| 736 | + | host := strings.ToLower(parsed.Hostname()) | |
| 737 | + | portValue := parsed.Port() | |
| 738 | + | if portValue == "" || (parsed.Scheme == "http" && portValue == "80") || (parsed.Scheme == "https" && portValue == "443") { | |
| 739 | + | if strings.Contains(host, ":") { | |
| 740 | + | return "[" + host + "]" | |
| 741 | + | } | |
| 742 | + | return host | |
| 743 | + | } | |
| 744 | + | return net.JoinHostPort(host, portValue) | |
| 745 | + | } | |
| 746 | + | ||
| 747 | + | func normalizePathForHistory(path string) string { | |
| 748 | + | if path == "/" { | |
| 749 | + | return "" | |
| 750 | + | } | |
| 751 | + | if strings.HasSuffix(path, "/") { | |
| 752 | + | return strings.TrimSuffix(path, "/") | |
| 753 | + | } | |
| 754 | + | return path | |
| 755 | + | } | |
| 756 | + | ||
| 757 | + | func isTrackingQueryParam(key string) bool { | |
| 758 | + | key = strings.ToLower(strings.TrimSpace(key)) | |
| 759 | + | if key == "" { | |
| 760 | + | return false | |
| 761 | + | } | |
| 762 | + | if strings.HasPrefix(key, "utm_") { | |
| 763 | + | return true | |
| 764 | + | } | |
| 765 | + | switch key { | |
| 766 | + | case "fbclid", "gclid", "msclkid", "dclid", "yclid", "mc_cid", "mc_eid", "_hsenc", "_hsmi", "igshid": | |
| 767 | + | return true | |
| 768 | + | } | |
| 769 | + | return false | |
| 770 | + | } | |
| 771 | + | ||
| 772 | + | func isHashOnlyTransition(previous, current string) bool { | |
| 773 | + | if previous == "" || current == "" || previous == current { | |
| 774 | + | return false | |
| 775 | + | } | |
| 776 | + | ||
| 777 | + | prevParsed, prevErr := url.Parse(previous) | |
| 778 | + | currParsed, currErr := url.Parse(current) | |
| 779 | + | if prevErr != nil || currErr != nil { | |
| 780 | + | return false | |
| 781 | + | } | |
| 782 | + | ||
| 783 | + | if prevParsed.Fragment == currParsed.Fragment { | |
| 784 | + | return false | |
| 785 | + | } | |
| 786 | + | ||
| 787 | + | opts := historyCanonicalizationOptions{StripTrackingParams: stripTrackingParamsForHistoryDedup} | |
| 788 | + | prevCanonical := canonicalizeURLForHistory(previous, opts) | |
| 789 | + | currCanonical := canonicalizeURLForHistory(current, opts) | |
| 790 | + | return prevCanonical != "" && prevCanonical == currCanonical | |
| 791 | + | } | |
| 792 | + | ||
| 793 | + | func normalizePaneID(paneID string) string { | |
| 794 | + | if strings.TrimSpace(paneID) == "" { | |
| 795 | + | return "__default__" | |
| 796 | + | } | |
| 797 | + | return paneID | |
| 798 | + | } | |
A internal/application/usecase/history_recorder_test.go +650 -0
| @@ -0,0 +1,650 @@ | |||
| 1 | + | package usecase | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "errors" | |
| 6 | + | "sync" | |
| 7 | + | "testing" | |
| 8 | + | "time" | |
| 9 | + | ||
| 10 | + | "github.com/bnema/dumber/internal/application/dto" | |
| 11 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 12 | + | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | |
| 13 | + | "github.com/stretchr/testify/mock" | |
| 14 | + | "github.com/stretchr/testify/require" | |
| 15 | + | ) | |
| 16 | + | ||
| 17 | + | type recordingHistoryChangeSink struct { | |
| 18 | + | mu sync.Mutex | |
| 19 | + | changes []dto.HistoryChange | |
| 20 | + | } | |
| 21 | + | ||
| 22 | + | func (s *recordingHistoryChangeSink) OnHistoryChanged(_ context.Context, change dto.HistoryChange) { | |
| 23 | + | s.mu.Lock() | |
| 24 | + | defer s.mu.Unlock() | |
| 25 | + | s.changes = append(s.changes, change) | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func (s *recordingHistoryChangeSink) snapshot() []dto.HistoryChange { | |
| 29 | + | s.mu.Lock() | |
| 30 | + | defer s.mu.Unlock() | |
| 31 | + | out := make([]dto.HistoryChange, len(s.changes)) | |
| 32 | + | copy(out, s.changes) | |
| 33 | + | return out | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | func TestNormalizeHistoryChangeSink_TreatsTypedNilAsNoop(t *testing.T) { | |
| 37 | + | var sink *recordingHistoryChangeSink | |
| 38 | + | ||
| 39 | + | normalized := normalizeHistoryChangeSink(sink) | |
| 40 | + | ||
| 41 | + | require.NotNil(t, normalized) | |
| 42 | + | require.NotPanics(t, func() { | |
| 43 | + | normalized.OnHistoryChanged(context.Background(), dto.HistoryChange{Reasons: []dto.HistoryChangeReason{dto.HistoryChangeReasonVisit}}) | |
| 44 | + | }) | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | type metadataUpdatingHistoryRepository struct { | |
| 48 | + | *repomocks.MockHistoryRepository | |
| 49 | + | mu sync.Mutex | |
| 50 | + | entry *entity.HistoryEntry | |
| 51 | + | } | |
| 52 | + | ||
| 53 | + | func (r *metadataUpdatingHistoryRepository) UpdateMetadata(_ context.Context, entry *entity.HistoryEntry) error { | |
| 54 | + | r.mu.Lock() | |
| 55 | + | defer r.mu.Unlock() | |
| 56 | + | clone := *entry | |
| 57 | + | r.entry = &clone | |
| 58 | + | return nil | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | func (r *metadataUpdatingHistoryRepository) snapshotEntry() *entity.HistoryEntry { | |
| 62 | + | r.mu.Lock() | |
| 63 | + | defer r.mu.Unlock() | |
| 64 | + | if r.entry == nil { | |
| 65 | + | return nil | |
| 66 | + | } | |
| 67 | + | clone := *r.entry | |
| 68 | + | return &clone | |
| 69 | + | } | |
| 70 | + | ||
| 71 | + | func TestCanonicalizeURLForHistory_StripsHashTrackingAndTrailingSlash(t *testing.T) { | |
| 72 | + | got := canonicalizeURLForHistory( | |
| 73 | + | "https://Example.com/path/?utm_source=newsletter&utm_campaign=launch&a=2&b=1#section-1", | |
| 74 | + | historyCanonicalizationOptions{StripTrackingParams: true}, | |
| 75 | + | ) | |
| 76 | + | ||
| 77 | + | require.Equal(t, "https://example.com/path?a=2&b=1", got) | |
| 78 | + | } | |
| 79 | + | ||
| 80 | + | func TestCanonicalizeURLForHistory_OptionallyKeepsTrackingParams(t *testing.T) { | |
| 81 | + | got := canonicalizeURLForHistory( | |
| 82 | + | "https://example.com/path/?utm_source=newsletter&b=1#section-1", | |
| 83 | + | historyCanonicalizationOptions{StripTrackingParams: false}, | |
| 84 | + | ) | |
| 85 | + | ||
| 86 | + | require.Equal(t, "https://example.com/path?b=1&utm_source=newsletter", got) | |
| 87 | + | } | |
| 88 | + | ||
| 89 | + | func TestCanonicalizeURLForHistory_RemovesUserinfo(t *testing.T) { | |
| 90 | + | got := canonicalizeURLForHistory( | |
| 91 | + | "https://user:password@example.com/path#section-1", | |
| 92 | + | historyCanonicalizationOptions{StripTrackingParams: true}, | |
| 93 | + | ) | |
| 94 | + | ||
| 95 | + | require.Equal(t, "https://example.com/path", got) | |
| 96 | + | } | |
| 97 | + | ||
| 98 | + | func TestCanonicalizeURLForHistory_RemovesSchemeDefaultPorts(t *testing.T) { | |
| 99 | + | require.Equal(t, "http://example.com/path", canonicalizeURLForHistory( | |
| 100 | + | "http://Example.com:80/path", | |
| 101 | + | historyCanonicalizationOptions{StripTrackingParams: true}, | |
| 102 | + | )) | |
| 103 | + | require.Equal(t, "https://example.com/path", canonicalizeURLForHistory( | |
| 104 | + | "https://Example.com:443/path", | |
| 105 | + | historyCanonicalizationOptions{StripTrackingParams: true}, | |
| 106 | + | )) | |
| 107 | + | require.Equal(t, "https://example.com:8443/path", canonicalizeURLForHistory( | |
| 108 | + | "https://Example.com:8443/path", | |
| 109 | + | historyCanonicalizationOptions{StripTrackingParams: true}, | |
| 110 | + | )) | |
| 111 | + | } | |
| 112 | + | ||
| 113 | + | func TestHistoryRecorder_RecordHistory_IgnoresNonWebSchemes(t *testing.T) { | |
| 114 | + | ctx := context.Background() | |
| 115 | + | sink := &recordingHistoryChangeSink{} | |
| 116 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 117 | + | ||
| 118 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 119 | + | uc.RecordHistory(ctx, "pane-1", "file:///private/history.txt") | |
| 120 | + | uc.RecordHistory(ctx, "pane-1", "data:text/plain,secret") | |
| 121 | + | uc.RecordHistory(ctx, "pane-1", "dumb://history") | |
| 122 | + | uc.RecordHistory(ctx, "pane-1", "about:blank") | |
| 123 | + | uc.Close() | |
| 124 | + | ||
| 125 | + | require.Empty(t, sink.snapshot()) | |
| 126 | + | } | |
| 127 | + | ||
| 128 | + | func TestHistoryRecorder_UpdateHistoryTitle_IgnoresNonWebSchemes(t *testing.T) { | |
| 129 | + | ctx := context.Background() | |
| 130 | + | sink := &recordingHistoryChangeSink{} | |
| 131 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 132 | + | ||
| 133 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 134 | + | uc.UpdateHistoryTitle(ctx, "file:///private/history.txt", "Secret") | |
| 135 | + | uc.UpdateHistoryTitle(ctx, "dumb://history", "History") | |
| 136 | + | uc.Close() | |
| 137 | + | ||
| 138 | + | require.Empty(t, sink.snapshot()) | |
| 139 | + | } | |
| 140 | + | ||
| 141 | + | func TestIsHashOnlyTransition_UsesCanonicalComparison(t *testing.T) { | |
| 142 | + | tests := []struct { | |
| 143 | + | name string | |
| 144 | + | previous string | |
| 145 | + | current string | |
| 146 | + | }{ | |
| 147 | + | { | |
| 148 | + | name: "query order and tracking params", | |
| 149 | + | previous: "https://example.com/docs?utm_source=newsletter&b=2&a=1#intro", | |
| 150 | + | current: "https://example.com/docs/?a=1&b=2#api", | |
| 151 | + | }, | |
| 152 | + | { | |
| 153 | + | name: "default port", | |
| 154 | + | previous: "https://example.com:443/docs#intro", | |
| 155 | + | current: "https://example.com/docs#api", | |
| 156 | + | }, | |
| 157 | + | } | |
| 158 | + | ||
| 159 | + | for _, tt := range tests { | |
| 160 | + | t.Run(tt.name, func(t *testing.T) { | |
| 161 | + | require.True(t, isHashOnlyTransition(tt.previous, tt.current)) | |
| 162 | + | }) | |
| 163 | + | } | |
| 164 | + | } | |
| 165 | + | ||
| 166 | + | func TestHistoryRecorder_RecordHistory_CoalescesVisitsAndPublishesOneChange(t *testing.T) { | |
| 167 | + | ctx := context.Background() | |
| 168 | + | sink := &recordingHistoryChangeSink{} | |
| 169 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 170 | + | ||
| 171 | + | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/article").Return(nil, nil).Once() | |
| 172 | + | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 173 | + | return entry.URL == "https://example.com/article" && entry.VisitCount == 2 | |
| 174 | + | })).Return(nil).Once() | |
| 175 | + | ||
| 176 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 177 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/article?utm_source=feed") | |
| 178 | + | uc.RecordHistory(ctx, "pane-2", "https://example.com/article") | |
| 179 | + | uc.Close() | |
| 180 | + | ||
| 181 | + | changes := sink.snapshot() | |
| 182 | + | require.Len(t, changes, 1) | |
| 183 | + | require.Equal(t, []dto.HistoryChangeReason{dto.HistoryChangeReasonVisit}, changes[0].Reasons) | |
| 184 | + | require.Equal(t, 2, changes[0].VisitCount) | |
| 185 | + | require.Zero(t, changes[0].TitleCount) | |
| 186 | + | } | |
| 187 | + | ||
| 188 | + | func TestHistoryRecorder_CloseDrainsPendingAndPublishes(t *testing.T) { | |
| 189 | + | ctx := context.Background() | |
| 190 | + | sink := &recordingHistoryChangeSink{} | |
| 191 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 192 | + | ||
| 193 | + | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/close").Return(nil, nil).Once() | |
| 194 | + | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 195 | + | return entry.URL == "https://example.com/close" && entry.VisitCount == 1 | |
| 196 | + | })).Return(nil).Once() | |
| 197 | + | ||
| 198 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 199 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/close") | |
| 200 | + | uc.Close() | |
| 201 | + | ||
| 202 | + | changes := sink.snapshot() | |
| 203 | + | require.Len(t, changes, 1) | |
| 204 | + | require.Equal(t, 1, changes[0].VisitCount) | |
| 205 | + | } | |
| 206 | + | ||
| 207 | + | func TestHistoryRecorder_CloseCancelsInFlightWorkerFlushAndDrains(t *testing.T) { | |
| 208 | + | ctx := context.Background() | |
| 209 | + | sink := &recordingHistoryChangeSink{} | |
| 210 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 211 | + | const historyURL = "https://example.com/cancel-close" | |
| 212 | + | ||
| 213 | + | findStarted := make(chan struct{}) | |
| 214 | + | var findOnce sync.Once | |
| 215 | + | repo.EXPECT().FindByURL(mock.Anything, historyURL).RunAndReturn( | |
| 216 | + | func(ctx context.Context, _ string) (*entity.HistoryEntry, error) { | |
| 217 | + | first := false | |
| 218 | + | findOnce.Do(func() { first = true }) | |
| 219 | + | if first { | |
| 220 | + | close(findStarted) | |
| 221 | + | <-ctx.Done() | |
| 222 | + | return nil, ctx.Err() | |
| 223 | + | } | |
| 224 | + | return nil, nil | |
| 225 | + | }, | |
| 226 | + | ).Twice() | |
| 227 | + | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 228 | + | return entry.URL == historyURL && entry.VisitCount == 1 | |
| 229 | + | })).Return(nil).Once() | |
| 230 | + | ||
| 231 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 232 | + | uc.RecordHistory(ctx, "pane-1", historyURL) | |
| 233 | + | ||
| 234 | + | select { | |
| 235 | + | case <-findStarted: | |
| 236 | + | case <-time.After(time.Second): | |
| 237 | + | t.Fatal("timed out waiting for periodic history flush to start") | |
| 238 | + | } | |
| 239 | + | ||
| 240 | + | closed := make(chan struct{}) | |
| 241 | + | go func() { | |
| 242 | + | uc.Close() | |
| 243 | + | close(closed) | |
| 244 | + | }() | |
| 245 | + | ||
| 246 | + | select { | |
| 247 | + | case <-closed: | |
| 248 | + | case <-time.After(time.Second): | |
| 249 | + | t.Fatal("timed out waiting for history recorder shutdown") | |
| 250 | + | } | |
| 251 | + | ||
| 252 | + | changes := sink.snapshot() | |
| 253 | + | require.Len(t, changes, 1) | |
| 254 | + | require.Equal(t, 1, changes[0].VisitCount) | |
| 255 | + | } | |
| 256 | + | ||
| 257 | + | func TestHistoryRecorder_FailedPersistencePublishesNoChange(t *testing.T) { | |
| 258 | + | ctx := context.Background() | |
| 259 | + | sink := &recordingHistoryChangeSink{} | |
| 260 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 261 | + | ||
| 262 | + | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/fail").Return(nil, errors.New("db down")).Times(3) | |
| 263 | + | ||
| 264 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 265 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/fail") | |
| 266 | + | uc.Close() | |
| 267 | + | ||
| 268 | + | require.Empty(t, sink.snapshot()) | |
| 269 | + | } | |
| 270 | + | ||
| 271 | + | func TestHistoryRecorder_RecordHistory_DoesNotUpdateDedupeStateWhenEnqueueFails(t *testing.T) { | |
| 272 | + | ctx := context.Background() | |
| 273 | + | uc := &HistoryRecorderUseCase{ | |
| 274 | + | recentVisits: make(map[string]paneHistoryState), | |
| 275 | + | historyQueue: make(chan historyRecord, 1), | |
| 276 | + | } | |
| 277 | + | uc.historyQueue <- historyRecord{url: "https://queued.example", visits: 1} | |
| 278 | + | ||
| 279 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/full") | |
| 280 | + | ||
| 281 | + | uc.recentMu.Lock() | |
| 282 | + | _, ok := uc.recentVisits["pane-1"] | |
| 283 | + | uc.recentMu.Unlock() | |
| 284 | + | require.False(t, ok, "failed enqueue must not suppress a later retry through dedupe state") | |
| 285 | + | } | |
| 286 | + | ||
| 287 | + | func TestHistoryRecorder_UpdateHistoryTitle_PublishesTitleOnlyWithoutIncrementingVisits(t *testing.T) { | |
| 288 | + | ctx := context.Background() | |
| 289 | + | sink := &recordingHistoryChangeSink{} | |
| 290 | + | repo := &metadataUpdatingHistoryRepository{ | |
| 291 | + | MockHistoryRepository: repomocks.NewMockHistoryRepository(t), | |
| 292 | + | entry: &entity.HistoryEntry{ | |
| 293 | + | URL: "https://example.com/article", | |
| 294 | + | Title: "Old", | |
| 295 | + | VisitCount: 7, | |
| 296 | + | }, | |
| 297 | + | } | |
| 298 | + | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/article").RunAndReturn( | |
| 299 | + | func(context.Context, string) (*entity.HistoryEntry, error) { | |
| 300 | + | return repo.snapshotEntry(), nil | |
| 301 | + | }, | |
| 302 | + | ).Once() | |
| 303 | + | ||
| 304 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 305 | + | uc.UpdateHistoryTitle(ctx, "https://example.com/article", "New") | |
| 306 | + | uc.Close() | |
| 307 | + | ||
| 308 | + | after := repo.snapshotEntry() | |
| 309 | + | require.NotNil(t, after) | |
| 310 | + | require.Equal(t, "New", after.Title) | |
| 311 | + | require.Equal(t, int64(7), after.VisitCount) | |
| 312 | + | ||
| 313 | + | changes := sink.snapshot() | |
| 314 | + | require.Len(t, changes, 1) | |
| 315 | + | require.Equal(t, []dto.HistoryChangeReason{dto.HistoryChangeReasonTitle}, changes[0].Reasons) | |
| 316 | + | require.Zero(t, changes[0].VisitCount) | |
| 317 | + | require.Equal(t, 1, changes[0].TitleCount) | |
| 318 | + | } | |
| 319 | + | ||
| 320 | + | func TestHistoryRecorder_FlushPendingHistoryDiscardsTitleWhenHistoryRowMissing(t *testing.T) { | |
| 321 | + | ctx := context.Background() | |
| 322 | + | sink := &recordingHistoryChangeSink{} | |
| 323 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 324 | + | const historyURL = "https://example.com/missing-title" | |
| 325 | + | ||
| 326 | + | repo.EXPECT().FindByURL(mock.Anything, historyURL).Return(nil, nil).Once() | |
| 327 | + | ||
| 328 | + | uc := &HistoryRecorderUseCase{historyRepo: repo, changeSink: sink, ctx: ctx} | |
| 329 | + | pending := newPendingHistoryRecords() | |
| 330 | + | pending.add(historyRecord{url: historyURL, title: "Missing"}) | |
| 331 | + | ||
| 332 | + | err := uc.flushPendingHistory(ctx, pending) | |
| 333 | + | ||
| 334 | + | require.NoError(t, err) | |
| 335 | + | require.Empty(t, pending.visits) | |
| 336 | + | require.Empty(t, pending.titles) | |
| 337 | + | require.Empty(t, sink.snapshot()) | |
| 338 | + | } | |
| 339 | + | ||
| 340 | + | func TestHistoryRecorder_RecordHistory_IgnoresHashOnlyTransitionsAndPublishesNoSecondChange(t *testing.T) { | |
| 341 | + | ctx := context.Background() | |
| 342 | + | sink := &recordingHistoryChangeSink{} | |
| 343 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 344 | + | ||
| 345 | + | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/docs").Return(nil, nil).Once() | |
| 346 | + | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 347 | + | return entry.URL == "https://example.com/docs" && entry.VisitCount == 1 | |
| 348 | + | })).Return(nil).Once() | |
| 349 | + | ||
| 350 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 351 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/docs#intro") | |
| 352 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/docs#api") | |
| 353 | + | uc.Close() | |
| 354 | + | ||
| 355 | + | changes := sink.snapshot() | |
| 356 | + | require.Len(t, changes, 1) | |
| 357 | + | require.Equal(t, 1, changes[0].VisitCount) | |
| 358 | + | } | |
| 359 | + | ||
| 360 | + | func TestHistoryRecorder_RecordHistory_DedupIsPerPane(t *testing.T) { | |
| 361 | + | ctx := context.Background() | |
| 362 | + | sink := &recordingHistoryChangeSink{} | |
| 363 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 364 | + | ||
| 365 | + | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/dedupe").Return(nil, nil).Once() | |
| 366 | + | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 367 | + | return entry.URL == "https://example.com/dedupe" && entry.VisitCount == 2 | |
| 368 | + | })).Return(nil).Once() | |
| 369 | + | ||
| 370 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 371 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/dedupe") | |
| 372 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/dedupe") | |
| 373 | + | uc.RecordHistory(ctx, "pane-2", "https://example.com/dedupe") | |
| 374 | + | uc.Close() | |
| 375 | + | ||
| 376 | + | changes := sink.snapshot() | |
| 377 | + | require.Len(t, changes, 1) | |
| 378 | + | require.Equal(t, 2, changes[0].VisitCount) | |
| 379 | + | } | |
| 380 | + | ||
| 381 | + | func TestHistoryRecorder_ClearPaneHistorySerializesWithRecorderAndAllowsPaneReuse(t *testing.T) { | |
| 382 | + | ctx := context.Background() | |
| 383 | + | sink := &recordingHistoryChangeSink{} | |
| 384 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 385 | + | const historyURL = "https://example.com/reused-pane" | |
| 386 | + | ||
| 387 | + | repo.EXPECT().FindByURL(mock.Anything, historyURL).Return(nil, nil).Once() | |
| 388 | + | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 389 | + | return entry.URL == historyURL && entry.VisitCount == 2 | |
| 390 | + | })).Return(nil).Once() | |
| 391 | + | ||
| 392 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 393 | + | uc.RecordHistory(ctx, "pane-1", historyURL) | |
| 394 | + | uc.ClearPaneHistory("pane-1") | |
| 395 | + | uc.RecordHistory(ctx, "pane-1", historyURL) | |
| 396 | + | uc.Close() | |
| 397 | + | ||
| 398 | + | changes := sink.snapshot() | |
| 399 | + | require.Len(t, changes, 1) | |
| 400 | + | require.Equal(t, 2, changes[0].VisitCount) | |
| 401 | + | } | |
| 402 | + | ||
| 403 | + | func TestHistoryRecorder_FlushPendingHistoryTracksPartialFallbackVisitWrites(t *testing.T) { | |
| 404 | + | ctx := context.Background() | |
| 405 | + | sink := &recordingHistoryChangeSink{} | |
| 406 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 407 | + | const historyURL = "https://example.com/partial" | |
| 408 | + | increments := 0 | |
| 409 | + | ||
| 410 | + | repo.EXPECT().FindByURL(mock.Anything, historyURL).Return(&entity.HistoryEntry{URL: historyURL}, nil).Once() | |
| 411 | + | repo.EXPECT().IncrementVisitCount(mock.Anything, historyURL).RunAndReturn(func(context.Context, string) error { | |
| 412 | + | increments++ | |
| 413 | + | if increments == 3 { | |
| 414 | + | return errors.New("increment failed") | |
| 415 | + | } | |
| 416 | + | return nil | |
| 417 | + | }).Times(3) | |
| 418 | + | ||
| 419 | + | uc := &HistoryRecorderUseCase{historyRepo: repo, changeSink: sink, ctx: ctx} | |
| 420 | + | pending := newPendingHistoryRecords() | |
| 421 | + | pending.add(historyRecord{url: historyURL, visits: 3}) | |
| 422 | + | ||
| 423 | + | err := uc.flushPendingHistory(ctx, pending) | |
| 424 | + | ||
| 425 | + | require.ErrorIs(t, err, errHistoryPersistenceIncomplete) | |
| 426 | + | require.Equal(t, map[string]int{historyURL: 1}, pending.visits) | |
| 427 | + | require.Empty(t, pending.titles) | |
| 428 | + | changes := sink.snapshot() | |
| 429 | + | require.Len(t, changes, 1) | |
| 430 | + | require.Equal(t, 2, changes[0].VisitCount) | |
| 431 | + | } | |
| 432 | + | ||
| 433 | + | func TestHistoryRecorder_HandleHistoryControlKeepsDedupeWhenFlushFails(t *testing.T) { | |
| 434 | + | ctx := context.Background() | |
| 435 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 436 | + | const historyURL = "https://example.com/fail-reset" | |
| 437 | + | ||
| 438 | + | repo.EXPECT().FindByURL(mock.Anything, historyURL).Return(nil, errors.New("db down")).Once() | |
| 439 | + | ||
| 440 | + | uc := &HistoryRecorderUseCase{ | |
| 441 | + | historyRepo: repo, | |
| 442 | + | changeSink: &recordingHistoryChangeSink{}, | |
| 443 | + | recentVisits: map[string]paneHistoryState{ | |
| 444 | + | "pane-1": {lastCanonicalURL: historyURL}, | |
| 445 | + | }, | |
| 446 | + | ctx: ctx, | |
| 447 | + | } | |
| 448 | + | pending := newPendingHistoryRecords() | |
| 449 | + | pending.add(historyRecord{url: historyURL, visits: 1}) | |
| 450 | + | req := historyControlRequest{kind: historyControlFlushAndReset, ctx: ctx, done: make(chan error, 1)} | |
| 451 | + | ||
| 452 | + | uc.handleHistoryControl(pending, req) | |
| 453 | + | ||
| 454 | + | require.ErrorIs(t, <-req.done, errHistoryPersistenceIncomplete) | |
| 455 | + | require.Contains(t, uc.recentVisits, "pane-1") | |
| 456 | + | } | |
| 457 | + | ||
| 458 | + | func TestHistoryRecorder_BeginHistoryMutationFlushesPendingAndResetsDedupe(t *testing.T) { | |
| 459 | + | ctx := context.Background() | |
| 460 | + | sink := &recordingHistoryChangeSink{} | |
| 461 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 462 | + | ||
| 463 | + | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/reset").Return(nil, nil).Twice() | |
| 464 | + | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 465 | + | return entry.URL == "https://example.com/reset" && entry.VisitCount == 1 | |
| 466 | + | })).Return(nil).Twice() | |
| 467 | + | ||
| 468 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 469 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/reset") | |
| 470 | + | ||
| 471 | + | release, err := uc.BeginHistoryMutation(ctx) | |
| 472 | + | require.NoError(t, err) | |
| 473 | + | release() | |
| 474 | + | ||
| 475 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/reset") | |
| 476 | + | uc.Close() | |
| 477 | + | ||
| 478 | + | changes := sink.snapshot() | |
| 479 | + | require.Len(t, changes, 2) | |
| 480 | + | require.Equal(t, 1, changes[0].VisitCount) | |
| 481 | + | require.Equal(t, 1, changes[1].VisitCount) | |
| 482 | + | } | |
| 483 | + | ||
| 484 | + | func TestHistoryRecorder_BeginHistoryMutationUsesCallerContextForFlush(t *testing.T) { | |
| 485 | + | type contextKey string | |
| 486 | + | ctx := context.WithValue(context.Background(), contextKey("mutation"), "flush") | |
| 487 | + | sink := &recordingHistoryChangeSink{} | |
| 488 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 489 | + | ||
| 490 | + | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/context").RunAndReturn( | |
| 491 | + | func(ctx context.Context, _ string) (*entity.HistoryEntry, error) { | |
| 492 | + | require.Equal(t, "flush", ctx.Value(contextKey("mutation"))) | |
| 493 | + | return nil, nil | |
| 494 | + | }, | |
| 495 | + | ).Once() | |
| 496 | + | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 497 | + | return entry.URL == "https://example.com/context" && entry.VisitCount == 1 | |
| 498 | + | })).RunAndReturn(func(ctx context.Context, _ *entity.HistoryEntry) error { | |
| 499 | + | require.Equal(t, "flush", ctx.Value(contextKey("mutation"))) | |
| 500 | + | return nil | |
| 501 | + | }).Once() | |
| 502 | + | ||
| 503 | + | uc := NewHistoryRecorderUseCase(repo, sink) | |
| 504 | + | uc.RecordHistory(context.Background(), "pane-1", "https://example.com/context") | |
| 505 | + | ||
| 506 | + | release, err := uc.BeginHistoryMutation(ctx) | |
| 507 | + | require.NoError(t, err) | |
| 508 | + | release() | |
| 509 | + | uc.Close() | |
| 510 | + | } | |
| 511 | + | ||
| 512 | + | func TestHistoryRecorder_BeginHistoryMutationWaitsForInFlightShutdownDrain(t *testing.T) { | |
| 513 | + | ctx := context.Background() | |
| 514 | + | saveStarted := make(chan struct{}) | |
| 515 | + | allowSave := make(chan struct{}) | |
| 516 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 517 | + | ||
| 518 | + | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/shutdown").Return(nil, nil).Once() | |
| 519 | + | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 520 | + | return entry.URL == "https://example.com/shutdown" && entry.VisitCount == 1 | |
| 521 | + | })).RunAndReturn(func(context.Context, *entity.HistoryEntry) error { | |
| 522 | + | close(saveStarted) | |
| 523 | + | <-allowSave | |
| 524 | + | return nil | |
| 525 | + | }).Once() | |
| 526 | + | ||
| 527 | + | uc := NewHistoryRecorderUseCase(repo, nil) | |
| 528 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/shutdown") | |
| 529 | + | ||
| 530 | + | closeDone := make(chan struct{}) | |
| 531 | + | go func() { | |
| 532 | + | uc.Close() | |
| 533 | + | close(closeDone) | |
| 534 | + | }() | |
| 535 | + | select { | |
| 536 | + | case <-saveStarted: | |
| 537 | + | case <-time.After(time.Second): | |
| 538 | + | t.Fatal("timed out waiting for history save to start") | |
| 539 | + | } | |
| 540 | + | ||
| 541 | + | mutationDone := make(chan error, 1) | |
| 542 | + | beginStarted := make(chan struct{}) | |
| 543 | + | go func() { | |
| 544 | + | close(beginStarted) | |
| 545 | + | release, err := uc.BeginHistoryMutation(ctx) | |
| 546 | + | if release != nil { | |
| 547 | + | release() | |
| 548 | + | } | |
| 549 | + | mutationDone <- err | |
| 550 | + | }() | |
| 551 | + | select { | |
| 552 | + | case <-beginStarted: | |
| 553 | + | case <-time.After(time.Second): | |
| 554 | + | t.Fatal("timed out waiting for history mutation to start") | |
| 555 | + | } | |
| 556 | + | ||
| 557 | + | select { | |
| 558 | + | case err := <-mutationDone: | |
| 559 | + | t.Fatalf("history mutation returned before shutdown drain finished: %v", err) | |
| 560 | + | case <-time.After(50 * time.Millisecond): | |
| 561 | + | } | |
| 562 | + | ||
| 563 | + | close(allowSave) | |
| 564 | + | select { | |
| 565 | + | case err := <-mutationDone: | |
| 566 | + | require.NoError(t, err) | |
| 567 | + | case <-time.After(time.Second): | |
| 568 | + | t.Fatal("timed out waiting for history mutation to finish") | |
| 569 | + | } | |
| 570 | + | select { | |
| 571 | + | case <-closeDone: | |
| 572 | + | case <-time.After(time.Second): | |
| 573 | + | t.Fatal("timed out waiting for history recorder shutdown") | |
| 574 | + | } | |
| 575 | + | } | |
| 576 | + | ||
| 577 | + | func TestHistoryWorker_ReenqueueDuringFinalShutdownFlushIsRejected(t *testing.T) { | |
| 578 | + | assertReenqueueDuringFlush(t, false, "") | |
| 579 | + | } | |
| 580 | + | ||
| 581 | + | func TestHistoryWorker_ReenqueueDuringPeriodicFlushIsPersisted(t *testing.T) { | |
| 582 | + | assertReenqueueDuringFlush(t, true, "Queued title") | |
| 583 | + | } | |
| 584 | + | ||
| 585 | + | func assertReenqueueDuringFlush(t *testing.T, waitForPeriodicFlush bool, expectedTitle string) { | |
| 586 | + | t.Helper() | |
| 587 | + | ctx := context.Background() | |
| 588 | + | ||
| 589 | + | var mu sync.Mutex | |
| 590 | + | store := make(map[string]*entity.HistoryEntry) | |
| 591 | + | saved := make(chan string, 4) | |
| 592 | + | var saveOnce sync.Once | |
| 593 | + | var uc *HistoryRecorderUseCase | |
| 594 | + | ||
| 595 | + | repo := repomocks.NewMockHistoryRepository(t) | |
| 596 | + | ||
| 597 | + | repo.EXPECT().FindByURL(mock.Anything, mock.Anything).RunAndReturn( | |
| 598 | + | func(_ context.Context, url string) (*entity.HistoryEntry, error) { | |
| 599 | + | mu.Lock() | |
| 600 | + | defer mu.Unlock() | |
| 601 | + | e := store[url] | |
| 602 | + | if e == nil { | |
| 603 | + | return nil, nil | |
| 604 | + | } | |
| 605 | + | clone := *e | |
| 606 | + | return &clone, nil | |
| 607 | + | }, | |
| 608 | + | ).Maybe() | |
| 609 | + | ||
| 610 | + | repo.EXPECT().Save(mock.Anything, mock.Anything).RunAndReturn( | |
| 611 | + | func(_ context.Context, entry *entity.HistoryEntry) error { | |
| 612 | + | clone := *entry | |
| 613 | + | mu.Lock() | |
| 614 | + | store[entry.URL] = &clone | |
| 615 | + | mu.Unlock() | |
| 616 | + | ||
| 617 | + | saveOnce.Do(func() { | |
| 618 | + | if entry.URL == "https://example.com/article" { | |
| 619 | + | uc.UpdateHistoryTitle(ctx, entry.URL, "Queued title") | |
| 620 | + | } | |
| 621 | + | }) | |
| 622 | + | select { | |
| 623 | + | case saved <- entry.URL: | |
| 624 | + | default: | |
| 625 | + | } | |
| 626 | + | return nil | |
| 627 | + | }, | |
| 628 | + | ).Maybe() | |
| 629 | + | ||
| 630 | + | uc = NewHistoryRecorderUseCase(repo, nil) | |
| 631 | + | uc.RecordHistory(ctx, "pane-1", "https://example.com/article") | |
| 632 | + | if waitForPeriodicFlush { | |
| 633 | + | require.Eventually(t, func() bool { | |
| 634 | + | select { | |
| 635 | + | case url := <-saved: | |
| 636 | + | return url == "https://example.com/article" | |
| 637 | + | default: | |
| 638 | + | return false | |
| 639 | + | } | |
| 640 | + | }, time.Second, 10*time.Millisecond, "periodic flush should save the initial visit before Close") | |
| 641 | + | } | |
| 642 | + | uc.Close() | |
| 643 | + | ||
| 644 | + | mu.Lock() | |
| 645 | + | entry := store["https://example.com/article"] | |
| 646 | + | mu.Unlock() | |
| 647 | + | require.NotNil(t, entry) | |
| 648 | + | require.Equal(t, int64(1), entry.VisitCount) | |
| 649 | + | require.Equal(t, expectedTitle, entry.Title) | |
| 650 | + | } | |
M internal/application/usecase/navigate.go +7 -429
| @@ -3,108 +3,26 @@ package usecase | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | - | "net/url" | |
| 7 | - | "strings" | |
| 8 | - | "sync" | |
| 9 | - | "time" | |
| 6 | + | "math" | |
| 10 | 7 | ||
| 11 | 8 | "github.com/bnema/dumber/internal/application/port" | |
| 12 | 9 | "github.com/bnema/dumber/internal/domain/entity" | |
| 13 | - | "github.com/bnema/dumber/internal/domain/repository" | |
| 14 | 10 | "github.com/bnema/dumber/internal/logging" | |
| 15 | 11 | ) | |
| 16 | 12 | ||
| 17 | - | const ( | |
| 18 | - | // historyQueueSize is the buffer size for the async history queue. | |
| 19 | - | // If the queue is full, new records are dropped with a warning. | |
| 20 | - | historyQueueSize = 100 | |
| 21 | - | ||
| 22 | - | // logURLMaxLen is the max length for URLs in log messages. | |
| 23 | - | logURLMaxLen = 60 | |
| 24 | - | ||
| 25 | - | // historyWorkerFlushInterval coalesces bursts into fewer persistence writes. | |
| 26 | - | historyWorkerFlushInterval = 100 * time.Millisecond | |
| 27 | - | ||
| 28 | - | // historyFallbackIncrementCap bounds DB writes when delta-increment API is unavailable. | |
| 29 | - | historyFallbackIncrementCap = 256 | |
| 30 | - | ) | |
| 31 | - | ||
| 32 | - | // historyRecord holds data for async history recording. | |
| 33 | - | type historyRecord struct { | |
| 34 | - | url string | |
| 35 | - | title string // non-empty for title-update-only records | |
| 36 | - | visits int | |
| 37 | - | } | |
| 38 | - | ||
| 39 | - | type historyVisitDeltaIncrementer interface { | |
| 40 | - | IncrementVisitCountBy(ctx context.Context, url string, delta int) error | |
| 41 | - | } | |
| 42 | - | ||
| 43 | - | type historyMetadataUpdater interface { | |
| 44 | - | UpdateMetadata(ctx context.Context, entry *entity.HistoryEntry) error | |
| 45 | - | } | |
| 46 | - | ||
| 47 | - | type paneHistoryState struct { | |
| 48 | - | lastRawURL string | |
| 49 | - | lastCanonicalURL string | |
| 50 | - | lastRecordedAt time.Time | |
| 51 | - | } | |
| 52 | - | ||
| 53 | - | type historyCanonicalizationOptions struct { | |
| 54 | - | StripTrackingParams bool | |
| 55 | - | } | |
| 56 | - | ||
| 57 | - | // NavigateUseCase handles URL navigation with history recording and zoom application. | |
| 13 | + | // NavigateUseCase handles URL navigation with zoom application. | |
| 58 | 14 | type NavigateUseCase struct { | |
| 59 | - | historyRepo repository.HistoryRepository | |
| 60 | - | zoomRepo repository.ZoomRepository | |
| 61 | - | defaultZoom float64 | |
| 62 | - | recentMu sync.Mutex | |
| 63 | - | recentVisits map[string]paneHistoryState // key: pane ID, value: per-pane history state | |
| 64 | - | closeOnce sync.Once | |
| 65 | - | ||
| 66 | - | // Async history recording | |
| 67 | - | historyQueue chan historyRecord | |
| 68 | - | done chan struct{} | |
| 69 | - | wg sync.WaitGroup | |
| 70 | - | ctx context.Context // Base context for background worker | |
| 15 | + | defaultZoom float64 | |
| 71 | 16 | } | |
| 72 | 17 | ||
| 73 | 18 | // NewNavigateUseCase creates a new navigation use case. | |
| 74 | 19 | // defaultZoom is the zoom level to use when no per-domain zoom is saved (typically from config). | |
| 75 | - | func NewNavigateUseCase( | |
| 76 | - | historyRepo repository.HistoryRepository, | |
| 77 | - | zoomRepo repository.ZoomRepository, | |
| 78 | - | defaultZoom float64, | |
| 79 | - | ) *NavigateUseCase { | |
| 80 | - | if defaultZoom <= 0 { | |
| 20 | + | func NewNavigateUseCase(defaultZoom float64) *NavigateUseCase { | |
| 21 | + | if defaultZoom <= 0 || math.IsNaN(defaultZoom) || math.IsInf(defaultZoom, 0) { | |
| 81 | 22 | defaultZoom = entity.ZoomDefault | |
| 82 | 23 | } | |
| 83 | 24 | ||
| 84 | - | uc := &NavigateUseCase{ | |
| 85 | - | historyRepo: historyRepo, | |
| 86 | - | zoomRepo: zoomRepo, | |
| 87 | - | defaultZoom: defaultZoom, | |
| 88 | - | recentVisits: make(map[string]paneHistoryState), | |
| 89 | - | historyQueue: make(chan historyRecord, historyQueueSize), | |
| 90 | - | done: make(chan struct{}), | |
| 91 | - | ctx: context.Background(), | |
| 92 | - | } | |
| 93 | - | ||
| 94 | - | // Start background history worker | |
| 95 | - | uc.wg.Add(1) | |
| 96 | - | go uc.historyWorker() | |
| 97 | - | ||
| 98 | - | return uc | |
| 99 | - | } | |
| 100 | - | ||
| 101 | - | // Close shuts down the background history worker and drains any pending records. | |
| 102 | - | // This should be called when the application is shutting down. | |
| 103 | - | func (uc *NavigateUseCase) Close() { | |
| 104 | - | uc.closeOnce.Do(func() { | |
| 105 | - | close(uc.done) | |
| 106 | - | }) | |
| 107 | - | uc.wg.Wait() | |
| 25 | + | return &NavigateUseCase{defaultZoom: defaultZoom} | |
| 108 | 26 | } | |
| 109 | 27 | ||
| 110 | 28 | // NavigateInput contains parameters for navigation. | |
| @@ -119,7 +37,7 @@ type NavigateOutput struct { | |||
| 119 | 37 | AppliedZoom float64 | |
| 120 | 38 | } | |
| 121 | 39 | ||
| 122 | - | // Execute navigates to a URL and records history. | |
| 40 | + | // Execute navigates to a URL. | |
| 123 | 41 | // Zoom is applied later via LoadCommitted event in ContentCoordinator. | |
| 124 | 42 | func (uc *NavigateUseCase) Execute(ctx context.Context, input NavigateInput) (*NavigateOutput, error) { | |
| 125 | 43 | log := logging.FromContext(ctx) | |
| @@ -143,259 +61,6 @@ func (uc *NavigateUseCase) Execute(ctx context.Context, input NavigateInput) (*N | |||
| 143 | 61 | }, nil | |
| 144 | 62 | } | |
| 145 | 63 | ||
| 146 | - | // historyDeduplicationWindow is the time window for deduplicating history visits. | |
| 147 | - | // Visits to the same URL within this window count as a single visit. | |
| 148 | - | // This prevents inflation from redirects and rapid navigation. | |
| 149 | - | const historyDeduplicationWindow = 2 * time.Second | |
| 150 | - | ||
| 151 | - | // stripTrackingParamsForHistoryDedup controls whether known tracking parameters | |
| 152 | - | // are removed while canonicalizing URLs for history deduplication/storage. | |
| 153 | - | const stripTrackingParamsForHistoryDedup = true | |
| 154 | - | ||
| 155 | - | // RecordHistory queues a history entry for async recording. | |
| 156 | - | // This is non-blocking to avoid SQLite I/O on the GTK main thread. | |
| 157 | - | // Should be called on LoadCommitted when URI is guaranteed correct. | |
| 158 | - | func (uc *NavigateUseCase) RecordHistory(ctx context.Context, paneID, rawURL string) { | |
| 159 | - | log := logging.FromContext(ctx) | |
| 160 | - | rawURL = strings.TrimSpace(rawURL) | |
| 161 | - | if rawURL == "" { | |
| 162 | - | return | |
| 163 | - | } | |
| 164 | - | ||
| 165 | - | canonicalURL := canonicalizeURLForHistory(rawURL, historyCanonicalizationOptions{ | |
| 166 | - | StripTrackingParams: stripTrackingParamsForHistoryDedup, | |
| 167 | - | }) | |
| 168 | - | if canonicalURL == "" { | |
| 169 | - | return | |
| 170 | - | } | |
| 171 | - | ||
| 172 | - | paneID = normalizePaneID(paneID) | |
| 173 | - | ||
| 174 | - | now := time.Now() | |
| 175 | - | ||
| 176 | - | uc.recentMu.Lock() | |
| 177 | - | state := uc.recentVisits[paneID] | |
| 178 | - | ||
| 179 | - | // Ignore in-page hash transitions for history persistence. | |
| 180 | - | if isHashOnlyTransition(state.lastRawURL, rawURL) { | |
| 181 | - | state.lastRawURL = rawURL | |
| 182 | - | uc.recentVisits[paneID] = state | |
| 183 | - | uc.recentMu.Unlock() | |
| 184 | - | return | |
| 185 | - | } | |
| 186 | - | ||
| 187 | - | // Per-pane short-window deduplication by canonical URL. | |
| 188 | - | if state.lastCanonicalURL == canonicalURL && now.Sub(state.lastRecordedAt) < historyDeduplicationWindow { | |
| 189 | - | state.lastRawURL = rawURL | |
| 190 | - | uc.recentVisits[paneID] = state | |
| 191 | - | uc.recentMu.Unlock() | |
| 192 | - | return | |
| 193 | - | } | |
| 194 | - | ||
| 195 | - | state.lastRawURL = rawURL | |
| 196 | - | state.lastCanonicalURL = canonicalURL | |
| 197 | - | state.lastRecordedAt = now | |
| 198 | - | uc.recentVisits[paneID] = state | |
| 199 | - | uc.recentMu.Unlock() | |
| 200 | - | ||
| 201 | - | // Non-blocking send to async queue | |
| 202 | - | select { | |
| 203 | - | case uc.historyQueue <- historyRecord{url: canonicalURL, visits: 1}: | |
| 204 | - | default: | |
| 205 | - | // Queue full - log warning but don't block navigation | |
| 206 | - | log.Warn().Str("url", logging.TruncateURL(canonicalURL, logURLMaxLen)).Msg("history queue full, dropping record") | |
| 207 | - | } | |
| 208 | - | } | |
| 209 | - | ||
| 210 | - | // ClearPaneHistory removes per-pane deduplication state to prevent unbounded growth. | |
| 211 | - | // Callers should invoke this when a pane is closed. | |
| 212 | - | func (uc *NavigateUseCase) ClearPaneHistory(paneID string) { | |
| 213 | - | paneID = normalizePaneID(paneID) | |
| 214 | - | uc.recentMu.Lock() | |
| 215 | - | delete(uc.recentVisits, paneID) | |
| 216 | - | uc.recentMu.Unlock() | |
| 217 | - | } | |
| 218 | - | ||
| 219 | - | // historyWorker is a background goroutine that processes history records. | |
| 220 | - | // It drains the queue and persists records to the database without blocking the UI. | |
| 221 | - | func (uc *NavigateUseCase) historyWorker() { | |
| 222 | - | defer uc.wg.Done() | |
| 223 | - | ||
| 224 | - | log := logging.FromContext(uc.ctx).With(). | |
| 225 | - | Str("component", "history-worker"). | |
| 226 | - | Logger() | |
| 227 | - | ||
| 228 | - | ticker := time.NewTicker(historyWorkerFlushInterval) | |
| 229 | - | defer ticker.Stop() | |
| 230 | - | ||
| 231 | - | pendingVisits := make(map[string]int) | |
| 232 | - | pendingTitles := make(map[string]string) | |
| 233 | - | ||
| 234 | - | processRecord := func(record historyRecord) { | |
| 235 | - | if record.title != "" { | |
| 236 | - | pendingTitles[record.url] = record.title | |
| 237 | - | } | |
| 238 | - | if record.visits > 0 { | |
| 239 | - | pendingVisits[record.url] += record.visits | |
| 240 | - | } | |
| 241 | - | } | |
| 242 | - | ||
| 243 | - | flushPending := func() { | |
| 244 | - | visitCount := len(pendingVisits) | |
| 245 | - | titleCount := len(pendingTitles) | |
| 246 | - | if visitCount == 0 && titleCount == 0 { | |
| 247 | - | return | |
| 248 | - | } | |
| 249 | - | ||
| 250 | - | log.Debug().Int("visits", visitCount).Int("titles", titleCount).Msg("flushing pending history") | |
| 251 | - | ||
| 252 | - | // Swap maps first, then materialize immutable slices before invoking | |
| 253 | - | // persistence callbacks. This keeps any re-enqueued records isolated to | |
| 254 | - | // the fresh pending maps for the next flush cycle. | |
| 255 | - | visits := pendingVisits | |
| 256 | - | titles := pendingTitles | |
| 257 | - | pendingVisits = make(map[string]int) | |
| 258 | - | pendingTitles = make(map[string]string) | |
| 259 | - | ||
| 260 | - | visitRecords := make([]historyRecord, 0, len(visits)) | |
| 261 | - | for historyURL, v := range visits { | |
| 262 | - | visitRecords = append(visitRecords, historyRecord{url: historyURL, visits: v}) | |
| 263 | - | } | |
| 264 | - | titleUpdates := make([]historyRecord, 0, len(titles)) | |
| 265 | - | for historyURL, title := range titles { | |
| 266 | - | titleUpdates = append(titleUpdates, historyRecord{url: historyURL, title: title}) | |
| 267 | - | } | |
| 268 | - | ||
| 269 | - | for _, record := range visitRecords { | |
| 270 | - | uc.persistHistory(uc.ctx, record) | |
| 271 | - | } | |
| 272 | - | for _, record := range titleUpdates { | |
| 273 | - | uc.persistTitleUpdate(uc.ctx, record.url, record.title) | |
| 274 | - | } | |
| 275 | - | } | |
| 276 | - | ||
| 277 | - | drainQueue := func() { | |
| 278 | - | for { | |
| 279 | - | select { | |
| 280 | - | case record := <-uc.historyQueue: | |
| 281 | - | processRecord(record) | |
| 282 | - | default: | |
| 283 | - | return | |
| 284 | - | } | |
| 285 | - | } | |
| 286 | - | } | |
| 287 | - | ||
| 288 | - | for { | |
| 289 | - | select { | |
| 290 | - | case record := <-uc.historyQueue: | |
| 291 | - | processRecord(record) | |
| 292 | - | case <-ticker.C: | |
| 293 | - | flushPending() | |
| 294 | - | case <-uc.done: | |
| 295 | - | log.Debug().Int("remaining", len(uc.historyQueue)).Msg("draining history queue") | |
| 296 | - | drainQueue() | |
| 297 | - | flushPending() | |
| 298 | - | log.Debug().Msg("history worker shutdown complete") | |
| 299 | - | return | |
| 300 | - | } | |
| 301 | - | } | |
| 302 | - | } | |
| 303 | - | ||
| 304 | - | // persistHistory writes a history record to the database. | |
| 305 | - | // Called from the background worker goroutine. | |
| 306 | - | func (uc *NavigateUseCase) persistHistory(ctx context.Context, record historyRecord) { | |
| 307 | - | log := logging.FromContext(ctx) | |
| 308 | - | ||
| 309 | - | // Check if entry exists | |
| 310 | - | existing, err := uc.historyRepo.FindByURL(ctx, record.url) | |
| 311 | - | if err != nil { | |
| 312 | - | log.Warn().Err(err).Str("url", logging.RedactURL(record.url)).Msg("failed to check history") | |
| 313 | - | return | |
| 314 | - | } | |
| 315 | - | ||
| 316 | - | if existing != nil { | |
| 317 | - | delta := max(1, record.visits) | |
| 318 | - | if deltaWriter, ok := uc.historyRepo.(historyVisitDeltaIncrementer); ok { | |
| 319 | - | if err := deltaWriter.IncrementVisitCountBy(ctx, record.url, delta); err != nil { | |
| 320 | - | log.Warn().Err(err).Str("url", logging.RedactURL(record.url)).Int("delta", delta).Msg("failed to increment visit count by delta") | |
| 321 | - | return | |
| 322 | - | } | |
| 323 | - | return | |
| 324 | - | } | |
| 325 | - | iterations := delta | |
| 326 | - | if iterations > historyFallbackIncrementCap { | |
| 327 | - | log.Warn(). | |
| 328 | - | Str("url", logging.RedactURL(record.url)). | |
| 329 | - | Int("delta", delta). | |
| 330 | - | Int("cap", historyFallbackIncrementCap). | |
| 331 | - | Msg("visit count fallback increment capped") | |
| 332 | - | iterations = historyFallbackIncrementCap | |
| 333 | - | } | |
| 334 | - | for i := 0; i < iterations; i++ { | |
| 335 | - | if err := uc.historyRepo.IncrementVisitCount(ctx, record.url); err != nil { | |
| 336 | - | log.Warn().Err(err).Str("url", logging.RedactURL(record.url)).Msg("failed to increment visit count") | |
| 337 | - | return | |
| 338 | - | } | |
| 339 | - | } | |
| 340 | - | } else { | |
| 341 | - | // Create new entry | |
| 342 | - | entry := entity.NewHistoryEntry(record.url, "") | |
| 343 | - | entry.VisitCount = int64(max(1, record.visits)) | |
| 344 | - | if err := uc.historyRepo.Save(ctx, entry); err != nil { | |
| 345 | - | log.Warn().Err(err).Str("url", logging.RedactURL(record.url)).Msg("failed to save history") | |
| 346 | - | } | |
| 347 | - | } | |
| 348 | - | } | |
| 349 | - | ||
| 350 | - | // persistTitleUpdate writes a title update to the database. | |
| 351 | - | // Called from the background worker goroutine. | |
| 352 | - | func (uc *NavigateUseCase) persistTitleUpdate(ctx context.Context, historyURL, title string) { | |
| 353 | - | log := logging.FromContext(ctx) | |
| 354 | - | ||
| 355 | - | if title == "" { | |
| 356 | - | return | |
| 357 | - | } | |
| 358 | - | ||
| 359 | - | entry, err := uc.historyRepo.FindByURL(ctx, historyURL) | |
| 360 | - | if err != nil { | |
| 361 | - | log.Warn().Err(err).Str("url", logging.RedactURL(historyURL)).Msg("failed to find history entry for title update") | |
| 362 | - | return | |
| 363 | - | } | |
| 364 | - | if entry == nil { | |
| 365 | - | return | |
| 366 | - | } | |
| 367 | - | ||
| 368 | - | entry.Title = title | |
| 369 | - | if updater, ok := uc.historyRepo.(historyMetadataUpdater); ok { | |
| 370 | - | if err := updater.UpdateMetadata(ctx, entry); err != nil { | |
| 371 | - | log.Warn().Err(err).Str("url", logging.RedactURL(historyURL)).Msg("failed to update history metadata") | |
| 372 | - | } | |
| 373 | - | return | |
| 374 | - | } | |
| 375 | - | if err := uc.historyRepo.Save(ctx, entry); err != nil { | |
| 376 | - | log.Warn().Err(err).Str("url", logging.RedactURL(historyURL)).Msg("failed to update history title") | |
| 377 | - | } | |
| 378 | - | } | |
| 379 | - | ||
| 380 | - | // UpdateHistoryTitle queues a title update for a history entry. The actual | |
| 381 | - | // DB write happens in the background historyWorker to avoid concurrent | |
| 382 | - | // repo access from the GTK main thread and the worker goroutine. | |
| 383 | - | func (uc *NavigateUseCase) UpdateHistoryTitle(_ context.Context, historyURL, title string) { | |
| 384 | - | historyURL = canonicalizeURLForHistory(historyURL, historyCanonicalizationOptions{ | |
| 385 | - | StripTrackingParams: stripTrackingParamsForHistoryDedup, | |
| 386 | - | }) | |
| 387 | - | if historyURL == "" { | |
| 388 | - | return | |
| 389 | - | } | |
| 390 | - | ||
| 391 | - | // Non-blocking send — same queue as visit records. | |
| 392 | - | select { | |
| 393 | - | case uc.historyQueue <- historyRecord{url: historyURL, title: title}: | |
| 394 | - | default: | |
| 395 | - | logging.FromContext(uc.ctx).Warn().Str("url", logging.RedactURL(historyURL)).Msg("history queue full, title update dropped") | |
| 396 | - | } | |
| 397 | - | } | |
| 398 | - | ||
| 399 | 64 | // Reload reloads the current page. | |
| 400 | 65 | func (uc *NavigateUseCase) Reload(ctx context.Context, webview port.WebView, bypassCache bool) error { | |
| 401 | 66 | log := logging.FromContext(ctx).With().Float64("default_zoom", uc.defaultZoom).Logger() | |
| @@ -413,90 +78,3 @@ func (uc *NavigateUseCase) Stop(ctx context.Context, webview port.WebView) error | |||
| 413 | 78 | log.Debug().Msg("stopping page load") | |
| 414 | 79 | return webview.Stop(ctx) | |
| 415 | 80 | } | |
| 416 | - | ||
| 417 | - | func canonicalizeURLForHistory(raw string, opts historyCanonicalizationOptions) string { | |
| 418 | - | raw = strings.TrimSpace(raw) | |
| 419 | - | if raw == "" { | |
| 420 | - | return "" | |
| 421 | - | } | |
| 422 | - | ||
| 423 | - | parsed, err := url.Parse(raw) | |
| 424 | - | if err != nil { | |
| 425 | - | return strings.TrimSuffix(raw, "/") | |
| 426 | - | } | |
| 427 | - | ||
| 428 | - | parsed.Scheme = strings.ToLower(parsed.Scheme) | |
| 429 | - | parsed.Host = strings.ToLower(parsed.Host) | |
| 430 | - | parsed.Fragment = "" | |
| 431 | - | parsed.Path = normalizePathForHistory(parsed.Path) | |
| 432 | - | ||
| 433 | - | query := parsed.Query() | |
| 434 | - | if opts.StripTrackingParams { | |
| 435 | - | for key := range query { | |
| 436 | - | if isTrackingQueryParam(key) { | |
| 437 | - | query.Del(key) | |
| 438 | - | } | |
| 439 | - | } | |
| 440 | - | } | |
| 441 | - | parsed.RawQuery = query.Encode() | |
| 442 | - | ||
| 443 | - | return parsed.String() | |
| 444 | - | } | |
| 445 | - | ||
| 446 | - | func normalizePathForHistory(path string) string { | |
| 447 | - | if path == "/" { | |
| 448 | - | return "" | |
| 449 | - | } | |
| 450 | - | if strings.HasSuffix(path, "/") { | |
| 451 | - | return strings.TrimSuffix(path, "/") | |
| 452 | - | } | |
| 453 | - | return path | |
| 454 | - | } | |
| 455 | - | ||
| 456 | - | func isTrackingQueryParam(key string) bool { | |
| 457 | - | key = strings.ToLower(strings.TrimSpace(key)) | |
| 458 | - | if key == "" { | |
| 459 | - | return false | |
| 460 | - | } | |
| 461 | - | if strings.HasPrefix(key, "utm_") { | |
| 462 | - | return true | |
| 463 | - | } | |
| 464 | - | switch key { | |
| 465 | - | case "fbclid", "gclid", "msclkid", "dclid", "yclid", "mc_cid", "mc_eid", "_hsenc", "_hsmi", "igshid": | |
| 466 | - | return true | |
| 467 | - | } | |
| 468 | - | return false | |
| 469 | - | } | |
| 470 | - | ||
| 471 | - | func isHashOnlyTransition(previous, current string) bool { | |
| 472 | - | if previous == "" || current == "" || previous == current { | |
| 473 | - | return false | |
| 474 | - | } | |
| 475 | - | ||
| 476 | - | prevParsed, prevErr := url.Parse(previous) | |
| 477 | - | currParsed, currErr := url.Parse(current) | |
| 478 | - | if prevErr != nil || currErr != nil { | |
| 479 | - | return false | |
| 480 | - | } | |
| 481 | - | ||
| 482 | - | if !strings.EqualFold(prevParsed.Scheme, currParsed.Scheme) { | |
| 483 | - | return false | |
| 484 | - | } | |
| 485 | - | if !strings.EqualFold(prevParsed.Host, currParsed.Host) { | |
| 486 | - | return false | |
| 487 | - | } | |
| 488 | - | if normalizePathForHistory(prevParsed.Path) != normalizePathForHistory(currParsed.Path) { | |
| 489 | - | return false | |
| 490 | - | } | |
| 491 | - | if prevParsed.RawQuery != currParsed.RawQuery { | |
| 492 | - | return false | |
| 493 | - | } | |
| 494 | - | return prevParsed.Fragment != currParsed.Fragment | |
| 495 | - | } | |
| 496 | - | ||
| 497 | - | func normalizePaneID(paneID string) string { | |
| 498 | - | if strings.TrimSpace(paneID) == "" { | |
| 499 | - | return "__default__" | |
| 500 | - | } | |
| 501 | - | return paneID | |
| 502 | - | } | |
M internal/application/usecase/navigate_test.go +81 -132
| @@ -2,156 +2,105 @@ package usecase | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | - | "path/filepath" | |
| 6 | - | "sync" | |
| 5 | + | "errors" | |
| 6 | + | "math" | |
| 7 | 7 | "testing" | |
| 8 | - | "time" | |
| 9 | 8 | ||
| 9 | + | "github.com/bnema/dumber/internal/application/port" | |
| 10 | 10 | "github.com/bnema/dumber/internal/domain/entity" | |
| 11 | - | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | |
| 12 | - | "github.com/bnema/dumber/internal/infrastructure/persistence/sqlite" | |
| 13 | - | "github.com/stretchr/testify/mock" | |
| 14 | 11 | "github.com/stretchr/testify/require" | |
| 15 | 12 | ) | |
| 16 | 13 | ||
| 17 | - | func TestCanonicalizeURLForHistory_StripsHashTrackingAndTrailingSlash(t *testing.T) { | |
| 18 | - | got := canonicalizeURLForHistory( | |
| 19 | - | "https://Example.com/path/?utm_source=newsletter&utm_campaign=launch&a=2&b=1#section-1", | |
| 20 | - | historyCanonicalizationOptions{StripTrackingParams: true}, | |
| 21 | - | ) | |
| 22 | - | ||
| 23 | - | require.Equal(t, "https://example.com/path?a=2&b=1", got) | |
| 14 | + | type fakeWebView struct { | |
| 15 | + | loaded string | |
| 16 | + | loadErr error | |
| 24 | 17 | } | |
| 25 | 18 | ||
| 26 | - | func TestCanonicalizeURLForHistory_OptionallyKeepsTrackingParams(t *testing.T) { | |
| 27 | - | got := canonicalizeURLForHistory( | |
| 28 | - | "https://example.com/path/?utm_source=newsletter&b=1#section-1", | |
| 29 | - | historyCanonicalizationOptions{StripTrackingParams: false}, | |
| 30 | - | ) | |
| 31 | - | ||
| 32 | - | require.Equal(t, "https://example.com/path?b=1&utm_source=newsletter", got) | |
| 19 | + | func (f *fakeWebView) LoadURI(_ context.Context, uri string) error { | |
| 20 | + | if f.loadErr != nil { | |
| 21 | + | return f.loadErr | |
| 22 | + | } | |
| 23 | + | f.loaded = uri | |
| 24 | + | return nil | |
| 33 | 25 | } | |
| 34 | - | ||
| 35 | - | func TestRecordHistory_IgnoresHashOnlyTransitions(t *testing.T) { | |
| 36 | - | ctx := context.Background() | |
| 37 | - | repo := repomocks.NewMockHistoryRepository(t) | |
| 38 | - | ||
| 39 | - | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/docs").Return(nil, nil).Once() | |
| 40 | - | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 41 | - | return entry.URL == "https://example.com/docs" && entry.VisitCount == 1 | |
| 42 | - | })).Return(nil).Once() | |
| 43 | - | ||
| 44 | - | uc := NewNavigateUseCase(repo, nil, entity.ZoomDefault) | |
| 45 | - | uc.RecordHistory(ctx, "pane-1", "https://example.com/docs#intro") | |
| 46 | - | uc.RecordHistory(ctx, "pane-1", "https://example.com/docs#api") | |
| 47 | - | uc.Close() | |
| 26 | + | func (*fakeWebView) LoadHTML(context.Context, string, string) error { return nil } | |
| 27 | + | func (*fakeWebView) Reload(context.Context) error { return nil } | |
| 28 | + | func (*fakeWebView) ReloadBypassCache(context.Context) error { return nil } | |
| 29 | + | func (*fakeWebView) Stop(context.Context) error { return nil } | |
| 30 | + | func (*fakeWebView) GoBack(context.Context) error { return nil } | |
| 31 | + | func (*fakeWebView) GoForward(context.Context) error { return nil } | |
| 32 | + | func (*fakeWebView) ID() port.WebViewID { return 1 } | |
| 33 | + | func (*fakeWebView) State() port.WebViewState { return port.WebViewState{} } | |
| 34 | + | func (f *fakeWebView) URI() string { return f.loaded } | |
| 35 | + | func (*fakeWebView) Title() string { return "" } | |
| 36 | + | func (*fakeWebView) IsLoading() bool { return false } | |
| 37 | + | func (*fakeWebView) EstimatedProgress() float64 { return 1 } | |
| 38 | + | func (*fakeWebView) CanGoBack() bool { return false } | |
| 39 | + | func (*fakeWebView) CanGoForward() bool { return false } | |
| 40 | + | func (*fakeWebView) SetZoomLevel(context.Context, float64) error { return nil } | |
| 41 | + | func (*fakeWebView) GetZoomLevel() float64 { return 1 } | |
| 42 | + | func (*fakeWebView) GetFindController() port.FindController { return nil } | |
| 43 | + | func (*fakeWebView) SetCallbacks(*port.WebViewCallbacks) {} | |
| 44 | + | func (*fakeWebView) RunJavaScript(context.Context, string) {} | |
| 45 | + | func (*fakeWebView) SetBackgroundColor(float64, float64, float64, float64) { | |
| 48 | 46 | } | |
| 49 | - | ||
| 50 | - | func TestRecordHistory_DedupIsPerPaneAndCoalescesVisits(t *testing.T) { | |
| 47 | + | func (*fakeWebView) ResetBackgroundToDefault() {} | |
| 48 | + | func (*fakeWebView) Favicon() port.Texture { return nil } | |
| 49 | + | func (*fakeWebView) Generation() uint64 { return 0 } | |
| 50 | + | func (*fakeWebView) IsFullscreen() bool { return false } | |
| 51 | + | func (*fakeWebView) IsPlayingAudio() bool { return false } | |
| 52 | + | func (*fakeWebView) IsDestroyed() bool { return false } | |
| 53 | + | func (*fakeWebView) Destroy() {} | |
| 54 | + | ||
| 55 | + | func TestNavigateUseCase_ExecuteLoadsURLAndReturnsDefaultZoom(t *testing.T) { | |
| 51 | 56 | ctx := context.Background() | |
| 52 | - | repo := repomocks.NewMockHistoryRepository(t) | |
| 53 | - | ||
| 54 | - | repo.EXPECT().FindByURL(mock.Anything, "https://example.com/article").Return(nil, nil).Once() | |
| 55 | - | repo.EXPECT().Save(mock.Anything, mock.MatchedBy(func(entry *entity.HistoryEntry) bool { | |
| 56 | - | return entry.URL == "https://example.com/article" && entry.VisitCount == 2 | |
| 57 | - | })).Return(nil).Once() | |
| 57 | + | wv := &fakeWebView{} | |
| 58 | + | uc := NewNavigateUseCase(entity.ZoomDefault) | |
| 58 | 59 | ||
| 59 | - | uc := NewNavigateUseCase(repo, nil, entity.ZoomDefault) | |
| 60 | - | uc.RecordHistory(ctx, "pane-1", "https://example.com/article?utm_source=feed") | |
| 61 | - | uc.RecordHistory(ctx, "pane-1", "https://example.com/article?utm_source=feed") | |
| 62 | - | uc.RecordHistory(ctx, "pane-2", "https://example.com/article") | |
| 63 | - | uc.Close() | |
| 64 | - | } | |
| 60 | + | out, err := uc.Execute(ctx, NavigateInput{URL: "https://example.com", PaneID: "pane-1", WebView: wv}) | |
| 65 | 61 | ||
| 66 | - | func TestUpdateHistoryTitle_UsesMetadataUpdateWithoutIncrementingVisits(t *testing.T) { | |
| 67 | - | ctx := context.Background() | |
| 68 | - | dbPath := filepath.Join(t.TempDir(), "dumber.db") | |
| 69 | - | db, err := sqlite.NewConnection(ctx, dbPath) | |
| 70 | 62 | require.NoError(t, err) | |
| 71 | - | t.Cleanup(func() { _ = db.Close() }) | |
| 72 | - | ||
| 73 | - | repo := sqlite.NewHistoryRepository(db) | |
| 74 | - | require.NoError(t, repo.Save(ctx, &entity.HistoryEntry{ | |
| 75 | - | URL: "https://example.com/article", | |
| 76 | - | Title: "Old", | |
| 77 | - | })) | |
| 78 | - | ||
| 79 | - | before, err := repo.FindByURL(ctx, "https://example.com/article") | |
| 80 | - | require.NoError(t, err) | |
| 81 | - | require.NotNil(t, before) | |
| 82 | - | ||
| 83 | - | uc := NewNavigateUseCase(repo, nil, entity.ZoomDefault) | |
| 84 | - | uc.UpdateHistoryTitle(ctx, "https://example.com/article", "New") | |
| 85 | - | uc.Close() | |
| 63 | + | require.Equal(t, "https://example.com", wv.loaded) | |
| 64 | + | require.InDelta(t, entity.ZoomDefault, out.AppliedZoom, 0.0001) | |
| 65 | + | } | |
| 86 | 66 | ||
| 87 | - | after, err := repo.FindByURL(ctx, "https://example.com/article") | |
| 88 | - | require.NoError(t, err) | |
| 89 | - | require.NotNil(t, after) | |
| 90 | - | require.Equal(t, "New", after.Title) | |
| 91 | - | require.Equal(t, before.VisitCount, after.VisitCount) | |
| 67 | + | func TestNavigateUseCase_ExecuteUsesConfiguredOrFallbackZoom(t *testing.T) { | |
| 68 | + | tests := []struct { | |
| 69 | + | name string | |
| 70 | + | configuredZoom float64 | |
| 71 | + | wantZoom float64 | |
| 72 | + | }{ | |
| 73 | + | {name: "zero falls back", configuredZoom: 0, wantZoom: entity.ZoomDefault}, | |
| 74 | + | {name: "negative falls back", configuredZoom: -0.5, wantZoom: entity.ZoomDefault}, | |
| 75 | + | {name: "NaN falls back", configuredZoom: math.NaN(), wantZoom: entity.ZoomDefault}, | |
| 76 | + | {name: "positive infinity falls back", configuredZoom: math.Inf(1), wantZoom: entity.ZoomDefault}, | |
| 77 | + | {name: "positive is preserved", configuredZoom: 1.25, wantZoom: 1.25}, | |
| 78 | + | } | |
| 79 | + | ||
| 80 | + | for _, tt := range tests { | |
| 81 | + | t.Run(tt.name, func(t *testing.T) { | |
| 82 | + | ctx := context.Background() | |
| 83 | + | wv := &fakeWebView{} | |
| 84 | + | uc := NewNavigateUseCase(tt.configuredZoom) | |
| 85 | + | ||
| 86 | + | out, err := uc.Execute(ctx, NavigateInput{URL: "https://example.com", PaneID: "pane-1", WebView: wv}) | |
| 87 | + | ||
| 88 | + | require.NoError(t, err) | |
| 89 | + | require.Equal(t, "https://example.com", wv.loaded) | |
| 90 | + | require.InDelta(t, tt.wantZoom, out.AppliedZoom, 0.0001) | |
| 91 | + | }) | |
| 92 | + | } | |
| 92 | 93 | } | |
| 93 | 94 | ||
| 94 | - | func TestHistoryWorker_ReenqueueDuringFlushIsPersistedOnShutdown(t *testing.T) { | |
| 95 | + | func TestNavigateUseCase_ExecuteReturnsLoadError(t *testing.T) { | |
| 95 | 96 | ctx := context.Background() | |
| 97 | + | loadErr := errors.New("load failed") | |
| 98 | + | wv := &fakeWebView{loadErr: loadErr} | |
| 99 | + | uc := NewNavigateUseCase(entity.ZoomDefault) | |
| 96 | 100 | ||
| 97 | - | // Stateful store — Save writes here, FindByURL reads from here. | |
| 98 | - | var mu sync.Mutex | |
| 99 | - | store := make(map[string]*entity.HistoryEntry) | |
| 100 | - | ||
| 101 | - | // Fires once during the first Save to re-enqueue a title update while | |
| 102 | - | // the flush is in progress. This is the exact race that caused the | |
| 103 | - | // concurrent map crash (fixed by swapping maps in flushPending). | |
| 104 | - | var saveOnce sync.Once | |
| 105 | - | var uc *NavigateUseCase | |
| 106 | - | ||
| 107 | - | repo := repomocks.NewMockHistoryRepository(t) | |
| 108 | - | ||
| 109 | - | repo.EXPECT().FindByURL(mock.Anything, mock.Anything).RunAndReturn( | |
| 110 | - | func(_ context.Context, url string) (*entity.HistoryEntry, error) { | |
| 111 | - | mu.Lock() | |
| 112 | - | defer mu.Unlock() | |
| 113 | - | e := store[url] | |
| 114 | - | if e == nil { | |
| 115 | - | return nil, nil | |
| 116 | - | } | |
| 117 | - | clone := *e | |
| 118 | - | return &clone, nil | |
| 119 | - | }, | |
| 120 | - | ).Maybe() | |
| 121 | - | ||
| 122 | - | repo.EXPECT().Save(mock.Anything, mock.Anything).RunAndReturn( | |
| 123 | - | func(_ context.Context, entry *entity.HistoryEntry) error { | |
| 124 | - | clone := *entry | |
| 125 | - | mu.Lock() | |
| 126 | - | store[entry.URL] = &clone | |
| 127 | - | mu.Unlock() | |
| 128 | - | ||
| 129 | - | // Re-enqueue a title update during the first Save. | |
| 130 | - | saveOnce.Do(func() { | |
| 131 | - | if entry.URL == "https://example.com/article" { | |
| 132 | - | uc.UpdateHistoryTitle(ctx, entry.URL, "Queued title") | |
| 133 | - | } | |
| 134 | - | }) | |
| 135 | - | return nil | |
| 136 | - | }, | |
| 137 | - | ).Maybe() | |
| 138 | - | ||
| 139 | - | uc = NewNavigateUseCase(repo, nil, entity.ZoomDefault) | |
| 140 | - | uc.RecordHistory(ctx, "pane-1", "https://example.com/article") | |
| 141 | - | ||
| 142 | - | // The history worker flushes on a timer; there is no exported sync | |
| 143 | - | // mechanism, so we sleep before shutting down. Using 5× the flush | |
| 144 | - | // interval for headroom — this is a known flaky pattern, but | |
| 145 | - | // restructuring the worker for deterministic sync is out of scope. | |
| 146 | - | // Close() performs a final flush, so items re-enqueued during the | |
| 147 | - | // timed flush are still captured on shutdown. | |
| 148 | - | time.Sleep(historyWorkerFlushInterval * 5) | |
| 149 | - | uc.Close() | |
| 101 | + | out, err := uc.Execute(ctx, NavigateInput{URL: "https://example.com", PaneID: "pane-1", WebView: wv}) | |
| 150 | 102 | ||
| 151 | - | mu.Lock() | |
| 152 | - | entry := store["https://example.com/article"] | |
| 153 | - | mu.Unlock() | |
| 154 | - | require.NotNil(t, entry) | |
| 155 | - | require.Equal(t, int64(1), entry.VisitCount) | |
| 156 | - | require.Equal(t, "Queued title", entry.Title) | |
| 103 | + | require.Nil(t, out) | |
| 104 | + | require.ErrorIs(t, err, loadErr) | |
| 105 | + | require.Contains(t, err.Error(), "failed to load URL") | |
| 157 | 106 | } | |
M internal/application/usecase/search_history.go +132 -2
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | 6 | "strings" | |
| 7 | + | "sync" | |
| 7 | 8 | "time" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/bnema/dumber/internal/application/dto" | |
| @@ -34,14 +35,48 @@ const ( | |||
| 34 | 35 | ||
| 35 | 36 | // SearchHistoryUseCase handles history search and retrieval operations. | |
| 36 | 37 | type SearchHistoryUseCase struct { | |
| 37 | - | historyRepo repository.HistoryRepository | |
| 38 | + | historyRepo repository.HistoryRepository | |
| 39 | + | changeSinkMu sync.RWMutex | |
| 40 | + | changeSink port.HistoryChangeSink | |
| 41 | + | mutationCoordinatorMu sync.RWMutex | |
| 42 | + | mutationCoordinator port.HistoryMutationCoordinator | |
| 38 | 43 | } | |
| 39 | 44 | ||
| 40 | 45 | // NewSearchHistoryUseCase creates a new history search use case. | |
| 41 | - | func NewSearchHistoryUseCase(historyRepo repository.HistoryRepository) *SearchHistoryUseCase { | |
| 46 | + | func NewSearchHistoryUseCase(historyRepo repository.HistoryRepository, changeSink ...port.HistoryChangeSink) *SearchHistoryUseCase { | |
| 47 | + | sink := port.HistoryChangeSink(nil) | |
| 48 | + | if len(changeSink) > 0 { | |
| 49 | + | sink = changeSink[0] | |
| 50 | + | } | |
| 51 | + | ||
| 42 | 52 | return &SearchHistoryUseCase{ | |
| 43 | 53 | historyRepo: historyRepo, | |
| 54 | + | changeSink: normalizeHistoryChangeSink(sink), | |
| 55 | + | } | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | // SetHistoryChangeSink sets the sink for persisted history change notifications. | |
| 59 | + | func (uc *SearchHistoryUseCase) SetHistoryChangeSink(changeSink port.HistoryChangeSink) { | |
| 60 | + | if uc == nil { | |
| 61 | + | return | |
| 44 | 62 | } | |
| 63 | + | changeSink = normalizeHistoryChangeSink(changeSink) | |
| 64 | + | uc.changeSinkMu.Lock() | |
| 65 | + | uc.changeSink = changeSink | |
| 66 | + | uc.changeSinkMu.Unlock() | |
| 67 | + | } | |
| 68 | + | ||
| 69 | + | // SetHistoryMutationCoordinator sets the coordinator used before destructive history mutations. | |
| 70 | + | func (uc *SearchHistoryUseCase) SetHistoryMutationCoordinator(coordinator port.HistoryMutationCoordinator) { | |
| 71 | + | if uc == nil { | |
| 72 | + | return | |
| 73 | + | } | |
| 74 | + | if isNilInterface(coordinator) { | |
| 75 | + | coordinator = nil | |
| 76 | + | } | |
| 77 | + | uc.mutationCoordinatorMu.Lock() | |
| 78 | + | uc.mutationCoordinator = coordinator | |
| 79 | + | uc.mutationCoordinatorMu.Unlock() | |
| 45 | 80 | } | |
| 46 | 81 | ||
| 47 | 82 | // SearchInput is an alias for dto.HistorySearchInput. | |
| @@ -252,10 +287,19 @@ func (uc *SearchHistoryUseCase) ClearOlderThan(ctx context.Context, before time. | |||
| 252 | 287 | log := logging.FromContext(ctx) | |
| 253 | 288 | log.Debug().Time("before", before).Msg("clearing old history") | |
| 254 | 289 | ||
| 290 | + | release, err := uc.beginHistoryMutation(ctx) | |
| 291 | + | if err != nil { | |
| 292 | + | return fmt.Errorf("failed to prepare history mutation: %w", err) | |
| 293 | + | } | |
| 294 | + | release = onceRelease(release) | |
| 295 | + | defer release() | |
| 296 | + | ||
| 255 | 297 | if err := uc.historyRepo.DeleteOlderThan(ctx, before); err != nil { | |
| 256 | 298 | return fmt.Errorf("failed to clear history: %w", err) | |
| 257 | 299 | } | |
| 300 | + | release() | |
| 258 | 301 | ||
| 302 | + | uc.publishHistoryChange(ctx, dto.HistoryChangeReasonClear, 0, false) | |
| 259 | 303 | log.Info().Time("before", before).Msg("old history cleared") | |
| 260 | 304 | return nil | |
| 261 | 305 | } | |
| @@ -272,9 +316,18 @@ func (uc *SearchHistoryUseCase) ClearRange(ctx context.Context, rangeID string) | |||
| 272 | 316 | ||
| 273 | 317 | log := logging.FromContext(ctx) | |
| 274 | 318 | log.Debug().Time("since", cutoff).Str("range", rangeID).Msg("clearing recent history range") | |
| 319 | + | release, err := uc.beginHistoryMutation(ctx) | |
| 320 | + | if err != nil { | |
| 321 | + | return fmt.Errorf("failed to prepare history mutation: %w", err) | |
| 322 | + | } | |
| 323 | + | release = onceRelease(release) | |
| 324 | + | defer release() | |
| 325 | + | ||
| 275 | 326 | if err := uc.historyRepo.DeleteSince(ctx, cutoff); err != nil { | |
| 276 | 327 | return fmt.Errorf("failed to clear history range: %w", err) | |
| 277 | 328 | } | |
| 329 | + | release() | |
| 330 | + | uc.publishHistoryChange(ctx, dto.HistoryChangeReasonClear, 0, false) | |
| 278 | 331 | log.Info().Time("since", cutoff).Str("range", rangeID).Msg("recent history range cleared") | |
| 279 | 332 | return nil | |
| 280 | 333 | } | |
| @@ -284,10 +337,19 @@ func (uc *SearchHistoryUseCase) ClearAll(ctx context.Context) error { | |||
| 284 | 337 | log := logging.FromContext(ctx) | |
| 285 | 338 | log.Debug().Msg("clearing all history") | |
| 286 | 339 | ||
| 340 | + | release, err := uc.beginHistoryMutation(ctx) | |
| 341 | + | if err != nil { | |
| 342 | + | return fmt.Errorf("failed to prepare history mutation: %w", err) | |
| 343 | + | } | |
| 344 | + | release = onceRelease(release) | |
| 345 | + | defer release() | |
| 346 | + | ||
| 287 | 347 | if err := uc.historyRepo.DeleteAll(ctx); err != nil { | |
| 288 | 348 | return fmt.Errorf("failed to clear all history: %w", err) | |
| 289 | 349 | } | |
| 350 | + | release() | |
| 290 | 351 | ||
| 352 | + | uc.publishHistoryChange(ctx, dto.HistoryChangeReasonClear, 0, false) | |
| 291 | 353 | log.Info().Msg("all history cleared") | |
| 292 | 354 | return nil | |
| 293 | 355 | } | |
| @@ -297,10 +359,19 @@ func (uc *SearchHistoryUseCase) Delete(ctx context.Context, id int64) error { | |||
| 297 | 359 | log := logging.FromContext(ctx) | |
| 298 | 360 | log.Debug().Int64("id", id).Msg("deleting history entry") | |
| 299 | 361 | ||
| 362 | + | release, err := uc.beginHistoryMutation(ctx) | |
| 363 | + | if err != nil { | |
| 364 | + | return fmt.Errorf("failed to prepare history mutation: %w", err) | |
| 365 | + | } | |
| 366 | + | release = onceRelease(release) | |
| 367 | + | defer release() | |
| 368 | + | ||
| 300 | 369 | if err := uc.historyRepo.Delete(ctx, id); err != nil { | |
| 301 | 370 | return fmt.Errorf("failed to delete history entry: %w", err) | |
| 302 | 371 | } | |
| 372 | + | release() | |
| 303 | 373 | ||
| 374 | + | uc.publishHistoryChange(ctx, dto.HistoryChangeReasonDelete, 1, true) | |
| 304 | 375 | log.Debug().Int64("id", id).Msg("history entry deleted") | |
| 305 | 376 | return nil | |
| 306 | 377 | } | |
| @@ -315,14 +386,73 @@ func (uc *SearchHistoryUseCase) DeleteByDomain(ctx context.Context, domain strin | |||
| 315 | 386 | log := logging.FromContext(ctx) | |
| 316 | 387 | log.Debug().Str("domain", domain).Msg("deleting history by domain") | |
| 317 | 388 | ||
| 389 | + | release, err := uc.beginHistoryMutation(ctx) | |
| 390 | + | if err != nil { | |
| 391 | + | return fmt.Errorf("failed to prepare history mutation: %w", err) | |
| 392 | + | } | |
| 393 | + | release = onceRelease(release) | |
| 394 | + | defer release() | |
| 395 | + | ||
| 318 | 396 | if err := uc.historyRepo.DeleteByDomain(ctx, domain); err != nil { | |
| 319 | 397 | return fmt.Errorf("failed to delete history by domain: %w", err) | |
| 320 | 398 | } | |
| 399 | + | release() | |
| 321 | 400 | ||
| 401 | + | uc.publishHistoryChange(ctx, dto.HistoryChangeReasonDelete, 0, false) | |
| 322 | 402 | log.Info().Str("domain", domain).Msg("history deleted for domain") | |
| 323 | 403 | return nil | |
| 324 | 404 | } | |
| 325 | 405 | ||
| 406 | + | func onceRelease(release func()) func() { | |
| 407 | + | var once sync.Once | |
| 408 | + | return func() { | |
| 409 | + | once.Do(release) | |
| 410 | + | } | |
| 411 | + | } | |
| 412 | + | ||
| 413 | + | func (uc *SearchHistoryUseCase) beginHistoryMutation(ctx context.Context) (func(), error) { | |
| 414 | + | uc.mutationCoordinatorMu.RLock() | |
| 415 | + | coordinator := uc.mutationCoordinator | |
| 416 | + | uc.mutationCoordinatorMu.RUnlock() | |
| 417 | + | if coordinator == nil { | |
| 418 | + | return func() {}, nil | |
| 419 | + | } | |
| 420 | + | release, err := coordinator.BeginHistoryMutation(ctx) | |
| 421 | + | if err != nil { | |
| 422 | + | if release != nil { | |
| 423 | + | release() | |
| 424 | + | } | |
| 425 | + | return nil, err | |
| 426 | + | } | |
| 427 | + | if release == nil { | |
| 428 | + | release = func() {} | |
| 429 | + | } | |
| 430 | + | return release, nil | |
| 431 | + | } | |
| 432 | + | ||
| 433 | + | func (uc *SearchHistoryUseCase) publishHistoryChange( | |
| 434 | + | ctx context.Context, | |
| 435 | + | reason dto.HistoryChangeReason, | |
| 436 | + | deleteCount int, | |
| 437 | + | deleteCountKnown bool, | |
| 438 | + | ) { | |
| 439 | + | change := dto.HistoryChange{ | |
| 440 | + | Reasons: []dto.HistoryChangeReason{reason}, | |
| 441 | + | DeleteCount: deleteCount, | |
| 442 | + | DeleteCountKnown: deleteCountKnown, | |
| 443 | + | ChangedAt: time.Now(), | |
| 444 | + | } | |
| 445 | + | sink := uc.historyChangeSink() | |
| 446 | + | sink.OnHistoryChanged(ctx, change) | |
| 447 | + | } | |
| 448 | + | ||
| 449 | + | func (uc *SearchHistoryUseCase) historyChangeSink() port.HistoryChangeSink { | |
| 450 | + | uc.changeSinkMu.RLock() | |
| 451 | + | sink := uc.changeSink | |
| 452 | + | uc.changeSinkMu.RUnlock() | |
| 453 | + | return normalizeHistoryChangeSink(sink) | |
| 454 | + | } | |
| 455 | + | ||
| 326 | 456 | // GetDomainStats retrieves per-domain visit statistics. | |
| 327 | 457 | func (uc *SearchHistoryUseCase) GetDomainStats(ctx context.Context, limit int) ([]*entity.DomainStat, error) { | |
| 328 | 458 | log := logging.FromContext(ctx) | |
M internal/application/usecase/search_history_test.go +467 -9
| @@ -1,10 +1,12 @@ | |||
| 1 | 1 | package usecase_test | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "context" | |
| 4 | 5 | "errors" | |
| 5 | 6 | "testing" | |
| 6 | 7 | "time" | |
| 7 | 8 | ||
| 9 | + | "github.com/bnema/dumber/internal/application/dto" | |
| 8 | 10 | "github.com/bnema/dumber/internal/application/usecase" | |
| 9 | 11 | "github.com/bnema/dumber/internal/domain/entity" | |
| 10 | 12 | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | |
| @@ -13,6 +15,50 @@ import ( | |||
| 13 | 15 | "github.com/stretchr/testify/require" | |
| 14 | 16 | ) | |
| 15 | 17 | ||
| 18 | + | type recordingHistoryChangeSink struct { | |
| 19 | + | changes []dto.HistoryChange | |
| 20 | + | onChange func(dto.HistoryChange) | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | func (s *recordingHistoryChangeSink) OnHistoryChanged(_ context.Context, change dto.HistoryChange) { | |
| 24 | + | if s.onChange != nil { | |
| 25 | + | s.onChange(change) | |
| 26 | + | } | |
| 27 | + | s.changes = append(s.changes, change) | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | type recordingHistoryMutationCoordinator struct { | |
| 31 | + | beginCount int | |
| 32 | + | releaseCount int | |
| 33 | + | err error | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | func (c *recordingHistoryMutationCoordinator) BeginHistoryMutation(context.Context) (func(), error) { | |
| 37 | + | c.beginCount++ | |
| 38 | + | if c.err != nil { | |
| 39 | + | return nil, c.err | |
| 40 | + | } | |
| 41 | + | return func() { c.releaseCount++ }, nil | |
| 42 | + | } | |
| 43 | + | ||
| 44 | + | type nilPanicHistoryMutationCoordinator struct { | |
| 45 | + | called bool | |
| 46 | + | } | |
| 47 | + | ||
| 48 | + | func (c *nilPanicHistoryMutationCoordinator) BeginHistoryMutation(context.Context) (func(), error) { | |
| 49 | + | c.called = true | |
| 50 | + | return func() {}, nil | |
| 51 | + | } | |
| 52 | + | ||
| 53 | + | func requirePublishedHistoryChange(t *testing.T, sink *recordingHistoryChangeSink, reason dto.HistoryChangeReason) dto.HistoryChange { | |
| 54 | + | t.Helper() | |
| 55 | + | require.Len(t, sink.changes, 1) | |
| 56 | + | change := sink.changes[0] | |
| 57 | + | assert.Equal(t, []dto.HistoryChangeReason{reason}, change.Reasons) | |
| 58 | + | assert.False(t, change.ChangedAt.IsZero()) | |
| 59 | + | return change | |
| 60 | + | } | |
| 61 | + | ||
| 16 | 62 | func TestSearchHistoryUseCase_GetRecent_ZeroLimitMeansAll(t *testing.T) { | |
| 17 | 63 | ctx := testContext() | |
| 18 | 64 | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| @@ -269,7 +315,388 @@ func TestSearchHistoryUseCase_GetDomainStats_ClampsOversizedLimit(t *testing.T) | |||
| 269 | 315 | assert.Empty(t, result) | |
| 270 | 316 | } | |
| 271 | 317 | ||
| 272 | - | func TestSearchHistoryUseCase_ClearRange_RecentRangeDeletesSinceCutoff(t *testing.T) { | |
| 318 | + | func TestSearchHistoryUseCase_Delete_PublishesDeleteChangeAfterSuccess(t *testing.T) { | |
| 319 | + | ctx := testContext() | |
| 320 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 321 | + | historyRepo.EXPECT().Delete(mock.Anything, int64(42)).Return(nil).Once() | |
| 322 | + | sink := &recordingHistoryChangeSink{} | |
| 323 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 324 | + | ||
| 325 | + | err := uc.Delete(ctx, 42) | |
| 326 | + | ||
| 327 | + | require.NoError(t, err) | |
| 328 | + | change := requirePublishedHistoryChange(t, sink, dto.HistoryChangeReasonDelete) | |
| 329 | + | assert.Equal(t, 1, change.DeleteCount) | |
| 330 | + | assert.True(t, change.DeleteCountKnown) | |
| 331 | + | } | |
| 332 | + | ||
| 333 | + | func TestSearchHistoryUseCase_Delete_DoesNotPublishOnRepositoryError(t *testing.T) { | |
| 334 | + | ctx := testContext() | |
| 335 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 336 | + | historyRepo.EXPECT().Delete(mock.Anything, int64(42)).Return(errors.New("delete failed")).Once() | |
| 337 | + | sink := &recordingHistoryChangeSink{} | |
| 338 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 339 | + | ||
| 340 | + | err := uc.Delete(ctx, 42) | |
| 341 | + | ||
| 342 | + | require.Error(t, err) | |
| 343 | + | assert.Empty(t, sink.changes) | |
| 344 | + | } | |
| 345 | + | ||
| 346 | + | func TestSearchHistoryUseCase_DeleteByDomain_PublishesDeleteChangeAfterSuccess(t *testing.T) { | |
| 347 | + | ctx := testContext() | |
| 348 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 349 | + | historyRepo.EXPECT().DeleteByDomain(mock.Anything, "example.com").Return(nil).Once() | |
| 350 | + | sink := &recordingHistoryChangeSink{} | |
| 351 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 352 | + | ||
| 353 | + | err := uc.DeleteByDomain(ctx, "https://example.com/path") | |
| 354 | + | ||
| 355 | + | require.NoError(t, err) | |
| 356 | + | change := requirePublishedHistoryChange(t, sink, dto.HistoryChangeReasonDelete) | |
| 357 | + | assert.Zero(t, change.DeleteCount, "domain delete count is unknown for aggregate deletes") | |
| 358 | + | assert.False(t, change.DeleteCountKnown) | |
| 359 | + | } | |
| 360 | + | ||
| 361 | + | func TestSearchHistoryUseCase_DeleteByDomain_DoesNotPublishOnRepositoryError(t *testing.T) { | |
| 362 | + | ctx := testContext() | |
| 363 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 364 | + | historyRepo.EXPECT().DeleteByDomain(mock.Anything, "example.com").Return(errors.New("delete domain failed")).Once() | |
| 365 | + | sink := &recordingHistoryChangeSink{} | |
| 366 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 367 | + | ||
| 368 | + | err := uc.DeleteByDomain(ctx, "example.com") | |
| 369 | + | ||
| 370 | + | require.Error(t, err) | |
| 371 | + | assert.Empty(t, sink.changes) | |
| 372 | + | } | |
| 373 | + | ||
| 374 | + | func TestSearchHistoryUseCase_SetHistoryMutationCoordinator_TypedNilClearsCoordinator(t *testing.T) { | |
| 375 | + | ctx := testContext() | |
| 376 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 377 | + | historyRepo.EXPECT().DeleteAll(mock.Anything).Return(nil).Once() | |
| 378 | + | previous := &recordingHistoryMutationCoordinator{} | |
| 379 | + | var coordinator *nilPanicHistoryMutationCoordinator | |
| 380 | + | sink := &recordingHistoryChangeSink{} | |
| 381 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 382 | + | uc.SetHistoryMutationCoordinator(previous) | |
| 383 | + | uc.SetHistoryMutationCoordinator(coordinator) | |
| 384 | + | ||
| 385 | + | require.NotPanics(t, func() { | |
| 386 | + | require.NoError(t, uc.ClearAll(ctx)) | |
| 387 | + | }) | |
| 388 | + | require.Zero(t, previous.beginCount) | |
| 389 | + | requirePublishedHistoryChange(t, sink, dto.HistoryChangeReasonClear) | |
| 390 | + | } | |
| 391 | + | ||
| 392 | + | func TestSearchHistoryUseCase_ClearAll_PublishesClearChangeAfterSuccess(t *testing.T) { | |
| 393 | + | ctx := testContext() | |
| 394 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 395 | + | historyRepo.EXPECT().DeleteAll(mock.Anything).Return(nil).Once() | |
| 396 | + | sink := &recordingHistoryChangeSink{} | |
| 397 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 398 | + | ||
| 399 | + | err := uc.ClearAll(ctx) | |
| 400 | + | ||
| 401 | + | require.NoError(t, err) | |
| 402 | + | change := requirePublishedHistoryChange(t, sink, dto.HistoryChangeReasonClear) | |
| 403 | + | assert.Zero(t, change.DeleteCount) | |
| 404 | + | assert.False(t, change.DeleteCountKnown) | |
| 405 | + | } | |
| 406 | + | ||
| 407 | + | func TestSearchHistoryUseCase_ClearAll_CoordinatesPendingRecorderWritesBeforeDelete(t *testing.T) { | |
| 408 | + | ctx := testContext() | |
| 409 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 410 | + | coordinator := &recordingHistoryMutationCoordinator{} | |
| 411 | + | historyRepo.EXPECT().DeleteAll(mock.Anything).RunAndReturn(func(context.Context) error { | |
| 412 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 413 | + | require.Zero(t, coordinator.releaseCount) | |
| 414 | + | return nil | |
| 415 | + | }).Once() | |
| 416 | + | sink := &recordingHistoryChangeSink{onChange: func(dto.HistoryChange) { | |
| 417 | + | require.Equal(t, 1, coordinator.releaseCount, "history change should publish after mutation coordinator release") | |
| 418 | + | }} | |
| 419 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 420 | + | uc.SetHistoryMutationCoordinator(coordinator) | |
| 421 | + | ||
| 422 | + | err := uc.ClearAll(ctx) | |
| 423 | + | ||
| 424 | + | require.NoError(t, err) | |
| 425 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 426 | + | require.Equal(t, 1, coordinator.releaseCount) | |
| 427 | + | requirePublishedHistoryChange(t, sink, dto.HistoryChangeReasonClear) | |
| 428 | + | } | |
| 429 | + | ||
| 430 | + | func TestSearchHistoryUseCase_OtherDestructiveMutationsCoordinateBeforeDeleteAndPublish(t *testing.T) { | |
| 431 | + | ctx := testContext() | |
| 432 | + | before := time.Now().Add(-7 * 24 * time.Hour) | |
| 433 | + | ||
| 434 | + | tests := []struct { | |
| 435 | + | name string | |
| 436 | + | expectRepo func(*repomocks.MockHistoryRepository, *recordingHistoryMutationCoordinator) | |
| 437 | + | run func(*usecase.SearchHistoryUseCase) error | |
| 438 | + | reason dto.HistoryChangeReason | |
| 439 | + | deleteCount int | |
| 440 | + | deleteCountKnown bool | |
| 441 | + | }{ | |
| 442 | + | { | |
| 443 | + | name: "delete", | |
| 444 | + | expectRepo: func(repo *repomocks.MockHistoryRepository, coordinator *recordingHistoryMutationCoordinator) { | |
| 445 | + | repo.EXPECT().Delete(mock.Anything, int64(42)).RunAndReturn(func(context.Context, int64) error { | |
| 446 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 447 | + | require.Zero(t, coordinator.releaseCount) | |
| 448 | + | return nil | |
| 449 | + | }).Once() | |
| 450 | + | }, | |
| 451 | + | run: func(uc *usecase.SearchHistoryUseCase) error { | |
| 452 | + | return uc.Delete(ctx, 42) | |
| 453 | + | }, | |
| 454 | + | reason: dto.HistoryChangeReasonDelete, | |
| 455 | + | deleteCount: 1, | |
| 456 | + | deleteCountKnown: true, | |
| 457 | + | }, | |
| 458 | + | { | |
| 459 | + | name: "delete by domain", | |
| 460 | + | expectRepo: func(repo *repomocks.MockHistoryRepository, coordinator *recordingHistoryMutationCoordinator) { | |
| 461 | + | repo.EXPECT().DeleteByDomain(mock.Anything, "example.com").RunAndReturn(func(context.Context, string) error { | |
| 462 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 463 | + | require.Zero(t, coordinator.releaseCount) | |
| 464 | + | return nil | |
| 465 | + | }).Once() | |
| 466 | + | }, | |
| 467 | + | run: func(uc *usecase.SearchHistoryUseCase) error { | |
| 468 | + | return uc.DeleteByDomain(ctx, "https://example.com/path") | |
| 469 | + | }, | |
| 470 | + | reason: dto.HistoryChangeReasonDelete, | |
| 471 | + | }, | |
| 472 | + | { | |
| 473 | + | name: "clear older than", | |
| 474 | + | expectRepo: func(repo *repomocks.MockHistoryRepository, coordinator *recordingHistoryMutationCoordinator) { | |
| 475 | + | repo.EXPECT().DeleteOlderThan(mock.Anything, before).RunAndReturn(func(context.Context, time.Time) error { | |
| 476 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 477 | + | require.Zero(t, coordinator.releaseCount) | |
| 478 | + | return nil | |
| 479 | + | }).Once() | |
| 480 | + | }, | |
| 481 | + | run: func(uc *usecase.SearchHistoryUseCase) error { | |
| 482 | + | return uc.ClearOlderThan(ctx, before) | |
| 483 | + | }, | |
| 484 | + | reason: dto.HistoryChangeReasonClear, | |
| 485 | + | }, | |
| 486 | + | { | |
| 487 | + | name: "clear range", | |
| 488 | + | expectRepo: func(repo *repomocks.MockHistoryRepository, coordinator *recordingHistoryMutationCoordinator) { | |
| 489 | + | repo.EXPECT().DeleteSince(mock.Anything, mock.AnythingOfType("time.Time")).RunAndReturn(func(context.Context, time.Time) error { | |
| 490 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 491 | + | require.Zero(t, coordinator.releaseCount) | |
| 492 | + | return nil | |
| 493 | + | }).Once() | |
| 494 | + | }, | |
| 495 | + | run: func(uc *usecase.SearchHistoryUseCase) error { | |
| 496 | + | return uc.ClearRange(ctx, "hour") | |
| 497 | + | }, | |
| 498 | + | reason: dto.HistoryChangeReasonClear, | |
| 499 | + | }, | |
| 500 | + | } | |
| 501 | + | ||
| 502 | + | for _, tt := range tests { | |
| 503 | + | t.Run(tt.name, func(t *testing.T) { | |
| 504 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 505 | + | coordinator := &recordingHistoryMutationCoordinator{} | |
| 506 | + | tt.expectRepo(historyRepo, coordinator) | |
| 507 | + | sink := &recordingHistoryChangeSink{onChange: func(dto.HistoryChange) { | |
| 508 | + | require.Equal(t, 1, coordinator.releaseCount, "history change should publish after mutation coordinator release") | |
| 509 | + | }} | |
| 510 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 511 | + | uc.SetHistoryMutationCoordinator(coordinator) | |
| 512 | + | ||
| 513 | + | err := tt.run(uc) | |
| 514 | + | ||
| 515 | + | require.NoError(t, err) | |
| 516 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 517 | + | require.Equal(t, 1, coordinator.releaseCount) | |
| 518 | + | change := requirePublishedHistoryChange(t, sink, tt.reason) | |
| 519 | + | require.Equal(t, tt.deleteCount, change.DeleteCount) | |
| 520 | + | require.Equal(t, tt.deleteCountKnown, change.DeleteCountKnown) | |
| 521 | + | }) | |
| 522 | + | } | |
| 523 | + | } | |
| 524 | + | ||
| 525 | + | func TestSearchHistoryUseCase_DestructiveMutationsReleaseCoordinatorOnRepositoryError(t *testing.T) { | |
| 526 | + | ctx := testContext() | |
| 527 | + | before := time.Now().Add(-7 * 24 * time.Hour) | |
| 528 | + | repoErr := errors.New("repository failed") | |
| 529 | + | ||
| 530 | + | tests := []struct { | |
| 531 | + | name string | |
| 532 | + | expectRepo func(*repomocks.MockHistoryRepository, *recordingHistoryMutationCoordinator) | |
| 533 | + | run func(*usecase.SearchHistoryUseCase) error | |
| 534 | + | }{ | |
| 535 | + | { | |
| 536 | + | name: "delete", | |
| 537 | + | expectRepo: func(repo *repomocks.MockHistoryRepository, coordinator *recordingHistoryMutationCoordinator) { | |
| 538 | + | repo.EXPECT().Delete(mock.Anything, int64(42)).RunAndReturn(func(context.Context, int64) error { | |
| 539 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 540 | + | require.Zero(t, coordinator.releaseCount) | |
| 541 | + | return repoErr | |
| 542 | + | }).Once() | |
| 543 | + | }, | |
| 544 | + | run: func(uc *usecase.SearchHistoryUseCase) error { return uc.Delete(ctx, 42) }, | |
| 545 | + | }, | |
| 546 | + | { | |
| 547 | + | name: "delete by domain", | |
| 548 | + | expectRepo: func(repo *repomocks.MockHistoryRepository, coordinator *recordingHistoryMutationCoordinator) { | |
| 549 | + | repo.EXPECT().DeleteByDomain(mock.Anything, "example.com").RunAndReturn(func(context.Context, string) error { | |
| 550 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 551 | + | require.Zero(t, coordinator.releaseCount) | |
| 552 | + | return repoErr | |
| 553 | + | }).Once() | |
| 554 | + | }, | |
| 555 | + | run: func(uc *usecase.SearchHistoryUseCase) error { return uc.DeleteByDomain(ctx, "example.com") }, | |
| 556 | + | }, | |
| 557 | + | { | |
| 558 | + | name: "clear older than", | |
| 559 | + | expectRepo: func(repo *repomocks.MockHistoryRepository, coordinator *recordingHistoryMutationCoordinator) { | |
| 560 | + | repo.EXPECT().DeleteOlderThan(mock.Anything, before).RunAndReturn(func(context.Context, time.Time) error { | |
| 561 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 562 | + | require.Zero(t, coordinator.releaseCount) | |
| 563 | + | return repoErr | |
| 564 | + | }).Once() | |
| 565 | + | }, | |
| 566 | + | run: func(uc *usecase.SearchHistoryUseCase) error { return uc.ClearOlderThan(ctx, before) }, | |
| 567 | + | }, | |
| 568 | + | { | |
| 569 | + | name: "clear range", | |
| 570 | + | expectRepo: func(repo *repomocks.MockHistoryRepository, coordinator *recordingHistoryMutationCoordinator) { | |
| 571 | + | repo.EXPECT().DeleteSince(mock.Anything, mock.AnythingOfType("time.Time")).RunAndReturn(func(context.Context, time.Time) error { | |
| 572 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 573 | + | require.Zero(t, coordinator.releaseCount) | |
| 574 | + | return repoErr | |
| 575 | + | }).Once() | |
| 576 | + | }, | |
| 577 | + | run: func(uc *usecase.SearchHistoryUseCase) error { return uc.ClearRange(ctx, "hour") }, | |
| 578 | + | }, | |
| 579 | + | { | |
| 580 | + | name: "clear all", | |
| 581 | + | expectRepo: func(repo *repomocks.MockHistoryRepository, coordinator *recordingHistoryMutationCoordinator) { | |
| 582 | + | repo.EXPECT().DeleteAll(mock.Anything).RunAndReturn(func(context.Context) error { | |
| 583 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 584 | + | require.Zero(t, coordinator.releaseCount) | |
| 585 | + | return repoErr | |
| 586 | + | }).Once() | |
| 587 | + | }, | |
| 588 | + | run: func(uc *usecase.SearchHistoryUseCase) error { return uc.ClearAll(ctx) }, | |
| 589 | + | }, | |
| 590 | + | } | |
| 591 | + | ||
| 592 | + | for _, tt := range tests { | |
| 593 | + | t.Run(tt.name, func(t *testing.T) { | |
| 594 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 595 | + | coordinator := &recordingHistoryMutationCoordinator{} | |
| 596 | + | tt.expectRepo(historyRepo, coordinator) | |
| 597 | + | sink := &recordingHistoryChangeSink{} | |
| 598 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 599 | + | uc.SetHistoryMutationCoordinator(coordinator) | |
| 600 | + | ||
| 601 | + | err := tt.run(uc) | |
| 602 | + | ||
| 603 | + | require.Error(t, err) | |
| 604 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 605 | + | require.Equal(t, 1, coordinator.releaseCount) | |
| 606 | + | assert.Empty(t, sink.changes) | |
| 607 | + | }) | |
| 608 | + | } | |
| 609 | + | } | |
| 610 | + | ||
| 611 | + | func TestSearchHistoryUseCase_OtherDestructiveMutationsReturnCoordinatorErrorBeforeDeleting(t *testing.T) { | |
| 612 | + | ctx := testContext() | |
| 613 | + | before := time.Now().Add(-7 * 24 * time.Hour) | |
| 614 | + | tests := []struct { | |
| 615 | + | name string | |
| 616 | + | run func(*usecase.SearchHistoryUseCase) error | |
| 617 | + | }{ | |
| 618 | + | {name: "delete", run: func(uc *usecase.SearchHistoryUseCase) error { return uc.Delete(ctx, 42) }}, | |
| 619 | + | {name: "delete by domain", run: func(uc *usecase.SearchHistoryUseCase) error { return uc.DeleteByDomain(ctx, "example.com") }}, | |
| 620 | + | {name: "clear older than", run: func(uc *usecase.SearchHistoryUseCase) error { return uc.ClearOlderThan(ctx, before) }}, | |
| 621 | + | {name: "clear range", run: func(uc *usecase.SearchHistoryUseCase) error { return uc.ClearRange(ctx, "hour") }}, | |
| 622 | + | } | |
| 623 | + | ||
| 624 | + | for _, tt := range tests { | |
| 625 | + | t.Run(tt.name, func(t *testing.T) { | |
| 626 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 627 | + | coordinator := &recordingHistoryMutationCoordinator{err: errors.New("flush failed")} | |
| 628 | + | sink := &recordingHistoryChangeSink{} | |
| 629 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 630 | + | uc.SetHistoryMutationCoordinator(coordinator) | |
| 631 | + | ||
| 632 | + | err := tt.run(uc) | |
| 633 | + | ||
| 634 | + | require.Error(t, err) | |
| 635 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 636 | + | require.Zero(t, coordinator.releaseCount) | |
| 637 | + | assert.Empty(t, sink.changes) | |
| 638 | + | }) | |
| 639 | + | } | |
| 640 | + | } | |
| 641 | + | ||
| 642 | + | func TestSearchHistoryUseCase_ClearAll_ReturnsCoordinatorErrorBeforeDeleting(t *testing.T) { | |
| 643 | + | ctx := testContext() | |
| 644 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 645 | + | coordinator := &recordingHistoryMutationCoordinator{err: errors.New("flush failed")} | |
| 646 | + | sink := &recordingHistoryChangeSink{} | |
| 647 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 648 | + | uc.SetHistoryMutationCoordinator(coordinator) | |
| 649 | + | ||
| 650 | + | err := uc.ClearAll(ctx) | |
| 651 | + | ||
| 652 | + | require.Error(t, err) | |
| 653 | + | require.Equal(t, 1, coordinator.beginCount) | |
| 654 | + | require.Zero(t, coordinator.releaseCount) | |
| 655 | + | assert.Empty(t, sink.changes) | |
| 656 | + | } | |
| 657 | + | ||
| 658 | + | func TestSearchHistoryUseCase_ClearAll_DoesNotPublishOnRepositoryError(t *testing.T) { | |
| 659 | + | ctx := testContext() | |
| 660 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 661 | + | historyRepo.EXPECT().DeleteAll(mock.Anything).Return(errors.New("clear all failed")).Once() | |
| 662 | + | sink := &recordingHistoryChangeSink{} | |
| 663 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 664 | + | ||
| 665 | + | err := uc.ClearAll(ctx) | |
| 666 | + | ||
| 667 | + | require.Error(t, err) | |
| 668 | + | assert.Empty(t, sink.changes) | |
| 669 | + | } | |
| 670 | + | ||
| 671 | + | func TestSearchHistoryUseCase_ClearOlderThan_PublishesClearChangeAfterSuccess(t *testing.T) { | |
| 672 | + | ctx := testContext() | |
| 673 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 674 | + | before := time.Now().Add(-7 * 24 * time.Hour) | |
| 675 | + | historyRepo.EXPECT().DeleteOlderThan(mock.Anything, before).Return(nil).Once() | |
| 676 | + | sink := &recordingHistoryChangeSink{} | |
| 677 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 678 | + | ||
| 679 | + | err := uc.ClearOlderThan(ctx, before) | |
| 680 | + | ||
| 681 | + | require.NoError(t, err) | |
| 682 | + | requirePublishedHistoryChange(t, sink, dto.HistoryChangeReasonClear) | |
| 683 | + | } | |
| 684 | + | ||
| 685 | + | func TestSearchHistoryUseCase_ClearOlderThan_DoesNotPublishOnRepositoryError(t *testing.T) { | |
| 686 | + | ctx := testContext() | |
| 687 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 688 | + | before := time.Now().Add(-7 * 24 * time.Hour) | |
| 689 | + | historyRepo.EXPECT().DeleteOlderThan(mock.Anything, before).Return(errors.New("clear old failed")).Once() | |
| 690 | + | sink := &recordingHistoryChangeSink{} | |
| 691 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 692 | + | ||
| 693 | + | err := uc.ClearOlderThan(ctx, before) | |
| 694 | + | ||
| 695 | + | require.Error(t, err) | |
| 696 | + | assert.Empty(t, sink.changes) | |
| 697 | + | } | |
| 698 | + | ||
| 699 | + | func TestSearchHistoryUseCase_ClearRange_RecentRangeDeletesSinceCutoffAndPublishesClearChange(t *testing.T) { | |
| 273 | 700 | ctx := testContext() | |
| 274 | 701 | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 275 | 702 | now := time.Now() | |
| @@ -277,11 +704,26 @@ func TestSearchHistoryUseCase_ClearRange_RecentRangeDeletesSinceCutoff(t *testin | |||
| 277 | 704 | historyRepo.EXPECT().DeleteSince(mock.Anything, mock.MatchedBy(func(since time.Time) bool { | |
| 278 | 705 | return since.After(expectedCutoff.Add(-time.Minute)) && since.Before(expectedCutoff.Add(time.Minute)) | |
| 279 | 706 | })).Return(nil).Once() | |
| 280 | - | ||
| 281 | - | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |
| 707 | + | sink := &recordingHistoryChangeSink{} | |
| 708 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 282 | 709 | ||
| 283 | 710 | err := uc.ClearRange(ctx, "hour") | |
| 711 | + | ||
| 284 | 712 | require.NoError(t, err) | |
| 713 | + | requirePublishedHistoryChange(t, sink, dto.HistoryChangeReasonClear) | |
| 714 | + | } | |
| 715 | + | ||
| 716 | + | func TestSearchHistoryUseCase_ClearRange_RecentRangeDoesNotPublishOnRepositoryError(t *testing.T) { | |
| 717 | + | ctx := testContext() | |
| 718 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 719 | + | historyRepo.EXPECT().DeleteSince(mock.Anything, mock.AnythingOfType("time.Time")).Return(errors.New("delete since failed")).Once() | |
| 720 | + | sink := &recordingHistoryChangeSink{} | |
| 721 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 722 | + | ||
| 723 | + | err := uc.ClearRange(ctx, "hour") | |
| 724 | + | ||
| 725 | + | require.Error(t, err) | |
| 726 | + | assert.Empty(t, sink.changes) | |
| 285 | 727 | } | |
| 286 | 728 | ||
| 287 | 729 | func TestSearchHistoryUseCase_ClearRange_TodayDeletesSinceLocalMidnight(t *testing.T) { | |
| @@ -299,29 +741,45 @@ func TestSearchHistoryUseCase_ClearRange_TodayDeletesSinceLocalMidnight(t *testi | |||
| 299 | 741 | require.NoError(t, err) | |
| 300 | 742 | } | |
| 301 | 743 | ||
| 302 | - | func TestSearchHistoryUseCase_ClearRange_AllUsesClearAll(t *testing.T) { | |
| 744 | + | func TestSearchHistoryUseCase_ClearRange_AllUsesClearAllAndPublishesOnce(t *testing.T) { | |
| 303 | 745 | ctx := testContext() | |
| 304 | 746 | ||
| 305 | 747 | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 306 | 748 | ||
| 307 | - | historyRepo.EXPECT().DeleteAll(mock.Anything).Return(nil) | |
| 308 | - | ||
| 309 | - | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |
| 749 | + | historyRepo.EXPECT().DeleteAll(mock.Anything).Return(nil).Once() | |
| 750 | + | sink := &recordingHistoryChangeSink{} | |
| 751 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 310 | 752 | ||
| 311 | 753 | err := uc.ClearRange(ctx, "all") | |
| 312 | 754 | require.NoError(t, err) | |
| 755 | + | requirePublishedHistoryChange(t, sink, dto.HistoryChangeReasonClear) | |
| 313 | 756 | } | |
| 314 | 757 | ||
| 315 | - | func TestSearchHistoryUseCase_ClearRange_UnknownRangeDoesNotDelete(t *testing.T) { | |
| 758 | + | func TestSearchHistoryUseCase_ClearRange_AllDoesNotPublishOnRepositoryError(t *testing.T) { | |
| 316 | 759 | ctx := testContext() | |
| 317 | 760 | ||
| 318 | 761 | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 319 | 762 | ||
| 320 | - | uc := usecase.NewSearchHistoryUseCase(historyRepo) | |
| 763 | + | historyRepo.EXPECT().DeleteAll(mock.Anything).Return(errors.New("clear all failed")).Once() | |
| 764 | + | sink := &recordingHistoryChangeSink{} | |
| 765 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 766 | + | ||
| 767 | + | err := uc.ClearRange(ctx, "all") | |
| 768 | + | require.Error(t, err) | |
| 769 | + | assert.Empty(t, sink.changes) | |
| 770 | + | } | |
| 771 | + | ||
| 772 | + | func TestSearchHistoryUseCase_ClearRange_UnknownRangeDoesNotDeleteOrPublish(t *testing.T) { | |
| 773 | + | ctx := testContext() | |
| 774 | + | ||
| 775 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 776 | + | sink := &recordingHistoryChangeSink{} | |
| 777 | + | uc := usecase.NewSearchHistoryUseCase(historyRepo, sink) | |
| 321 | 778 | ||
| 322 | 779 | err := uc.ClearRange(ctx, "bogus") | |
| 323 | 780 | require.Error(t, err) | |
| 324 | 781 | assert.Contains(t, err.Error(), "unknown history delete range") | |
| 782 | + | assert.Empty(t, sink.changes) | |
| 325 | 783 | } | |
| 326 | 784 | ||
| 327 | 785 | func TestSearchHistoryUseCase_GetRecentByDomainAllowsUnderscoreDomains(t *testing.T) { | |
M internal/ui/app.go +22 -1
| @@ -220,6 +220,14 @@ func New(deps *Dependencies) (*App, error) { | |||
| 220 | 220 | glib.IdleAdd(&cb, 0) | |
| 221 | 221 | }) | |
| 222 | 222 | } | |
| 223 | + | historyChangeSink := newHistoryChangeAdapter(app) | |
| 224 | + | if deps.HistoryRecorderUC != nil { | |
| 225 | + | deps.HistoryRecorderUC.SetHistoryChangeSink(historyChangeSink) | |
| 226 | + | } | |
| 227 | + | if deps.HistoryUC != nil { | |
| 228 | + | deps.HistoryUC.SetHistoryChangeSink(historyChangeSink) | |
| 229 | + | } | |
| 230 | + | ||
| 223 | 231 | if faviconSetter, ok := deps.Engine.(port.SystemviewFaviconResolverSetter); ok { | |
| 224 | 232 | resolver := deps.SystemviewFaviconResolver | |
| 225 | 233 | if resolver == nil { | |
| @@ -249,6 +257,10 @@ func New(deps *Dependencies) (*App, error) { | |||
| 249 | 257 | }, | |
| 250 | 258 | HandlerDeps: deps.HandlerDeps, | |
| 251 | 259 | }); err != nil { | |
| 260 | + | cancel(err) | |
| 261 | + | if deps.HistoryRecorderUC != nil { | |
| 262 | + | deps.HistoryRecorderUC.Close() | |
| 263 | + | } | |
| 252 | 264 | return nil, err | |
| 253 | 265 | } | |
| 254 | 266 | ||
| @@ -3145,6 +3157,7 @@ func (a *App) onShutdown(ctx context.Context) { | |||
| 3145 | 3157 | a.cancel(errors.New("application shutdown")) | |
| 3146 | 3158 | ||
| 3147 | 3159 | // Cleanup resources | |
| 3160 | + | a.closeHistoryRecorder() | |
| 3148 | 3161 | if a.faviconAdapter != nil { | |
| 3149 | 3162 | a.faviconAdapter.Close() | |
| 3150 | 3163 | } | |
| @@ -3183,6 +3196,13 @@ func (a *App) onShutdown(ctx context.Context) { | |||
| 3183 | 3196 | log.Info().Msg("application shutdown complete") | |
| 3184 | 3197 | } | |
| 3185 | 3198 | ||
| 3199 | + | func (a *App) closeHistoryRecorder() { | |
| 3200 | + | if a.deps == nil || a.deps.HistoryRecorderUC == nil { | |
| 3201 | + | return | |
| 3202 | + | } | |
| 3203 | + | a.deps.HistoryRecorderUC.Close() | |
| 3204 | + | } | |
| 3205 | + | ||
| 3186 | 3206 | // initContentCoordinator creates the content coordinator and wires its optional dependencies. | |
| 3187 | 3207 | func (a *App) initContentCoordinator( | |
| 3188 | 3208 | ctx context.Context, | |
| @@ -3396,9 +3416,10 @@ func (a *App) initCoordinators(ctx context.Context) { | |||
| 3396 | 3416 | a.extractPaneToTabListUC = usecase.NewExtractPaneToTabListUseCase(a.generateID) | |
| 3397 | 3417 | ||
| 3398 | 3418 | // 4. Navigation Coordinator | |
| 3399 | - | a.navCoord = coordinator.NewNavigationCoordinator( | |
| 3419 | + | a.navCoord = coordinator.NewNavigationCoordinatorWithHistoryRecorder( | |
| 3400 | 3420 | ctx, | |
| 3401 | 3421 | a.deps.NavigateUC, | |
| 3422 | + | a.deps.HistoryRecorderUC, | |
| 3402 | 3423 | a.contentCoord, | |
| 3403 | 3424 | ) | |
| 3404 | 3425 | a.wsCoord.SetOnPaneClosed(func(paneID entity.PaneID) { | |
M internal/ui/app_clipboard_test.go +32 -0
| @@ -2,12 +2,15 @@ package ui | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "errors" | |
| 5 | 6 | "testing" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/dumber/internal/application/dto" | |
| 8 | 9 | "github.com/bnema/dumber/internal/application/port" | |
| 9 | 10 | portmocks "github.com/bnema/dumber/internal/application/port/mocks" | |
| 11 | + | "github.com/bnema/dumber/internal/application/usecase" | |
| 10 | 12 | "github.com/bnema/dumber/internal/domain/entity" | |
| 13 | + | repomocks "github.com/bnema/dumber/internal/domain/repository/mocks" | |
| 11 | 14 | "github.com/stretchr/testify/mock" | |
| 12 | 15 | "github.com/stretchr/testify/require" | |
| 13 | 16 | ) | |
| @@ -45,6 +48,35 @@ func TestNew_DoesNotWireClipboardOrchestratorWhenClipboardMissing(t *testing.T) | |||
| 45 | 48 | require.Nil(t, registerDeps.ClipboardTextOrchestrator) | |
| 46 | 49 | } | |
| 47 | 50 | ||
| 51 | + | func TestNew_ClosesHistoryRecorderWhenRegisterHandlersFails(t *testing.T) { | |
| 52 | + | ctx := context.Background() | |
| 53 | + | engine := portmocks.NewMockEngine(t) | |
| 54 | + | registerErr := errors.New("register failed") | |
| 55 | + | engine.EXPECT().RegisterHandlers(mock.Anything, mock.Anything).Return(registerErr).Once() | |
| 56 | + | runtimeConfig := portmocks.NewMockRuntimeConfigProvider(t) | |
| 57 | + | runtimeConfig.EXPECT().Current().Return(entity.RuntimeConfigSnapshot{}).Once() | |
| 58 | + | historyRepo := repomocks.NewMockHistoryRepository(t) | |
| 59 | + | historyRecorder := usecase.NewHistoryRecorderUseCase(historyRepo, nil) | |
| 60 | + | ||
| 61 | + | deps := &Dependencies{ | |
| 62 | + | Ctx: ctx, | |
| 63 | + | RuntimeConfig: runtimeConfig, | |
| 64 | + | Engine: engine, | |
| 65 | + | HistoryRecorderUC: historyRecorder, | |
| 66 | + | HandlerDeps: port.HandlerDeps{ | |
| 67 | + | SaveConfig: func(context.Context, dto.WebUIConfig) error { return nil }, | |
| 68 | + | SaveOmniboxInitialBehavior: func(context.Context, entity.OmniboxInitialBehavior) error { return nil }, | |
| 69 | + | }, | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | app, err := New(deps) | |
| 73 | + | require.Nil(t, app) | |
| 74 | + | require.ErrorIs(t, err, registerErr) | |
| 75 | + | ||
| 76 | + | historyRecorder.RecordHistory(ctx, "pane-1", "https://example.com/leak-check") | |
| 77 | + | historyRecorder.Close() | |
| 78 | + | } | |
| 79 | + | ||
| 48 | 80 | func TestNew_WiresClipboardOrchestratorWhenClipboardPresent(t *testing.T) { | |
| 49 | 81 | ctx := context.Background() | |
| 50 | 82 | engine := portmocks.NewMockEngine(t) | |
M internal/ui/browser_window.go +22 -20
| @@ -20,26 +20,27 @@ import ( | |||
| 20 | 20 | ) | |
| 21 | 21 | ||
| 22 | 22 | type browserWindow struct { | |
| 23 | - | id string | |
| 24 | - | initialURL string | |
| 25 | - | tabs *entity.TabList // per-window tab list (single source of truth for tab state) | |
| 26 | - | mainWindow *window.MainWindow | |
| 27 | - | appToaster *component.Toaster | |
| 28 | - | modeToaster *component.Toaster | |
| 29 | - | touchpadNavIndicator *component.TouchpadNavigationIndicator | |
| 30 | - | borderMgr *focus.BorderManager | |
| 31 | - | sessionManager *component.SessionManager | |
| 32 | - | tabPicker *component.TabPicker | |
| 33 | - | tabPickerWidget layout.Widget | |
| 34 | - | tabPickerPaneID entity.PaneID | |
| 35 | - | insertAccentUC *usecase.InsertAccentUseCase | |
| 36 | - | accentPicker *component.AccentPicker | |
| 37 | - | keyboardHandler *input.KeyboardHandler | |
| 38 | - | globalShortcutHandler *input.GlobalShortcutHandler | |
| 39 | - | permissionDialog port.PermissionDialogPresenter | |
| 40 | - | webrtcIndicator *component.WebRTCPermissionIndicator | |
| 41 | - | historySidebar *component.HistorySidebar | |
| 42 | - | sidebarVisible bool | |
| 23 | + | id string | |
| 24 | + | initialURL string | |
| 25 | + | tabs *entity.TabList // per-window tab list (single source of truth for tab state) | |
| 26 | + | mainWindow *window.MainWindow | |
| 27 | + | appToaster *component.Toaster | |
| 28 | + | modeToaster *component.Toaster | |
| 29 | + | touchpadNavIndicator *component.TouchpadNavigationIndicator | |
| 30 | + | borderMgr *focus.BorderManager | |
| 31 | + | sessionManager *component.SessionManager | |
| 32 | + | tabPicker *component.TabPicker | |
| 33 | + | tabPickerWidget layout.Widget | |
| 34 | + | tabPickerPaneID entity.PaneID | |
| 35 | + | insertAccentUC *usecase.InsertAccentUseCase | |
| 36 | + | accentPicker *component.AccentPicker | |
| 37 | + | keyboardHandler *input.KeyboardHandler | |
| 38 | + | globalShortcutHandler *input.GlobalShortcutHandler | |
| 39 | + | permissionDialog port.PermissionDialogPresenter | |
| 40 | + | webrtcIndicator *component.WebRTCPermissionIndicator | |
| 41 | + | historySidebar *component.HistorySidebar | |
| 42 | + | historySidebarReloader historySidebarReloader | |
| 43 | + | sidebarVisible bool | |
| 43 | 44 | } | |
| 44 | 45 | ||
| 45 | 46 | func (bw *browserWindow) detachInputForDestroy() { | |
| @@ -90,6 +91,7 @@ func (bw *browserWindow) clearShellState() { | |||
| 90 | 91 | bw.permissionDialog = nil | |
| 91 | 92 | bw.webrtcIndicator = nil | |
| 92 | 93 | bw.historySidebar = nil | |
| 94 | + | bw.historySidebarReloader = nil | |
| 93 | 95 | } | |
| 94 | 96 | ||
| 95 | 97 | func (bw *browserWindow) initChrome(ctx context.Context, a *App) { | |
M internal/ui/browser_window_history_sidebar_test.go +136 -0
| @@ -2,17 +2,23 @@ package ui | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "reflect" | |
| 6 | + | "sync" | |
| 5 | 7 | "testing" | |
| 8 | + | "unsafe" | |
| 6 | 9 | ||
| 10 | + | "github.com/bnema/dumber/internal/application/dto" | |
| 7 | 11 | "github.com/bnema/dumber/internal/application/usecase" | |
| 8 | 12 | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | 13 | "github.com/bnema/dumber/internal/infrastructure/config" | |
| 14 | + | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 10 | 15 | "github.com/bnema/dumber/internal/ui/component" | |
| 11 | 16 | "github.com/bnema/dumber/internal/ui/coordinator" | |
| 12 | 17 | contentcoord "github.com/bnema/dumber/internal/ui/coordinator/content" | |
| 13 | 18 | "github.com/bnema/dumber/internal/ui/dispatcher" | |
| 14 | 19 | "github.com/bnema/dumber/internal/ui/input" | |
| 15 | 20 | "github.com/bnema/dumber/internal/ui/window" | |
| 21 | + | "github.com/bnema/puregotk/v4/glib" | |
| 16 | 22 | "github.com/stretchr/testify/assert" | |
| 17 | 23 | "github.com/stretchr/testify/require" | |
| 18 | 24 | ) | |
| @@ -889,3 +895,133 @@ func TestBrowserWindow_ApplySidebarWidthConfig_NilDepsIsSafe(t *testing.T) { | |||
| 889 | 895 | app := &App{deps: nil} | |
| 890 | 896 | require.NotPanics(t, func() { bw.applySidebarWidthConfig(app) }) | |
| 891 | 897 | } | |
| 898 | + | ||
| 899 | + | type recordingHistorySidebarReloader struct { | |
| 900 | + | calls int | |
| 901 | + | reasons []string | |
| 902 | + | } | |
| 903 | + | ||
| 904 | + | func (r *recordingHistorySidebarReloader) RequestReloadIfVisible(reason string) { | |
| 905 | + | r.calls++ | |
| 906 | + | r.reasons = append(r.reasons, reason) | |
| 907 | + | } | |
| 908 | + | ||
| 909 | + | func TestBrowserWindow_ClearShellStateClearsHistorySidebarReloader(t *testing.T) { | |
| 910 | + | reloader := &recordingHistorySidebarReloader{} | |
| 911 | + | bw := &browserWindow{ | |
| 912 | + | historySidebar: &component.HistorySidebar{}, | |
| 913 | + | historySidebarReloader: reloader, | |
| 914 | + | } | |
| 915 | + | ||
| 916 | + | bw.clearShellState() | |
| 917 | + | ||
| 918 | + | assert.Nil(t, bw.historySidebar) | |
| 919 | + | assert.Nil(t, bw.historySidebarReloader) | |
| 920 | + | } | |
| 921 | + | ||
| 922 | + | func TestHistoryChangeAdapter_DispatchesBeforeRefreshingSidebars(t *testing.T) { | |
| 923 | + | reloader := &recordingHistorySidebarReloader{} | |
| 924 | + | bw := &browserWindow{id: "window-1", sidebarVisible: true, historySidebarReloader: reloader} | |
| 925 | + | var dispatched bool | |
| 926 | + | var directCalls int | |
| 927 | + | app := &App{ | |
| 928 | + | browserWindows: map[string]*browserWindow{bw.id: bw}, | |
| 929 | + | dispatchOnMainThread: func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 930 | + | dispatched = true | |
| 931 | + | assert.Equal(t, "ui.history_sidebar_refresh", label) | |
| 932 | + | assert.Zero(t, reloader.calls, "sidebar must not be touched before main-thread dispatch runs") | |
| 933 | + | directCalls = reloader.calls | |
| 934 | + | fn() | |
| 935 | + | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchCompleted} | |
| 936 | + | }, | |
| 937 | + | } | |
| 938 | + | ||
| 939 | + | newHistoryChangeAdapter(app).OnHistoryChanged(t.Context(), dto.HistoryChange{Reasons: []dto.HistoryChangeReason{dto.HistoryChangeReasonVisit}}) | |
| 940 | + | ||
| 941 | + | assert.True(t, dispatched) | |
| 942 | + | assert.Zero(t, directCalls) | |
| 943 | + | assert.Equal(t, 1, reloader.calls) | |
| 944 | + | assert.Equal(t, []string{"visit"}, reloader.reasons) | |
| 945 | + | } | |
| 946 | + | ||
| 947 | + | func TestHistoryChangeAdapter_SkipsRefreshWithoutDispatcher(t *testing.T) { | |
| 948 | + | reloader := &recordingHistorySidebarReloader{} | |
| 949 | + | bw := &browserWindow{id: "window-1", sidebarVisible: true, historySidebarReloader: reloader} | |
| 950 | + | app := &App{browserWindows: map[string]*browserWindow{bw.id: bw}} | |
| 951 | + | ||
| 952 | + | newHistoryChangeAdapter(app).OnHistoryChanged( | |
| 953 | + | t.Context(), | |
| 954 | + | dto.HistoryChange{Reasons: []dto.HistoryChangeReason{dto.HistoryChangeReasonVisit}}, | |
| 955 | + | ) | |
| 956 | + | ||
| 957 | + | assert.Zero(t, reloader.calls) | |
| 958 | + | } | |
| 959 | + | ||
| 960 | + | func TestApp_RefreshVisibleHistorySidebars_OnlyVisibleInitializedWindows(t *testing.T) { | |
| 961 | + | visibleOne := &recordingHistorySidebarReloader{} | |
| 962 | + | visibleTwo := &recordingHistorySidebarReloader{} | |
| 963 | + | hidden := &recordingHistorySidebarReloader{} | |
| 964 | + | fallbackSidebar := &component.HistorySidebar{} | |
| 965 | + | var fallbackMu sync.Mutex | |
| 966 | + | var fallbackIdle []glib.SourceFunc | |
| 967 | + | var fallbackTimers []glib.SourceFunc | |
| 968 | + | setComponentHistorySidebarField(t, fallbackSidebar, "visible", true) | |
| 969 | + | setComponentHistorySidebarField(t, fallbackSidebar, "idleScheduler", func(cb glib.SourceFunc) { | |
| 970 | + | fallbackMu.Lock() | |
| 971 | + | defer fallbackMu.Unlock() | |
| 972 | + | fallbackIdle = append(fallbackIdle, cb) | |
| 973 | + | }) | |
| 974 | + | setComponentHistorySidebarField(t, fallbackSidebar, "timeoutAdd", func(_ uint, cb glib.SourceFunc) uint { | |
| 975 | + | fallbackMu.Lock() | |
| 976 | + | defer fallbackMu.Unlock() | |
| 977 | + | fallbackTimers = append(fallbackTimers, cb) | |
| 978 | + | return uint(100 + len(fallbackTimers)) | |
| 979 | + | }) | |
| 980 | + | app := &App{browserWindows: map[string]*browserWindow{ | |
| 981 | + | "visible-1": {id: "visible-1", sidebarVisible: true, historySidebarReloader: visibleOne}, | |
| 982 | + | "hidden": {id: "hidden", sidebarVisible: false, historySidebarReloader: hidden}, | |
| 983 | + | "uninitialized": {id: "uninitialized", sidebarVisible: true}, | |
| 984 | + | "visible-2": {id: "visible-2", sidebarVisible: true, historySidebarReloader: visibleTwo}, | |
| 985 | + | "visible-fallback": {id: "visible-fallback", sidebarVisible: true, historySidebar: fallbackSidebar}, | |
| 986 | + | "destroyed-no-op": nil, | |
| 987 | + | }} | |
| 988 | + | ||
| 989 | + | require.NotPanics(t, func() { | |
| 990 | + | app.refreshVisibleHistorySidebars(dto.HistoryChange{Reasons: []dto.HistoryChangeReason{dto.HistoryChangeReasonClear}}) | |
| 991 | + | }) | |
| 992 | + | ||
| 993 | + | assert.Equal(t, 1, visibleOne.calls) | |
| 994 | + | assert.Equal(t, 1, visibleTwo.calls) | |
| 995 | + | assert.Zero(t, hidden.calls) | |
| 996 | + | assert.Equal(t, []string{"clear"}, visibleOne.reasons) | |
| 997 | + | assert.Equal(t, []string{"clear"}, visibleTwo.reasons) | |
| 998 | + | ||
| 999 | + | fallbackMu.Lock() | |
| 1000 | + | require.Len(t, fallbackIdle, 1) | |
| 1001 | + | idleCb := fallbackIdle[0] | |
| 1002 | + | fallbackMu.Unlock() | |
| 1003 | + | assert.False(t, idleCb(0)) | |
| 1004 | + | ||
| 1005 | + | fallbackMu.Lock() | |
| 1006 | + | require.Len(t, fallbackTimers, 1) | |
| 1007 | + | timerCb := fallbackTimers[0] | |
| 1008 | + | fallbackMu.Unlock() | |
| 1009 | + | assert.False(t, timerCb(0)) | |
| 1010 | + | assert.True(t, getComponentHistorySidebarBoolField(t, fallbackSidebar, "loadStarted")) | |
| 1011 | + | } | |
| 1012 | + | ||
| 1013 | + | func setComponentHistorySidebarField(t *testing.T, hs *component.HistorySidebar, name string, value any) { | |
| 1014 | + | t.Helper() | |
| 1015 | + | rv := reflect.ValueOf(hs).Elem() | |
| 1016 | + | field := rv.FieldByName(name) | |
| 1017 | + | require.Truef(t, field.IsValid(), "component.HistorySidebar.%s field missing", name) | |
| 1018 | + | reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())).Elem().Set(reflect.ValueOf(value)) | |
| 1019 | + | } | |
| 1020 | + | ||
| 1021 | + | func getComponentHistorySidebarBoolField(t *testing.T, hs *component.HistorySidebar, name string) bool { | |
| 1022 | + | t.Helper() | |
| 1023 | + | rv := reflect.ValueOf(hs).Elem() | |
| 1024 | + | field := rv.FieldByName(name) | |
| 1025 | + | require.Truef(t, field.IsValid(), "component.HistorySidebar.%s field missing", name) | |
| 1026 | + | return reflect.NewAt(field.Type(), unsafe.Pointer(field.UnsafeAddr())).Elem().Bool() | |
| 1027 | + | } | |
M internal/ui/component/history_model.go +8 -2
| @@ -36,11 +36,14 @@ const ( | |||
| 36 | 36 | // "January 2, 2006" (other years). Within each group entries remain in | |
| 37 | 37 | // the order they arrived (assumed most-recent-first). | |
| 38 | 38 | func groupHistoryByDay(entries []*entity.HistoryEntry) []historyGroup { | |
| 39 | + | return groupHistoryByDayAt(entries, time.Now()) | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | func groupHistoryByDayAt(entries []*entity.HistoryEntry, now time.Time) []historyGroup { | |
| 39 | 43 | if len(entries) == 0 { | |
| 40 | 44 | return nil | |
| 41 | 45 | } | |
| 42 | 46 | ||
| 43 | - | now := time.Now() | |
| 44 | 47 | local := now.Location() | |
| 45 | 48 | todayStart := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, local) | |
| 46 | 49 | ||
| @@ -108,7 +111,10 @@ func readableURL(rawURL string) string { | |||
| 108 | 111 | // relativeTime returns a compact relative time label for a timestamp. | |
| 109 | 112 | // Examples: "2m ago", "1h ago", "3d ago", "Jan 2". | |
| 110 | 113 | func relativeTime(t time.Time) string { | |
| 111 | - | now := time.Now() | |
| 114 | + | return relativeTimeAt(t, time.Now()) | |
| 115 | + | } | |
| 116 | + | ||
| 117 | + | func relativeTimeAt(t, now time.Time) string { | |
| 112 | 118 | d := now.Sub(t) | |
| 113 | 119 | ||
| 114 | 120 | switch { | |
M internal/ui/component/history_model_test.go +4 -4
| @@ -46,12 +46,12 @@ func TestGroupHistoryByDay_TodayYesterdayOlder(t *testing.T) { | |||
| 46 | 46 | } | |
| 47 | 47 | ||
| 48 | 48 | func TestGroupHistoryByDay_SameDayEntriesGrouped(t *testing.T) { | |
| 49 | - | now := time.Now() | |
| 49 | + | now := time.Date(2026, time.June, 24, 12, 0, 0, 0, time.Local) | |
| 50 | 50 | entries := []*entity.HistoryEntry{ | |
| 51 | 51 | {ID: 1, URL: "https://a.com", Title: "A", LastVisited: now}, | |
| 52 | 52 | {ID: 2, URL: "https://b.com", Title: "B", LastVisited: now.Add(-time.Hour)}, | |
| 53 | 53 | } | |
| 54 | - | groups := groupHistoryByDay(entries) | |
| 54 | + | groups := groupHistoryByDayAt(entries, now) | |
| 55 | 55 | require.Len(t, groups, 1) | |
| 56 | 56 | assert.Equal(t, "Today", groups[0].Label) | |
| 57 | 57 | assert.Len(t, groups[0].Entries, 2) | |
| @@ -106,13 +106,13 @@ func TestGroupHistoryByDay_CrossYearDifferentLabels(t *testing.T) { | |||
| 106 | 106 | } | |
| 107 | 107 | ||
| 108 | 108 | func TestGroupHistoryByDay_MaintainsInputOrderWithinDay(t *testing.T) { | |
| 109 | - | now := time.Now() | |
| 109 | + | now := time.Date(2026, time.June, 24, 12, 0, 0, 0, time.Local) | |
| 110 | 110 | entries := []*entity.HistoryEntry{ | |
| 111 | 111 | {ID: 1, URL: "https://first.com", Title: "First", LastVisited: now}, | |
| 112 | 112 | {ID: 2, URL: "https://second.com", Title: "Second", LastVisited: now.Add(-30 * time.Minute)}, | |
| 113 | 113 | {ID: 3, URL: "https://third.com", Title: "Third", LastVisited: now.Add(-2 * time.Hour)}, | |
| 114 | 114 | } | |
| 115 | - | groups := groupHistoryByDay(entries) | |
| 115 | + | groups := groupHistoryByDayAt(entries, now) | |
| 116 | 116 | require.Len(t, groups, 1) | |
| 117 | 117 | require.Len(t, groups[0].Entries, 3) | |
| 118 | 118 | // Entries within the same day maintain input order | |
M internal/ui/component/history_sidebar.go +49 -7
| @@ -3,6 +3,7 @@ package component | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "sync" | |
| 6 | + | "time" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/puregotk/v4/glib" | |
| 8 | 9 | "github.com/bnema/puregotk/v4/gtk" | |
| @@ -19,6 +20,8 @@ const ( | |||
| 19 | 20 | sidebarMinWidth = 280 | |
| 20 | 21 | sidebarDefaultWidth = 320 | |
| 21 | 22 | sidebarSearchDebounceMs = 150 | |
| 23 | + | sidebarReloadDebounceMs = 200 | |
| 24 | + | sidebarRelativeTickMs = 60 * 1000 | |
| 22 | 25 | sidebarPageSize = 100 // entries fetched per page | |
| 23 | 26 | sidebarSearchLimit = 100 // max FTS search results | |
| 24 | 27 | ) | |
| @@ -86,8 +89,14 @@ type HistorySidebar struct { | |||
| 86 | 89 | prevScrollValue float64 | |
| 87 | 90 | prevSelectedURL string | |
| 88 | 91 | ||
| 89 | - | // Search debounce timer | |
| 90 | - | debounceTimer uint | |
| 92 | + | // Search/reload/relative-time timers. GLib source IDs are owned and | |
| 93 | + | // created/removed on the GTK main context. | |
| 94 | + | debounceTimer uint | |
| 95 | + | reloadDebounceTimer uint | |
| 96 | + | relativeTimeTicker uint | |
| 97 | + | relativeTimeLabelBinds []relativeTimeLabelBinding | |
| 98 | + | relativeTimeDayKey dayKey | |
| 99 | + | relativeTimeDayKeySet bool | |
| 91 | 100 | ||
| 92 | 101 | // Retained callbacks | |
| 93 | 102 | retainedCallbacks []interface{} | |
| @@ -95,6 +104,9 @@ type HistorySidebar struct { | |||
| 95 | 104 | // idleScheduler dispatches work onto the GTK main thread. | |
| 96 | 105 | // Tests may override it to exercise scheduled callbacks deterministically. | |
| 97 | 106 | idleScheduler func(glib.SourceFunc) | |
| 107 | + | timeoutAdd func(uint, glib.SourceFunc) uint | |
| 108 | + | sourceRemove func(uint) | |
| 109 | + | now func() time.Time | |
| 98 | 110 | ||
| 99 | 111 | // Context | |
| 100 | 112 | ctx context.Context | |
| @@ -202,6 +214,10 @@ func (hs *HistorySidebar) Destroy() { | |||
| 202 | 214 | hs.destroyed = true | |
| 203 | 215 | timerID := hs.debounceTimer | |
| 204 | 216 | hs.debounceTimer = 0 | |
| 217 | + | reloadTimerID := hs.cancelReloadDebounceLocked() | |
| 218 | + | tickerID := hs.relativeTimeTicker | |
| 219 | + | hs.relativeTimeTicker = 0 | |
| 220 | + | hs.clearRelativeTimeBindingsLocked() | |
| 205 | 221 | hs.mu.Unlock() | |
| 206 | 222 | ||
| 207 | 223 | if hs.cancel != nil { | |
| @@ -209,7 +225,13 @@ func (hs *HistorySidebar) Destroy() { | |||
| 209 | 225 | } | |
| 210 | 226 | ||
| 211 | 227 | if timerID != 0 { | |
| 212 | - | glib.SourceRemove(timerID) | |
| 228 | + | hs.removeSource(timerID) | |
| 229 | + | } | |
| 230 | + | if reloadTimerID != 0 { | |
| 231 | + | hs.removeSource(reloadTimerID) | |
| 232 | + | } | |
| 233 | + | if tickerID != 0 { | |
| 234 | + | hs.removeSource(tickerID) | |
| 213 | 235 | } | |
| 214 | 236 | } | |
| 215 | 237 | ||
| @@ -239,10 +261,17 @@ func (hs *HistorySidebar) Show() { | |||
| 239 | 261 | return | |
| 240 | 262 | } | |
| 241 | 263 | ||
| 264 | + | reloadTimerID := hs.reloadDebounceTimer | |
| 265 | + | hs.reloadDebounceTimer = 0 | |
| 242 | 266 | hs.outerBox.SetVisible(true) | |
| 243 | 267 | hs.visible = true | |
| 244 | 268 | hs.mu.Unlock() | |
| 245 | 269 | ||
| 270 | + | if reloadTimerID != 0 { | |
| 271 | + | hs.removeSource(reloadTimerID) | |
| 272 | + | } | |
| 273 | + | hs.startRelativeTimeTicker() | |
| 274 | + | ||
| 246 | 275 | // Schedule a background reload so the sidebar shows fresh data | |
| 247 | 276 | // when it becomes visible, not stale data captured at init time. | |
| 248 | 277 | reloadCb := glib.SourceFunc(func(uintptr) bool { | |
| @@ -275,14 +304,27 @@ func (hs *HistorySidebar) Show() { | |||
| 275 | 304 | // Hide hides the sidebar. | |
| 276 | 305 | func (hs *HistorySidebar) Hide() { | |
| 277 | 306 | hs.mu.Lock() | |
| 278 | - | defer hs.mu.Unlock() | |
| 279 | - | ||
| 280 | - | if hs.outerBox == nil || hs.destroyed { | |
| 307 | + | if hs.destroyed { | |
| 308 | + | hs.mu.Unlock() | |
| 281 | 309 | return | |
| 282 | 310 | } | |
| 283 | 311 | ||
| 284 | - | hs.outerBox.SetVisible(false) | |
| 312 | + | reloadTimerID := hs.cancelReloadDebounceLocked() | |
| 313 | + | tickerID := hs.relativeTimeTicker | |
| 314 | + | hs.relativeTimeTicker = 0 | |
| 315 | + | hs.clearRelativeTimeBindingsLocked() | |
| 316 | + | if hs.outerBox != nil { | |
| 317 | + | hs.outerBox.SetVisible(false) | |
| 318 | + | } | |
| 285 | 319 | hs.visible = false | |
| 320 | + | hs.mu.Unlock() | |
| 321 | + | ||
| 322 | + | if reloadTimerID != 0 { | |
| 323 | + | hs.removeSource(reloadTimerID) | |
| 324 | + | } | |
| 325 | + | if tickerID != 0 { | |
| 326 | + | hs.removeSource(tickerID) | |
| 327 | + | } | |
| 286 | 328 | } | |
| 287 | 329 | ||
| 288 | 330 | // IsVisible returns whether the sidebar is visible. | |
M internal/ui/component/history_sidebar_loading_search.go +12 -9
| @@ -184,10 +184,10 @@ func (hs *HistorySidebar) preserveScrollAndSelection() { | |||
| 184 | 184 | ||
| 185 | 185 | // restoreScrollAndSelection restores the previously saved scroll position and | |
| 186 | 186 | // selection after a rebuild. Called on the GTK main thread. | |
| 187 | - | func (hs *HistorySidebar) restoreScrollAndSelection() { | |
| 187 | + | func (hs *HistorySidebar) restoreScrollAndSelectionInListBox(listBox *gtk.ListBox) { | |
| 188 | 188 | // Restore selection first (changes scroll position) | |
| 189 | 189 | if hs.prevSelectedURL != "" { | |
| 190 | - | hs.selectRowByURL(hs.prevSelectedURL) | |
| 190 | + | hs.selectRowByURLInListBox(listBox, hs.prevSelectedURL) | |
| 191 | 191 | } | |
| 192 | 192 | ||
| 193 | 193 | // Then restore scroll position if we have one | |
| @@ -232,12 +232,12 @@ func (hs *HistorySidebar) entryURLAtIndex(index int) string { | |||
| 232 | 232 | } | |
| 233 | 233 | ||
| 234 | 234 | // selectRowByURL finds and selects a row whose URL matches. | |
| 235 | - | func (hs *HistorySidebar) selectRowByURL(url string) { | |
| 236 | - | if url == "" || hs.listBox == nil { | |
| 235 | + | func (hs *HistorySidebar) selectRowByURLInListBox(listBox *gtk.ListBox, url string) { | |
| 236 | + | if url == "" || listBox == nil { | |
| 237 | 237 | return | |
| 238 | 238 | } | |
| 239 | 239 | for i := 0; ; i++ { | |
| 240 | - | row := hs.listBox.GetRowAtIndex(i) | |
| 240 | + | row := listBox.GetRowAtIndex(i) | |
| 241 | 241 | if row == nil { | |
| 242 | 242 | break | |
| 243 | 243 | } | |
| @@ -245,7 +245,7 @@ func (hs *HistorySidebar) selectRowByURL(url string) { | |||
| 245 | 245 | continue | |
| 246 | 246 | } | |
| 247 | 247 | if hs.getRowURL(row) == url { | |
| 248 | - | hs.listBox.SelectRow(row) | |
| 248 | + | listBox.SelectRow(row) | |
| 249 | 249 | return | |
| 250 | 250 | } | |
| 251 | 251 | } | |
| @@ -280,20 +280,20 @@ func (hs *HistorySidebar) onSearchChanged() { | |||
| 280 | 280 | hs.mu.Unlock() | |
| 281 | 281 | ||
| 282 | 282 | if oldTimer != 0 { | |
| 283 | - | glib.SourceRemove(oldTimer) | |
| 283 | + | hs.removeSource(oldTimer) | |
| 284 | 284 | } | |
| 285 | 285 | ||
| 286 | 286 | filterCb := glib.SourceFunc(func(uintptr) bool { | |
| 287 | 287 | hs.applyFilter() | |
| 288 | 288 | return false | |
| 289 | 289 | }) | |
| 290 | - | timerID := glib.TimeoutAdd(uint(sidebarSearchDebounceMs), &filterCb, 0) | |
| 290 | + | timerID := hs.addTimeout(sidebarSearchDebounceMs, filterCb) | |
| 291 | 291 | ||
| 292 | 292 | hs.mu.Lock() | |
| 293 | 293 | if hs.destroyed { | |
| 294 | 294 | hs.mu.Unlock() | |
| 295 | 295 | if timerID != 0 { | |
| 296 | - | glib.SourceRemove(timerID) | |
| 296 | + | hs.removeSource(timerID) | |
| 297 | 297 | } | |
| 298 | 298 | return | |
| 299 | 299 | } | |
| @@ -419,6 +419,9 @@ func (hs *HistorySidebar) scheduleClearList() { | |||
| 419 | 419 | if destroyed || listBox == nil { | |
| 420 | 420 | return false | |
| 421 | 421 | } | |
| 422 | + | hs.mu.Lock() | |
| 423 | + | hs.clearRelativeTimeBindingsLocked() | |
| 424 | + | hs.mu.Unlock() | |
| 422 | 425 | listBox.RemoveAll() | |
| 423 | 426 | return false | |
| 424 | 427 | }) | |
A internal/ui/component/history_sidebar_reactivity.go +215 -0
| @@ -0,0 +1,215 @@ | |||
| 1 | + | package component | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "time" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/puregotk/v4/glib" | |
| 7 | + | "github.com/bnema/puregotk/v4/gtk" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | type relativeTimeLabelBinding struct { | |
| 11 | + | label *gtk.Label | |
| 12 | + | lastVisited time.Time | |
| 13 | + | } | |
| 14 | + | ||
| 15 | + | func (hs *HistorySidebar) RequestReloadIfVisible(reason string) { | |
| 16 | + | _ = reason | |
| 17 | + | if hs == nil { | |
| 18 | + | return | |
| 19 | + | } | |
| 20 | + | hs.mu.RLock() | |
| 21 | + | if hs.destroyed || !hs.visible { | |
| 22 | + | hs.mu.RUnlock() | |
| 23 | + | return | |
| 24 | + | } | |
| 25 | + | hs.mu.RUnlock() | |
| 26 | + | cb := glib.SourceFunc(func(uintptr) bool { | |
| 27 | + | hs.requestReloadIfVisibleOnMain() | |
| 28 | + | return false | |
| 29 | + | }) | |
| 30 | + | hs.scheduleIdle(cb) | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | func (hs *HistorySidebar) requestReloadIfVisibleOnMain() { | |
| 34 | + | hs.mu.Lock() | |
| 35 | + | if hs.destroyed || !hs.visible { | |
| 36 | + | hs.mu.Unlock() | |
| 37 | + | return | |
| 38 | + | } | |
| 39 | + | oldTimer := hs.reloadDebounceTimer | |
| 40 | + | hs.reloadDebounceTimer = 0 | |
| 41 | + | hs.mu.Unlock() | |
| 42 | + | ||
| 43 | + | if oldTimer != 0 { | |
| 44 | + | hs.removeSource(oldTimer) | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | var timerID uint | |
| 48 | + | reloadCb := glib.SourceFunc(func(uintptr) bool { | |
| 49 | + | hs.mu.Lock() | |
| 50 | + | if hs.reloadDebounceTimer != timerID { | |
| 51 | + | hs.mu.Unlock() | |
| 52 | + | return false | |
| 53 | + | } | |
| 54 | + | hs.reloadDebounceTimer = 0 | |
| 55 | + | if hs.destroyed || !hs.visible { | |
| 56 | + | hs.mu.Unlock() | |
| 57 | + | return false | |
| 58 | + | } | |
| 59 | + | hs.mu.Unlock() | |
| 60 | + | hs.Reload() | |
| 61 | + | return false | |
| 62 | + | }) | |
| 63 | + | timerID = hs.addTimeout(sidebarReloadDebounceMs, reloadCb) | |
| 64 | + | ||
| 65 | + | hs.mu.Lock() | |
| 66 | + | if hs.destroyed || !hs.visible { | |
| 67 | + | hs.mu.Unlock() | |
| 68 | + | if timerID != 0 { | |
| 69 | + | hs.removeSource(timerID) | |
| 70 | + | } | |
| 71 | + | return | |
| 72 | + | } | |
| 73 | + | hs.reloadDebounceTimer = timerID | |
| 74 | + | hs.mu.Unlock() | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | func (hs *HistorySidebar) cancelReloadDebounceLocked() uint { | |
| 78 | + | timerID := hs.reloadDebounceTimer | |
| 79 | + | hs.reloadDebounceTimer = 0 | |
| 80 | + | return timerID | |
| 81 | + | } | |
| 82 | + | ||
| 83 | + | func (hs *HistorySidebar) bindRelativeTimeLabel(label *gtk.Label, visited time.Time) { | |
| 84 | + | if label == nil { | |
| 85 | + | return | |
| 86 | + | } | |
| 87 | + | hs.mu.Lock() | |
| 88 | + | if !hs.destroyed { | |
| 89 | + | hs.relativeTimeLabelBinds = append(hs.relativeTimeLabelBinds, relativeTimeLabelBinding{label: label, lastVisited: visited}) | |
| 90 | + | } | |
| 91 | + | hs.mu.Unlock() | |
| 92 | + | } | |
| 93 | + | ||
| 94 | + | func (hs *HistorySidebar) clearRelativeTimeBindingsLocked() { | |
| 95 | + | hs.relativeTimeLabelBinds = nil | |
| 96 | + | } | |
| 97 | + | ||
| 98 | + | func (hs *HistorySidebar) startRelativeTimeTicker() { | |
| 99 | + | hs.mu.Lock() | |
| 100 | + | if hs.destroyed || !hs.visible { | |
| 101 | + | hs.mu.Unlock() | |
| 102 | + | return | |
| 103 | + | } | |
| 104 | + | oldTicker := hs.relativeTimeTicker | |
| 105 | + | hs.relativeTimeTicker = 0 | |
| 106 | + | hs.relativeTimeDayKey = hs.currentDayKey() | |
| 107 | + | hs.relativeTimeDayKeySet = true | |
| 108 | + | hs.mu.Unlock() | |
| 109 | + | ||
| 110 | + | if oldTicker != 0 { | |
| 111 | + | hs.removeSource(oldTicker) | |
| 112 | + | } | |
| 113 | + | ||
| 114 | + | hs.scheduleRelativeTimeRefresh() | |
| 115 | + | ||
| 116 | + | var tickerID uint | |
| 117 | + | tickCb := glib.SourceFunc(func(uintptr) bool { | |
| 118 | + | hs.mu.RLock() | |
| 119 | + | keep := !hs.destroyed && hs.visible && hs.relativeTimeTicker == tickerID | |
| 120 | + | hs.mu.RUnlock() | |
| 121 | + | if !keep { | |
| 122 | + | return false | |
| 123 | + | } | |
| 124 | + | hs.scheduleRelativeTimeRefresh() | |
| 125 | + | return true | |
| 126 | + | }) | |
| 127 | + | tickerID = hs.addTimeout(sidebarRelativeTickMs, tickCb) | |
| 128 | + | ||
| 129 | + | hs.mu.Lock() | |
| 130 | + | if hs.destroyed || !hs.visible { | |
| 131 | + | hs.mu.Unlock() | |
| 132 | + | if tickerID != 0 { | |
| 133 | + | hs.removeSource(tickerID) | |
| 134 | + | } | |
| 135 | + | return | |
| 136 | + | } | |
| 137 | + | hs.relativeTimeTicker = tickerID | |
| 138 | + | hs.mu.Unlock() | |
| 139 | + | } | |
| 140 | + | ||
| 141 | + | func (hs *HistorySidebar) scheduleRelativeTimeRefresh() { | |
| 142 | + | cb := glib.SourceFunc(func(uintptr) bool { | |
| 143 | + | hs.updateRelativeTimeLabelsOnMain() | |
| 144 | + | return false | |
| 145 | + | }) | |
| 146 | + | hs.scheduleIdle(cb) | |
| 147 | + | } | |
| 148 | + | ||
| 149 | + | func (hs *HistorySidebar) updateRelativeTimeLabelsOnMain() { | |
| 150 | + | now := hs.currentTime() | |
| 151 | + | hs.mu.RLock() | |
| 152 | + | if hs.destroyed || !hs.visible { | |
| 153 | + | hs.mu.RUnlock() | |
| 154 | + | return | |
| 155 | + | } | |
| 156 | + | currentDay := dayKeyFromTime(now) | |
| 157 | + | lastDay := hs.relativeTimeDayKey | |
| 158 | + | daySet := hs.relativeTimeDayKeySet | |
| 159 | + | bindings := append([]relativeTimeLabelBinding(nil), hs.relativeTimeLabelBinds...) | |
| 160 | + | hs.mu.RUnlock() | |
| 161 | + | ||
| 162 | + | if daySet && currentDay != lastDay { | |
| 163 | + | shouldReload := false | |
| 164 | + | hs.mu.Lock() | |
| 165 | + | if !hs.destroyed && hs.visible && hs.relativeTimeDayKeySet && hs.relativeTimeDayKey != currentDay { | |
| 166 | + | hs.relativeTimeDayKey = currentDay | |
| 167 | + | shouldReload = true | |
| 168 | + | } | |
| 169 | + | hs.mu.Unlock() | |
| 170 | + | if shouldReload { | |
| 171 | + | hs.RequestReloadIfVisible("day-boundary") | |
| 172 | + | } | |
| 173 | + | return | |
| 174 | + | } | |
| 175 | + | ||
| 176 | + | for _, binding := range bindings { | |
| 177 | + | if binding.label == nil { | |
| 178 | + | continue | |
| 179 | + | } | |
| 180 | + | binding.label.SetText(relativeTimeAt(binding.lastVisited, now)) | |
| 181 | + | } | |
| 182 | + | } | |
| 183 | + | ||
| 184 | + | func (hs *HistorySidebar) currentTime() time.Time { | |
| 185 | + | if hs != nil && hs.now != nil { | |
| 186 | + | return hs.now() | |
| 187 | + | } | |
| 188 | + | return time.Now() | |
| 189 | + | } | |
| 190 | + | ||
| 191 | + | func (hs *HistorySidebar) currentDayKey() dayKey { | |
| 192 | + | return dayKeyFromTime(hs.currentTime()) | |
| 193 | + | } | |
| 194 | + | ||
| 195 | + | func dayKeyFromTime(t time.Time) dayKey { | |
| 196 | + | return dayKey{year: t.Year(), month: t.Month(), day: t.Day()} | |
| 197 | + | } | |
| 198 | + | ||
| 199 | + | func (hs *HistorySidebar) addTimeout(ms uint, cb glib.SourceFunc) uint { | |
| 200 | + | if hs != nil && hs.timeoutAdd != nil { | |
| 201 | + | return hs.timeoutAdd(ms, cb) | |
| 202 | + | } | |
| 203 | + | return glib.TimeoutAdd(ms, &cb, 0) | |
| 204 | + | } | |
| 205 | + | ||
| 206 | + | func (hs *HistorySidebar) removeSource(id uint) { | |
| 207 | + | if id == 0 { | |
| 208 | + | return | |
| 209 | + | } | |
| 210 | + | if hs != nil && hs.sourceRemove != nil { | |
| 211 | + | hs.sourceRemove(id) | |
| 212 | + | return | |
| 213 | + | } | |
| 214 | + | glib.SourceRemove(id) | |
| 215 | + | } | |
A internal/ui/component/history_sidebar_reactivity_test.go +282 -0
| @@ -0,0 +1,282 @@ | |||
| 1 | + | package component | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "sync" | |
| 5 | + | "testing" | |
| 6 | + | "time" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | + | "github.com/bnema/puregotk/v4/glib" | |
| 10 | + | "github.com/stretchr/testify/assert" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | type sidebarReactivityHarness struct { | |
| 15 | + | hs *HistorySidebar | |
| 16 | + | idle []glib.SourceFunc | |
| 17 | + | timers map[uint]glib.SourceFunc | |
| 18 | + | removed []uint | |
| 19 | + | nextID uint | |
| 20 | + | idleMu sync.Mutex | |
| 21 | + | timerMu sync.Mutex | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | func newSidebarReactivityHarness() *sidebarReactivityHarness { | |
| 25 | + | h := &sidebarReactivityHarness{timers: make(map[uint]glib.SourceFunc), nextID: 10} | |
| 26 | + | hs := newTestSidebarSearchHarness() | |
| 27 | + | hs.now = func() time.Time { return time.Date(2026, 6, 24, 12, 0, 0, 0, time.UTC) } | |
| 28 | + | hs.idleScheduler = func(cb glib.SourceFunc) { | |
| 29 | + | h.idleMu.Lock() | |
| 30 | + | defer h.idleMu.Unlock() | |
| 31 | + | h.idle = append(h.idle, cb) | |
| 32 | + | } | |
| 33 | + | hs.timeoutAdd = func(_ uint, cb glib.SourceFunc) uint { | |
| 34 | + | h.timerMu.Lock() | |
| 35 | + | defer h.timerMu.Unlock() | |
| 36 | + | h.nextID++ | |
| 37 | + | h.timers[h.nextID] = cb | |
| 38 | + | return h.nextID | |
| 39 | + | } | |
| 40 | + | hs.sourceRemove = func(id uint) { | |
| 41 | + | h.timerMu.Lock() | |
| 42 | + | defer h.timerMu.Unlock() | |
| 43 | + | h.removed = append(h.removed, id) | |
| 44 | + | delete(h.timers, id) | |
| 45 | + | } | |
| 46 | + | h.hs = hs | |
| 47 | + | return h | |
| 48 | + | } | |
| 49 | + | ||
| 50 | + | func (h *sidebarReactivityHarness) runIdle() int { | |
| 51 | + | h.idleMu.Lock() | |
| 52 | + | callbacks := append([]glib.SourceFunc(nil), h.idle...) | |
| 53 | + | h.idle = nil | |
| 54 | + | h.idleMu.Unlock() | |
| 55 | + | for _, cb := range callbacks { | |
| 56 | + | cb(0) | |
| 57 | + | } | |
| 58 | + | return len(callbacks) | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | func (h *sidebarReactivityHarness) singleTimer(t *testing.T) (uint, glib.SourceFunc) { | |
| 62 | + | t.Helper() | |
| 63 | + | h.timerMu.Lock() | |
| 64 | + | defer h.timerMu.Unlock() | |
| 65 | + | require.Len(t, h.timers, 1) | |
| 66 | + | for id, cb := range h.timers { | |
| 67 | + | return id, cb | |
| 68 | + | } | |
| 69 | + | return 0, nil | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | func TestHistorySidebar_RequestReloadIfVisible_HiddenNoop(t *testing.T) { | |
| 73 | + | h := newSidebarReactivityHarness() | |
| 74 | + | h.hs.RequestReloadIfVisible("hidden") | |
| 75 | + | require.Zero(t, h.runIdle()) | |
| 76 | + | assert.Empty(t, h.timers) | |
| 77 | + | assert.Zero(t, h.hs.reloadDebounceTimer) | |
| 78 | + | } | |
| 79 | + | ||
| 80 | + | func TestHistorySidebar_RequestReloadIfVisible_CoalescesVisible(t *testing.T) { | |
| 81 | + | h := newSidebarReactivityHarness() | |
| 82 | + | h.hs.visible = true | |
| 83 | + | h.hs.RequestReloadIfVisible("a") | |
| 84 | + | h.hs.RequestReloadIfVisible("b") | |
| 85 | + | require.Equal(t, 2, h.runIdle()) | |
| 86 | + | assert.Len(t, h.timers, 1) | |
| 87 | + | assert.Len(t, h.removed, 1, "first debounce source should be removed") | |
| 88 | + | assert.NotZero(t, h.hs.reloadDebounceTimer) | |
| 89 | + | } | |
| 90 | + | ||
| 91 | + | func TestHistorySidebar_ReloadDebounceCallbackReloadsOnceAndClearsTimer(t *testing.T) { | |
| 92 | + | h := newSidebarReactivityHarness() | |
| 93 | + | h.hs.visible = true | |
| 94 | + | h.hs.RequestReloadIfVisible("history-change") | |
| 95 | + | require.Equal(t, 1, h.runIdle()) | |
| 96 | + | timerID, cb := h.singleTimer(t) | |
| 97 | + | require.Equal(t, timerID, h.hs.reloadDebounceTimer) | |
| 98 | + | ||
| 99 | + | keep := cb(0) | |
| 100 | + | ||
| 101 | + | assert.False(t, keep, "debounce timeout should be one-shot") | |
| 102 | + | assert.Zero(t, h.hs.reloadDebounceTimer) | |
| 103 | + | assert.True(t, h.hs.loadStarted, "debounce should call Reload and start a history load") | |
| 104 | + | } | |
| 105 | + | ||
| 106 | + | func TestHistorySidebar_ReloadDebounceCallbackSkipsHiddenSidebar(t *testing.T) { | |
| 107 | + | h := newSidebarReactivityHarness() | |
| 108 | + | h.hs.visible = true | |
| 109 | + | h.hs.RequestReloadIfVisible("history-change") | |
| 110 | + | require.Equal(t, 1, h.runIdle()) | |
| 111 | + | _, cb := h.singleTimer(t) | |
| 112 | + | h.hs.visible = false | |
| 113 | + | ||
| 114 | + | keep := cb(0) | |
| 115 | + | ||
| 116 | + | assert.False(t, keep, "debounce timeout should be one-shot") | |
| 117 | + | assert.Zero(t, h.hs.reloadDebounceTimer) | |
| 118 | + | assert.False(t, h.hs.loadStarted, "hidden sidebar should not reload when debounce fires") | |
| 119 | + | } | |
| 120 | + | ||
| 121 | + | func TestHistorySidebar_ReloadDebounceCallbackIgnoresSupersededTimer(t *testing.T) { | |
| 122 | + | h := newSidebarReactivityHarness() | |
| 123 | + | h.hs.visible = true | |
| 124 | + | h.hs.RequestReloadIfVisible("first") | |
| 125 | + | require.Equal(t, 1, h.runIdle()) | |
| 126 | + | firstID, staleCB := h.singleTimer(t) | |
| 127 | + | ||
| 128 | + | h.hs.RequestReloadIfVisible("second") | |
| 129 | + | require.Equal(t, 1, h.runIdle()) | |
| 130 | + | currentID, currentCB := h.singleTimer(t) | |
| 131 | + | require.NotEqual(t, firstID, currentID) | |
| 132 | + | require.Equal(t, currentID, h.hs.reloadDebounceTimer) | |
| 133 | + | ||
| 134 | + | keep := staleCB(0) | |
| 135 | + | ||
| 136 | + | assert.False(t, keep, "stale debounce timeout should be one-shot") | |
| 137 | + | assert.Equal(t, currentID, h.hs.reloadDebounceTimer) | |
| 138 | + | assert.False(t, h.hs.loadStarted, "superseded debounce should not reload") | |
| 139 | + | ||
| 140 | + | keep = currentCB(0) | |
| 141 | + | assert.False(t, keep) | |
| 142 | + | assert.Zero(t, h.hs.reloadDebounceTimer) | |
| 143 | + | assert.True(t, h.hs.loadStarted, "current debounce should still reload") | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | func TestHistorySidebar_DestroyCancelsReloadTickerAndClearsBindings(t *testing.T) { | |
| 147 | + | h := newSidebarReactivityHarness() | |
| 148 | + | h.hs.visible = true | |
| 149 | + | h.hs.reloadDebounceTimer = 51 | |
| 150 | + | h.hs.relativeTimeTicker = 52 | |
| 151 | + | h.hs.relativeTimeLabelBinds = []relativeTimeLabelBinding{{lastVisited: h.hs.currentTime()}} | |
| 152 | + | ||
| 153 | + | h.hs.Destroy() | |
| 154 | + | ||
| 155 | + | h.hs.mu.RLock() | |
| 156 | + | destroyed := h.hs.destroyed | |
| 157 | + | reloadTimer := h.hs.reloadDebounceTimer | |
| 158 | + | ticker := h.hs.relativeTimeTicker | |
| 159 | + | bindings := h.hs.relativeTimeLabelBinds | |
| 160 | + | h.hs.mu.RUnlock() | |
| 161 | + | assert.True(t, destroyed) | |
| 162 | + | assert.Zero(t, reloadTimer) | |
| 163 | + | assert.Zero(t, ticker) | |
| 164 | + | assert.Empty(t, bindings) | |
| 165 | + | assert.Contains(t, h.removed, uint(51)) | |
| 166 | + | assert.Contains(t, h.removed, uint(52)) | |
| 167 | + | } | |
| 168 | + | ||
| 169 | + | func TestHistorySidebar_HideCancelsReloadTickerAndClearsBindingsWithoutDestroying(t *testing.T) { | |
| 170 | + | h := newSidebarReactivityHarness() | |
| 171 | + | h.hs.visible = true | |
| 172 | + | h.hs.reloadDebounceTimer = 61 | |
| 173 | + | h.hs.relativeTimeTicker = 62 | |
| 174 | + | h.hs.relativeTimeLabelBinds = []relativeTimeLabelBinding{{lastVisited: h.hs.currentTime()}} | |
| 175 | + | ||
| 176 | + | h.hs.Hide() | |
| 177 | + | ||
| 178 | + | h.hs.mu.RLock() | |
| 179 | + | destroyed := h.hs.destroyed | |
| 180 | + | visible := h.hs.visible | |
| 181 | + | reloadTimer := h.hs.reloadDebounceTimer | |
| 182 | + | ticker := h.hs.relativeTimeTicker | |
| 183 | + | bindings := h.hs.relativeTimeLabelBinds | |
| 184 | + | h.hs.mu.RUnlock() | |
| 185 | + | assert.False(t, destroyed) | |
| 186 | + | assert.False(t, visible) | |
| 187 | + | assert.Zero(t, reloadTimer) | |
| 188 | + | assert.Zero(t, ticker) | |
| 189 | + | assert.Empty(t, bindings) | |
| 190 | + | assert.Contains(t, h.removed, uint(61)) | |
| 191 | + | assert.Contains(t, h.removed, uint(62)) | |
| 192 | + | } | |
| 193 | + | ||
| 194 | + | func TestHistorySidebar_RelativeTimeBindingsLifecycleAndTicker(t *testing.T) { | |
| 195 | + | h := newSidebarReactivityHarness() | |
| 196 | + | h.hs.visible = true | |
| 197 | + | h.hs.bindRelativeTimeLabel(nil, h.hs.currentTime()) | |
| 198 | + | assert.Empty(t, h.hs.relativeTimeLabelBinds, "nil labels are skipped") | |
| 199 | + | h.hs.relativeTimeLabelBinds = []relativeTimeLabelBinding{{lastVisited: h.hs.currentTime().Add(-time.Minute)}} | |
| 200 | + | h.hs.mu.Lock() | |
| 201 | + | h.hs.clearRelativeTimeBindingsLocked() | |
| 202 | + | h.hs.mu.Unlock() | |
| 203 | + | assert.Empty(t, h.hs.relativeTimeLabelBinds) | |
| 204 | + | ||
| 205 | + | h.hs.startRelativeTimeTicker() | |
| 206 | + | assert.NotZero(t, h.hs.relativeTimeTicker) | |
| 207 | + | assert.Len(t, h.timers, 1) | |
| 208 | + | assert.Equal(t, 1, h.runIdle(), "initial label refresh is scheduled through idle path") | |
| 209 | + | ||
| 210 | + | ticker := h.hs.relativeTimeTicker | |
| 211 | + | h.hs.visible = false | |
| 212 | + | h.hs.relativeTimeTicker = 0 | |
| 213 | + | h.hs.removeSource(ticker) | |
| 214 | + | assert.Contains(t, h.removed, ticker) | |
| 215 | + | } | |
| 216 | + | ||
| 217 | + | func TestHistorySidebar_RelativeTimeTickerIgnoresSupersededTimer(t *testing.T) { | |
| 218 | + | h := newSidebarReactivityHarness() | |
| 219 | + | h.hs.visible = true | |
| 220 | + | ||
| 221 | + | h.hs.startRelativeTimeTicker() | |
| 222 | + | firstID, staleCB := h.singleTimer(t) | |
| 223 | + | require.Equal(t, 1, h.runIdle(), "clear initial refresh") | |
| 224 | + | ||
| 225 | + | h.hs.startRelativeTimeTicker() | |
| 226 | + | currentID, currentCB := h.singleTimer(t) | |
| 227 | + | require.NotEqual(t, firstID, currentID) | |
| 228 | + | require.Equal(t, currentID, h.hs.relativeTimeTicker) | |
| 229 | + | require.Equal(t, 1, h.runIdle(), "clear replacement initial refresh") | |
| 230 | + | ||
| 231 | + | keep := staleCB(0) | |
| 232 | + | ||
| 233 | + | assert.False(t, keep, "stale ticker should stop") | |
| 234 | + | assert.Equal(t, currentID, h.hs.relativeTimeTicker) | |
| 235 | + | assert.Zero(t, h.runIdle(), "stale ticker should not schedule refresh") | |
| 236 | + | ||
| 237 | + | keep = currentCB(0) | |
| 238 | + | assert.True(t, keep, "current ticker should keep running while visible") | |
| 239 | + | assert.Equal(t, 1, h.runIdle(), "current ticker schedules refresh") | |
| 240 | + | } | |
| 241 | + | ||
| 242 | + | func TestHistorySidebar_RelativeTimeDayBoundaryRequestsReloadNoRepoCall(t *testing.T) { | |
| 243 | + | h := newSidebarReactivityHarness() | |
| 244 | + | day1 := time.Date(2026, 6, 24, 23, 59, 0, 0, time.UTC) | |
| 245 | + | day2 := day1.Add(2 * time.Minute) | |
| 246 | + | now := day1 | |
| 247 | + | h.hs.now = func() time.Time { return now } | |
| 248 | + | h.hs.visible = true | |
| 249 | + | h.hs.relativeTimeDayKey = h.hs.currentDayKey() | |
| 250 | + | h.hs.relativeTimeDayKeySet = true | |
| 251 | + | h.hs.relativeTimeLabelBinds = []relativeTimeLabelBinding{{lastVisited: day1.Add(-time.Hour)}} | |
| 252 | + | now = day2 | |
| 253 | + | h.hs.updateRelativeTimeLabelsOnMain() | |
| 254 | + | assert.False(t, h.hs.loadStarted, "day boundary should not reload synchronously") | |
| 255 | + | assert.Equal(t, 1, h.runIdle(), "day boundary schedules debounced reload via RequestReloadIfVisible") | |
| 256 | + | assert.False(t, h.hs.loadStarted, "day boundary should wait for debounce before loading") | |
| 257 | + | assert.Equal(t, h.hs.currentDayKey(), h.hs.relativeTimeDayKey) | |
| 258 | + | assert.Len(t, h.timers, 1) | |
| 259 | + | } | |
| 260 | + | ||
| 261 | + | func TestRelativeTimeAtDeterministic(t *testing.T) { | |
| 262 | + | now := time.Date(2026, 6, 24, 12, 0, 0, 0, time.UTC) | |
| 263 | + | assert.Equal(t, "1m ago", relativeTimeAt(now.Add(-time.Minute), now)) | |
| 264 | + | assert.Equal(t, "2h ago", relativeTimeAt(now.Add(-2*time.Hour), now)) | |
| 265 | + | } | |
| 266 | + | ||
| 267 | + | func TestHistorySidebar_RelativeRefreshDoesNotTouchHistoryRepo(t *testing.T) { | |
| 268 | + | h := newSidebarReactivityHarness() | |
| 269 | + | h.hs.historyUC = nil | |
| 270 | + | h.hs.visible = true | |
| 271 | + | h.hs.relativeTimeLabelBinds = []relativeTimeLabelBinding{{lastVisited: h.hs.currentTime().Add(-time.Minute)}} | |
| 272 | + | h.hs.updateRelativeTimeLabelsOnMain() | |
| 273 | + | assert.Empty(t, h.timers, "label-only refresh does not schedule reload/repo work without day boundary") | |
| 274 | + | } | |
| 275 | + | ||
| 276 | + | func TestHistorySidebar_ReloadStillPreservesDisplayRowsSemantics(t *testing.T) { | |
| 277 | + | hs := newTestSidebarSearchHarness() | |
| 278 | + | entry := &entity.HistoryEntry{URL: "https://example.com", LastVisited: time.Now()} | |
| 279 | + | hs.setDisplayGroupsLocked(groupHistoryByDay([]*entity.HistoryEntry{entry})) | |
| 280 | + | require.Len(t, hs.displayRows, 2) | |
| 281 | + | assert.Equal(t, entry.URL, hs.entryURLAtIndex(1)) | |
| 282 | + | } | |
M internal/ui/component/history_sidebar_rendering.go +30 -30
| @@ -14,62 +14,61 @@ import ( | |||
| 14 | 14 | // rebuildList clears and repopulates the list box from current groups. | |
| 15 | 15 | // Must be called on the GTK main thread. Preserves scroll and selection. | |
| 16 | 16 | func (hs *HistorySidebar) rebuildList() { | |
| 17 | - | hs.mu.RLock() | |
| 17 | + | hs.mu.Lock() | |
| 18 | 18 | if hs.destroyed || hs.listBox == nil { | |
| 19 | - | hs.mu.RUnlock() | |
| 19 | + | hs.mu.Unlock() | |
| 20 | 20 | return | |
| 21 | 21 | } | |
| 22 | - | rows := hs.displayRows | |
| 22 | + | listBox := hs.listBox | |
| 23 | + | rows := append([]historyDisplayRow(nil), hs.displayRows...) | |
| 23 | 24 | query := hs.currentQuery | |
| 24 | 25 | hasSearchResults := hs.searchResults != nil | |
| 25 | 26 | searchDone := hs.searchDone | |
| 26 | 27 | totalLoaded := hs.totalLoaded | |
| 27 | - | hs.mu.RUnlock() | |
| 28 | + | isLoading := hs.isLoading | |
| 29 | + | hs.clearRelativeTimeBindingsLocked() | |
| 30 | + | hs.mu.Unlock() | |
| 28 | 31 | ||
| 29 | 32 | // Remove all rows | |
| 30 | - | hs.listBox.RemoveAll() | |
| 33 | + | listBox.RemoveAll() | |
| 31 | 34 | ||
| 32 | 35 | if len(rows) == 0 { | |
| 33 | 36 | if !hasSearchResults && totalLoaded == 0 { | |
| 34 | 37 | // Browse has not loaded yet AND no search has completed. | |
| 35 | - | hs.showLoadingOrEmpty(query, searchDone) | |
| 38 | + | hs.showLoadingOrEmpty(listBox, query, searchDone, isLoading) | |
| 36 | 39 | return | |
| 37 | 40 | } | |
| 38 | 41 | // Search completed with 0 results, or browse loaded but empty (no history). | |
| 39 | - | hs.showEmptyState(query) | |
| 40 | - | hs.restoreScrollAndSelection() | |
| 42 | + | hs.showEmptyState(listBox, query) | |
| 43 | + | hs.restoreScrollAndSelectionInListBox(listBox) | |
| 41 | 44 | return | |
| 42 | 45 | } | |
| 43 | 46 | ||
| 44 | 47 | for _, row := range rows { | |
| 45 | 48 | switch row.Kind { | |
| 46 | 49 | case historyDisplayRowHeader: | |
| 47 | - | hs.appendGroupHeader(row.Label) | |
| 50 | + | hs.appendGroupHeader(listBox, row.Label) | |
| 48 | 51 | case historyDisplayRowEntry: | |
| 49 | - | hs.appendEntryRow(row.Entry) | |
| 52 | + | hs.appendEntryRow(listBox, row.Entry) | |
| 50 | 53 | } | |
| 51 | 54 | } | |
| 52 | 55 | ||
| 53 | - | hs.listBox.Show() | |
| 56 | + | listBox.Show() | |
| 54 | 57 | ||
| 55 | 58 | // Restore previous scroll position and selection | |
| 56 | - | hs.restoreScrollAndSelection() | |
| 59 | + | hs.restoreScrollAndSelectionInListBox(listBox) | |
| 57 | 60 | ||
| 58 | 61 | // If no selection was restored and this is the first load, select first entry | |
| 59 | - | hs.ensureAtLeastOneSelection() | |
| 62 | + | hs.ensureAtLeastOneSelectionInListBox(listBox) | |
| 60 | 63 | } | |
| 61 | 64 | ||
| 62 | - | func (hs *HistorySidebar) showLoadingOrEmpty(query string, searchDone bool) { | |
| 65 | + | func (hs *HistorySidebar) showLoadingOrEmpty(listBox *gtk.ListBox, query string, searchDone, isLoading bool) { | |
| 63 | 66 | label := gtk.NewLabel(nil) | |
| 64 | 67 | if label == nil { | |
| 65 | 68 | return | |
| 66 | 69 | } | |
| 67 | 70 | label.AddCssClass("history-sidebar-loading") | |
| 68 | 71 | ||
| 69 | - | hs.mu.RLock() | |
| 70 | - | isLoading := hs.isLoading | |
| 71 | - | hs.mu.RUnlock() | |
| 72 | - | ||
| 73 | 72 | switch { | |
| 74 | 73 | case isLoading && query == "": | |
| 75 | 74 | label.SetText("Loading history...") | |
| @@ -92,14 +91,14 @@ func (hs *HistorySidebar) showLoadingOrEmpty(query string, searchDone bool) { | |||
| 92 | 91 | row.SetCanFocus(false) | |
| 93 | 92 | row.SetActivatable(false) | |
| 94 | 93 | row.SetChild(&label.Widget) | |
| 95 | - | hs.listBox.Append(&row.Widget) | |
| 94 | + | listBox.Append(&row.Widget) | |
| 96 | 95 | } | |
| 97 | 96 | ||
| 98 | 97 | func noResultsText(query string) string { | |
| 99 | 98 | return "No results for \"" + query + "\"" | |
| 100 | 99 | } | |
| 101 | 100 | ||
| 102 | - | func (hs *HistorySidebar) showEmptyState(query string) { | |
| 101 | + | func (hs *HistorySidebar) showEmptyState(listBox *gtk.ListBox, query string) { | |
| 103 | 102 | label := gtk.NewLabel(nil) | |
| 104 | 103 | if label == nil { | |
| 105 | 104 | return | |
| @@ -123,11 +122,11 @@ func (hs *HistorySidebar) showEmptyState(query string) { | |||
| 123 | 122 | row.SetCanFocus(false) | |
| 124 | 123 | row.SetActivatable(false) | |
| 125 | 124 | row.SetChild(&label.Widget) | |
| 126 | - | hs.listBox.Append(&row.Widget) | |
| 125 | + | listBox.Append(&row.Widget) | |
| 127 | 126 | } | |
| 128 | 127 | ||
| 129 | 128 | // appendGroupHeader adds a non-selectable group header label to the list. | |
| 130 | - | func (hs *HistorySidebar) appendGroupHeader(labelText string) { | |
| 129 | + | func (hs *HistorySidebar) appendGroupHeader(listBox *gtk.ListBox, labelText string) { | |
| 131 | 130 | label := gtk.NewLabel(&labelText) | |
| 132 | 131 | if label == nil { | |
| 133 | 132 | return | |
| @@ -144,11 +143,11 @@ func (hs *HistorySidebar) appendGroupHeader(labelText string) { | |||
| 144 | 143 | row.SetCanFocus(false) | |
| 145 | 144 | row.SetActivatable(false) | |
| 146 | 145 | row.SetChild(&label.Widget) | |
| 147 | - | hs.listBox.Append(&row.Widget) | |
| 146 | + | listBox.Append(&row.Widget) | |
| 148 | 147 | } | |
| 149 | 148 | ||
| 150 | 149 | // appendEntryRow adds a selectable two-line entry row to the list. | |
| 151 | - | func (hs *HistorySidebar) appendEntryRow(entry *entity.HistoryEntry) { | |
| 150 | + | func (hs *HistorySidebar) appendEntryRow(listBox *gtk.ListBox, entry *entity.HistoryEntry) { | |
| 152 | 151 | if entry == nil { | |
| 153 | 152 | return | |
| 154 | 153 | } | |
| @@ -192,7 +191,7 @@ func (hs *HistorySidebar) appendEntryRow(entry *entity.HistoryEntry) { | |||
| 192 | 191 | return | |
| 193 | 192 | } | |
| 194 | 193 | timeLabel.AddCssClass("history-sidebar-row-time") | |
| 195 | - | timeLabel.SetText(relativeTime(entry.LastVisited)) | |
| 194 | + | timeLabel.SetText(relativeTimeAt(entry.LastVisited, hs.currentTime())) | |
| 196 | 195 | timeLabel.SetXalign(1.0) | |
| 197 | 196 | ||
| 198 | 197 | subBox.Append(&urlLabel.Widget) | |
| @@ -213,21 +212,22 @@ func (hs *HistorySidebar) appendEntryRow(entry *entity.HistoryEntry) { | |||
| 213 | 212 | row.SetFocusOnClick(true) | |
| 214 | 213 | row.SetChild(&rowBox.Widget) | |
| 215 | 214 | ||
| 216 | - | hs.listBox.Append(&row.Widget) | |
| 215 | + | listBox.Append(&row.Widget) | |
| 216 | + | hs.bindRelativeTimeLabel(timeLabel, entry.LastVisited) | |
| 217 | 217 | } | |
| 218 | 218 | ||
| 219 | 219 | // ensureAtLeastOneSelection selects the first selectable row if nothing is selected. | |
| 220 | - | func (hs *HistorySidebar) ensureAtLeastOneSelection() { | |
| 221 | - | if hs.listBox.GetSelectedRow() != nil { | |
| 220 | + | func (hs *HistorySidebar) ensureAtLeastOneSelectionInListBox(listBox *gtk.ListBox) { | |
| 221 | + | if listBox == nil || listBox.GetSelectedRow() != nil { | |
| 222 | 222 | return | |
| 223 | 223 | } | |
| 224 | 224 | for i := 0; ; i++ { | |
| 225 | - | row := hs.listBox.GetRowAtIndex(i) | |
| 225 | + | row := listBox.GetRowAtIndex(i) | |
| 226 | 226 | if row == nil { | |
| 227 | 227 | break | |
| 228 | 228 | } | |
| 229 | 229 | if row.GetSelectable() { | |
| 230 | - | hs.listBox.SelectRow(row) | |
| 230 | + | listBox.SelectRow(row) | |
| 231 | 231 | return | |
| 232 | 232 | } | |
| 233 | 233 | } | |
M internal/ui/coordinator/navigation.go +20 -8
| @@ -22,6 +22,7 @@ type OmniboxProvider interface { | |||
| 22 | 22 | type NavigationCoordinator struct { | |
| 23 | 23 | contextProvider func() context.Context | |
| 24 | 24 | navigateUC *usecase.NavigateUseCase | |
| 25 | + | historyRecorder *usecase.HistoryRecorderUseCase | |
| 25 | 26 | contentCoord *content.Coordinator | |
| 26 | 27 | omniboxProvider OmniboxProvider | |
| 27 | 28 | } | |
| @@ -33,6 +34,16 @@ func NewNavigationCoordinator( | |||
| 33 | 34 | ctx context.Context, | |
| 34 | 35 | navigateUC *usecase.NavigateUseCase, | |
| 35 | 36 | contentCoord *content.Coordinator, | |
| 37 | + | ) *NavigationCoordinator { | |
| 38 | + | return NewNavigationCoordinatorWithHistoryRecorder(ctx, navigateUC, nil, contentCoord) | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | // NewNavigationCoordinatorWithHistoryRecorder creates a new NavigationCoordinator with history recording. | |
| 42 | + | func NewNavigationCoordinatorWithHistoryRecorder( | |
| 43 | + | ctx context.Context, | |
| 44 | + | navigateUC *usecase.NavigateUseCase, | |
| 45 | + | historyRecorder *usecase.HistoryRecorderUseCase, | |
| 46 | + | contentCoord *content.Coordinator, | |
| 36 | 47 | ) *NavigationCoordinator { | |
| 37 | 48 | log := logging.FromContext(ctx) | |
| 38 | 49 | log.Debug().Msg("creating navigation coordinator") | |
| @@ -42,8 +53,9 @@ func NewNavigationCoordinator( | |||
| 42 | 53 | contextProvider: func() context.Context { | |
| 43 | 54 | return logging.WithContext(context.Background(), callbackLogger) | |
| 44 | 55 | }, | |
| 45 | - | navigateUC: navigateUC, | |
| 46 | - | contentCoord: contentCoord, | |
| 56 | + | navigateUC: navigateUC, | |
| 57 | + | historyRecorder: historyRecorder, | |
| 58 | + | contentCoord: contentCoord, | |
| 47 | 59 | } | |
| 48 | 60 | } | |
| 49 | 61 | ||
| @@ -239,28 +251,28 @@ func (c *NavigationCoordinator) PrintWebView(ctx context.Context, wv port.WebVie | |||
| 239 | 251 | ||
| 240 | 252 | // UpdateHistoryTitle updates the title of a history entry after page load. | |
| 241 | 253 | func (c *NavigationCoordinator) UpdateHistoryTitle(ctx context.Context, paneID entity.PaneID, url, title string) { | |
| 242 | - | if c.navigateUC == nil { | |
| 254 | + | if c.historyRecorder == nil { | |
| 243 | 255 | return | |
| 244 | 256 | } | |
| 245 | 257 | ||
| 246 | - | c.navigateUC.UpdateHistoryTitle(ctx, url, title) | |
| 258 | + | c.historyRecorder.UpdateHistoryTitle(ctx, url, title) | |
| 247 | 259 | } | |
| 248 | 260 | ||
| 249 | 261 | // RecordHistory records a URL in history on page commit. | |
| 250 | 262 | func (c *NavigationCoordinator) RecordHistory(ctx context.Context, paneID entity.PaneID, url string) { | |
| 251 | - | if c.navigateUC == nil { | |
| 263 | + | if c.historyRecorder == nil { | |
| 252 | 264 | return | |
| 253 | 265 | } | |
| 254 | 266 | ||
| 255 | - | c.navigateUC.RecordHistory(ctx, string(paneID), url) | |
| 267 | + | c.historyRecorder.RecordHistory(ctx, string(paneID), url) | |
| 256 | 268 | } | |
| 257 | 269 | ||
| 258 | 270 | // ClearPaneHistory clears per-pane navigation history deduplication state. | |
| 259 | 271 | func (c *NavigationCoordinator) ClearPaneHistory(paneID entity.PaneID) { | |
| 260 | - | if c.navigateUC == nil { | |
| 272 | + | if c.historyRecorder == nil { | |
| 261 | 273 | return | |
| 262 | 274 | } | |
| 263 | - | c.navigateUC.ClearPaneHistory(string(paneID)) | |
| 275 | + | c.historyRecorder.ClearPaneHistory(string(paneID)) | |
| 264 | 276 | } | |
| 265 | 277 | ||
| 266 | 278 | // NotifyZoomChanged updates the omnibox zoom indicator. | |
M internal/ui/deps.go +9 -8
| @@ -51,14 +51,15 @@ type Dependencies struct { | |||
| 51 | 51 | FilterRepo repository.ContentWhitelistRepository | |
| 52 | 52 | ||
| 53 | 53 | // Use Cases | |
| 54 | - | TabsUC *usecase.ManageTabsUseCase | |
| 55 | - | PanesUC *usecase.ManagePanesUseCase | |
| 56 | - | NavigateUC *usecase.NavigateUseCase | |
| 57 | - | ZoomUC *usecase.ManageZoomUseCase | |
| 58 | - | PermissionUC *usecase.HandlePermissionUseCase | |
| 59 | - | FavoritesUC *usecase.ManageFavoritesUseCase | |
| 60 | - | HistoryUC *usecase.SearchHistoryUseCase | |
| 61 | - | CopyURLUC *usecase.CopyURLUseCase | |
| 54 | + | TabsUC *usecase.ManageTabsUseCase | |
| 55 | + | PanesUC *usecase.ManagePanesUseCase | |
| 56 | + | NavigateUC *usecase.NavigateUseCase | |
| 57 | + | HistoryRecorderUC *usecase.HistoryRecorderUseCase | |
| 58 | + | ZoomUC *usecase.ManageZoomUseCase | |
| 59 | + | PermissionUC *usecase.HandlePermissionUseCase | |
| 60 | + | FavoritesUC *usecase.ManageFavoritesUseCase | |
| 61 | + | HistoryUC *usecase.SearchHistoryUseCase | |
| 62 | + | CopyURLUC *usecase.CopyURLUseCase | |
| 62 | 63 | ||
| 63 | 64 | // Infrastructure Adapters | |
| 64 | 65 | Clipboard port.Clipboard | |
A internal/ui/history_changes.go +72 -0
| @@ -0,0 +1,72 @@ | |||
| 1 | + | package ui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | ||
| 6 | + | "github.com/bnema/dumber/internal/application/dto" | |
| 7 | + | "github.com/bnema/dumber/internal/application/port" | |
| 8 | + | "github.com/bnema/dumber/internal/logging" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | var _ port.HistoryChangeSink = (*historyChangeAdapter)(nil) | |
| 12 | + | ||
| 13 | + | type historySidebarReloader interface { | |
| 14 | + | RequestReloadIfVisible(reason string) | |
| 15 | + | } | |
| 16 | + | ||
| 17 | + | type historyChangeAdapter struct { | |
| 18 | + | app *App | |
| 19 | + | } | |
| 20 | + | ||
| 21 | + | func newHistoryChangeAdapter(app *App) *historyChangeAdapter { | |
| 22 | + | return &historyChangeAdapter{app: app} | |
| 23 | + | } | |
| 24 | + | ||
| 25 | + | func (h *historyChangeAdapter) OnHistoryChanged(ctx context.Context, change dto.HistoryChange) { | |
| 26 | + | if h == nil || h.app == nil { | |
| 27 | + | return | |
| 28 | + | } | |
| 29 | + | app := h.app | |
| 30 | + | run := func() { | |
| 31 | + | app.refreshVisibleHistorySidebars(change) | |
| 32 | + | } | |
| 33 | + | if app.dispatchOnMainThread == nil { | |
| 34 | + | logging.FromContext(ctx).Warn(). | |
| 35 | + | Msg("history sidebar refresh skipped: main-thread dispatcher unavailable") | |
| 36 | + | return | |
| 37 | + | } | |
| 38 | + | result := app.dispatchOnMainThread("ui.history_sidebar_refresh", run) | |
| 39 | + | if !result.Completed() { | |
| 40 | + | logging.FromContext(ctx).Warn(). | |
| 41 | + | Str("status", string(result.Status)). | |
| 42 | + | Dur("elapsed", result.Elapsed). | |
| 43 | + | Msg("history sidebar refresh dispatch did not complete") | |
| 44 | + | } | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | func (a *App) refreshVisibleHistorySidebars(change dto.HistoryChange) { | |
| 48 | + | if a == nil { | |
| 49 | + | return | |
| 50 | + | } | |
| 51 | + | reason := historyChangeReasonLabel(change) | |
| 52 | + | for _, bw := range a.browserWindows { | |
| 53 | + | if bw == nil || !bw.sidebarVisible { | |
| 54 | + | continue | |
| 55 | + | } | |
| 56 | + | reloader := bw.historySidebarReloader | |
| 57 | + | if reloader == nil { | |
| 58 | + | reloader = bw.historySidebar | |
| 59 | + | } | |
| 60 | + | if reloader == nil { | |
| 61 | + | continue | |
| 62 | + | } | |
| 63 | + | reloader.RequestReloadIfVisible(reason) | |
| 64 | + | } | |
| 65 | + | } | |
| 66 | + | ||
| 67 | + | func historyChangeReasonLabel(change dto.HistoryChange) string { | |
| 68 | + | if len(change.Reasons) == 0 { | |
| 69 | + | return "history-change" | |
| 70 | + | } | |
| 71 | + | return string(change.Reasons[0]) | |
| 72 | + | } | |