-
Notifications
You must be signed in to change notification settings - Fork 60k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Enhance cloud synchronization functionality, support syncing delete operations for messages and conversations, and add support for automatic sync settings #5236
base: main
Are you sure you want to change the base?
Changes from 4 commits
1d0a40b
78c4084
1cce87a
cd354cf
d957397
284d33b
c440637
22f6129
5065091
22c7959
faac0d9
4f876f3
648e600
93bfb55
4b22aaf
621b148
eae593d
2ee2d50
5e1064a
0a6ddda
b2336f5
31f2829
e515f0f
fdb89af
fc97c4b
0745b64
d0b7ddc
5c51fd2
2fdb35b
31baa10
f1d69cb
2d68f17
0638db1
e8c7ac0
2bf72d0
c204031
ccacfec
6dc8681
6f3d753
5ae4921
370ce3e
9551f5d
35f5288
144fdc9
659a389
60bd3c5
89edebd
41242ca
cf7c6f2
c6657d3
31900cb
c4ae73d
9a025ae
98ab561
f80da8a
3e02a71
f45a693
7f3ec6d
dfd3d24
af23929
9a95d32
b2381b2
bf5cdc9
e3a2e78
36edbcd
9f58a66
36525d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -64,6 +64,7 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
getMessageImages, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
isVisionModel, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
isDalle3, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
removeOutdatedEntries, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} from "../utils"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { uploadImage as uploadImageRemote } from "@/app/utils/chat"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -1023,10 +1024,20 @@ function _Chat() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
const deleteMessage = (msgId?: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
chatStore.updateCurrentSession( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
(session) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
(session.messages = session.messages.filter((m) => m.id !== msgId)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
chatStore.updateCurrentSession((session) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
session.deletedMessageIds && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
removeOutdatedEntries(session.deletedMessageIds); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
session.messages = session.messages.filter((m) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (m.id !== msgId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!session.deletedMessageIds) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
session.deletedMessageIds = {} as Record<string, number>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
session.deletedMessageIds[m.id] = Date.now(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Refactor to avoid side effects within the Modifying Consider refactoring the code to separate the side effects from the filtering logic: chatStore.updateCurrentSession((session) => {
+ if (!session.deletedMessageIds) {
+ session.deletedMessageIds = {} as Record<string, number>;
+ }
+ session.deletedMessageIds && removeOutdatedEntries(session.deletedMessageIds);
- session.messages = session.messages.filter((m) => {
- if (m.id !== msgId) {
- return true;
- }
- session.deletedMessageIds[m.id] = Date.now();
- return false;
- });
+ session.messages = session.messages.filter((m) => m.id !== msgId);
+
+ if (msgId) {
+ session.deletedMessageIds[msgId] = Date.now();
+ }
}); This refactoring improves readability by:
📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
const onDelete = (msgId: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -357,6 +357,21 @@ function SyncConfigModal(props: { onClose?: () => void }) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</select> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</ListItem> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<ListItem | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
title={Locale.Settings.Sync.Config.EnableAutoSync.Title} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
subTitle={Locale.Settings.Sync.Config.EnableAutoSync.SubTitle} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<input | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type="checkbox" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
checked={syncStore.enableAutoSync} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onChange={(e) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
syncStore.update( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(config) => (config.enableAutoSync = e.currentTarget.checked), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+374
to
+376
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid assignments within expressions. The assignment within the expression can lead to confusion and potential side effects. Consider refactoring to separate the assignment from the update function call. - syncStore.update(
- (config) => (config.enableAutoSync = e.currentTarget.checked),
- );
+ const enableAutoSync = e.currentTarget.checked;
+ syncStore.update((config) => {
+ config.enableAutoSync = enableAutoSync;
+ }); Committable suggestion
Suggested change
ToolsBiome
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
></input> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</ListItem> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+366
to
+379
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactor the assignment within the expression. The implementation of the automatic synchronization toggle looks good overall. However, there's a minor issue with the assignment within the expression in the To improve readability and avoid potential side effects, consider refactoring the onChange={(e) => {
- syncStore.update(
- (config) => (config.enableAutoSync = e.currentTarget.checked),
- );
+ const enableAutoSync = e.currentTarget.checked;
+ syncStore.update((config) => {
+ config.enableAutoSync = enableAutoSync;
+ });
}} This change separates the assignment from the update function call, making the code clearer and less prone to unexpected behavior. The overall implementation of the automatic synchronization feature is well-integrated into the existing UI and follows the established patterns in the codebase. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<ListItem | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
title={Locale.Settings.Sync.Config.Proxy.Title} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
subTitle={Locale.Settings.Sync.Config.Proxy.SubTitle} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,4 +1,8 @@ | ||||||
import { trimTopic, getMessageTextContent } from "../utils"; | ||||||
import { | ||||||
trimTopic, | ||||||
getMessageTextContent, | ||||||
removeOutdatedEntries, | ||||||
} from "../utils"; | ||||||
|
||||||
import Locale, { getLang } from "../locales"; | ||||||
import { showToast } from "../components/ui-lib"; | ||||||
|
@@ -26,6 +30,7 @@ import { nanoid } from "nanoid"; | |||||
import { createPersistStore } from "../utils/store"; | ||||||
import { collectModelsWithDefaultModel } from "../utils/model"; | ||||||
import { useAccessStore } from "./access"; | ||||||
import { useSyncStore } from "./sync"; | ||||||
import { isDalle3 } from "../utils"; | ||||||
|
||||||
export type ChatMessage = RequestMessage & { | ||||||
|
@@ -62,6 +67,7 @@ export interface ChatSession { | |||||
lastUpdate: number; | ||||||
lastSummarizeIndex: number; | ||||||
clearContextIndex?: number; | ||||||
deletedMessageIds?: Record<string, number>; | ||||||
|
||||||
mask: Mask; | ||||||
} | ||||||
|
@@ -85,6 +91,7 @@ function createEmptySession(): ChatSession { | |||||
}, | ||||||
lastUpdate: Date.now(), | ||||||
lastSummarizeIndex: 0, | ||||||
deletedMessageIds: {}, | ||||||
|
||||||
mask: createEmptyMask(), | ||||||
}; | ||||||
|
@@ -162,9 +169,19 @@ function fillTemplateWith(input: string, modelConfig: ModelConfig) { | |||||
return output; | ||||||
} | ||||||
|
||||||
let cloudSyncTimer: any = null; | ||||||
function noticeCloudSync(): void { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Specify a more precise type for Using the Apply this diff to specify a more precise type: -let cloudSyncTimer: any = null;
+let cloudSyncTimer: ReturnType<typeof setTimeout> | null = null; 📝 Committable suggestion
Suggested change
|
||||||
const syncStore = useSyncStore.getState(); | ||||||
cloudSyncTimer && clearTimeout(cloudSyncTimer); | ||||||
cloudSyncTimer = setTimeout(() => { | ||||||
syncStore.autoSync(); | ||||||
}, 500); | ||||||
} | ||||||
|
||||||
const DEFAULT_CHAT_STATE = { | ||||||
sessions: [createEmptySession()], | ||||||
currentSessionIndex: 0, | ||||||
deletedSessionIds: {} as Record<string, number>, | ||||||
}; | ||||||
|
||||||
export const useChatStore = createPersistStore( | ||||||
|
@@ -253,7 +270,18 @@ export const useChatStore = createPersistStore( | |||||
if (!deletedSession) return; | ||||||
|
||||||
const sessions = get().sessions.slice(); | ||||||
sessions.splice(index, 1); | ||||||
const deletedSessionIds = { ...get().deletedSessionIds }; | ||||||
|
||||||
removeOutdatedEntries(deletedSessionIds); | ||||||
|
||||||
const hasDelSessions = sessions.splice(index, 1); | ||||||
if (hasDelSessions?.length) { | ||||||
hasDelSessions.forEach((session) => { | ||||||
if (session.messages.length > 0) { | ||||||
deletedSessionIds[session.id] = Date.now(); | ||||||
} | ||||||
}); | ||||||
} | ||||||
|
||||||
const currentIndex = get().currentSessionIndex; | ||||||
let nextIndex = Math.min( | ||||||
|
@@ -270,19 +298,24 @@ export const useChatStore = createPersistStore( | |||||
const restoreState = { | ||||||
currentSessionIndex: get().currentSessionIndex, | ||||||
sessions: get().sessions.slice(), | ||||||
deletedSessionIds: get().deletedSessionIds, | ||||||
}; | ||||||
|
||||||
set(() => ({ | ||||||
currentSessionIndex: nextIndex, | ||||||
sessions, | ||||||
deletedSessionIds, | ||||||
})); | ||||||
|
||||||
noticeCloudSync(); | ||||||
|
||||||
showToast( | ||||||
Locale.Home.DeleteToast, | ||||||
{ | ||||||
text: Locale.Home.Revert, | ||||||
onClick() { | ||||||
set(() => restoreState); | ||||||
noticeCloudSync(); | ||||||
}, | ||||||
}, | ||||||
5000, | ||||||
|
@@ -310,6 +343,7 @@ export const useChatStore = createPersistStore( | |||||
}); | ||||||
get().updateStat(message); | ||||||
get().summarizeSession(); | ||||||
noticeCloudSync(); | ||||||
}, | ||||||
|
||||||
async onUserInput(content: string, attachImages?: string[]) { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -26,6 +26,7 @@ export type SyncStore = GetStoreState<typeof useSyncStore>; | |||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const DEFAULT_SYNC_STATE = { | ||||||||||||||||||||||||||||
provider: ProviderType.WebDAV, | ||||||||||||||||||||||||||||
enableAutoSync: true, | ||||||||||||||||||||||||||||
useProxy: true, | ||||||||||||||||||||||||||||
proxyUrl: corsPath(ApiPath.Cors), | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
@@ -91,6 +92,11 @@ export const useSyncStore = createPersistStore( | |||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
async sync() { | ||||||||||||||||||||||||||||
const enableAutoSync = get().enableAutoSync; | ||||||||||||||||||||||||||||
if (!enableAutoSync) { | ||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const localState = getLocalAppState(); | ||||||||||||||||||||||||||||
const provider = get().provider; | ||||||||||||||||||||||||||||
const config = get()[provider]; | ||||||||||||||||||||||||||||
|
@@ -100,15 +106,17 @@ export const useSyncStore = createPersistStore( | |||||||||||||||||||||||||||
const remoteState = await client.get(config.username); | ||||||||||||||||||||||||||||
if (!remoteState || remoteState === "") { | ||||||||||||||||||||||||||||
await client.set(config.username, JSON.stringify(localState)); | ||||||||||||||||||||||||||||
console.log("[Sync] Remote state is empty, using local state instead."); | ||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||
console.log( | ||||||||||||||||||||||||||||
"[Sync] Remote state is empty, using local state instead.", | ||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||
const parsedRemoteState = JSON.parse( | ||||||||||||||||||||||||||||
await client.get(config.username), | ||||||||||||||||||||||||||||
) as AppState; | ||||||||||||||||||||||||||||
mergeAppState(localState, parsedRemoteState); | ||||||||||||||||||||||||||||
setLocalAppState(localState); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} catch (e) { | ||||||||||||||||||||||||||||
console.log("[Sync] failed to get remote state", e); | ||||||||||||||||||||||||||||
throw e; | ||||||||||||||||||||||||||||
|
@@ -123,6 +131,14 @@ export const useSyncStore = createPersistStore( | |||||||||||||||||||||||||||
const client = this.getClient(); | ||||||||||||||||||||||||||||
return await client.check(); | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
async autoSync() { | ||||||||||||||||||||||||||||
const { lastSyncTime, provider } = get(); | ||||||||||||||||||||||||||||
const syncStore = useSyncStore.getState(); | ||||||||||||||||||||||||||||
if (lastSyncTime && syncStore.cloudSync()) { | ||||||||||||||||||||||||||||
syncStore.sync(); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
Comment on lines
+138
to
+144
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Simplify 'autoSync' method by using 'this' context and 'get()' Currently, the Consider refactoring async autoSync() {
- const { lastSyncTime, provider } = get();
- const syncStore = useSyncStore.getState();
- if (lastSyncTime && syncStore.cloudSync()) {
- syncStore.sync();
+ const { lastSyncTime } = get();
+ if (lastSyncTime && this.cloudSync()) {
+ await this.sync();
}
} This refactor makes the code more consistent and leverages the 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
name: StoreKey.Sync, | ||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -270,3 +270,16 @@ export function isVisionModel(model: string) { | |||||||||||||||||||||||||||||||||||||||||||||||||||
export function isDalle3(model: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
return "dall-e-3" === model; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
export function removeOutdatedEntries( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
timeMap: Record<string, number>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
): Record<string, number> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const oneMonthAgo = Date.now() - 30 * 24 * 60 * 60 * 1000; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
// Delete data from a month ago | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Object.keys(timeMap).forEach((id) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
if (timeMap[id] < oneMonthAgo) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
delete timeMap[id]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
return timeMap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+274
to
+285
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider enhancing type safety and robustness. While the basic functionality is correct, consider these improvements:
Here's a more robust implementation: export function removeOutdatedEntries(
- timeMap: Record<string, number>,
+ timeMap: Record<string, number>,
): Record<string, number> {
const oneMonthAgo = Date.now() - 30 * 24 * 60 * 60 * 1000;
- // Delete data from a month ago
- Object.keys(timeMap).forEach((id) => {
- if (timeMap[id] < oneMonthAgo) {
- delete timeMap[id];
- }
- });
- return timeMap;
+ // Create a new object instead of mutating the input
+ return Object.entries(timeMap).reduce((acc, [id, timestamp]) => {
+ // Ensure timestamp is valid
+ if (typeof timestamp === 'number' && !isNaN(timestamp) && timestamp >= oneMonthAgo) {
+ acc[id] = timestamp;
+ }
+ return acc;
+ }, {} as Record<string, number>);
} This implementation:
📝 Committable suggestion
Suggested change
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initialize
deletedMessageIds
before usageIn the
deleteMessage
function,session.deletedMessageIds
is used before it's ensured to be initialized. Ifsession.deletedMessageIds
is undefined, callingremoveOutdatedEntries(session.deletedMessageIds);
could lead to errors. It's important to initializesession.deletedMessageIds
before using it to prevent potential runtime exceptions.Please apply the following diff to initialize
deletedMessageIds
before use:📝 Committable suggestion