Skip to content
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

fix palette cloud sync #4061

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix palette cloud sync #4061

wants to merge 3 commits into from

Conversation

Skalii
Copy link
Contributor

@Skalii Skalii commented Oct 15, 2024

No description provided.

sync cloud/items code with android;
fix crash after starting sync with cloud;
minor fixes;
@@ -4037,8 +4037,21 @@ - (instancetype) init
[_profilePreferences setObject:_locationIcon forKey:@"location_icon"];

_appModeOrder = [OACommonInteger withKey:appModeOrderKey defValue:0];
[_appModeOrder setModeDefaultValue:@0 mode:OAApplicationMode.DEFAULT];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NSArray *modes = @[
OAApplicationMode.DEFAULT,
OAApplicationMode.CAR,
...
];
and for (NSInteger i = 0; i < modes.count; i++) {}

import Foundation

@objcMembers
final class BackupUtils: NSObject {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this class? is it possible to drop it and use existing classes like OABackupDbHelper and OABackupHelper?

@@ -67,7 +67,7 @@ - (BOOL)shouldShowDuplicates

- (long)localModifiedTime
{
return _historyMarkersHelper.getMarkersHistoryLastModifiedTime;
return [_historyMarkersHelper getMarkersHistoryLastModifiedTime];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@@ -93,7 +93,9 @@ - (void) apply
}

for (OAHistoryItem *historyItem in self.appliedItems)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

UserDefaults.standard.set(fileHashes.asDictionary(), forKey: Self.key)
}

func isHashUpdated(_ localFile: OALocalFile) -> Bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this method should have only return OAUtilities.fileMD5(localFile.filePath) != fileHashes.getValue(forKey: localFile.filePath)

@@ -85,11 +86,12 @@ - (OABackupInfo *) doInBackground
{
BOOL fileChangedLocally = localFile.localModifiedTime > (localFile.uploadTime / 1000);
BOOL fileChangedRemotely = remoteFile.updatetimems > localFile.uploadTime;
BOOL needToUpload = [hashHelper isHashUpdated:localFile];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use additional variable. fileChangedLocally seems to be enough.
BTW - may we use existing needMd5Digest / getMd5Digest / setMd5Digest in this issue?

if (exportType == nil || ![OAExportSettingsType isTypeEnabled:exportType])
continue;

BOOL hasRemoteFile = _uniqueRemoteFiles[localFile.getTypeFileName] != nil;
BOOL fileToDelete = [info.localFilesToDelete containsObject:localFile];
if (!hasRemoteFile && !fileToDelete)
BOOL needToUpload = [hashHelper isHashUpdated:localFile];
if (!hasRemoteFile && !fileToDelete && needToUpload)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like not needed if fileChangedLocally will be used

}

func setHash(withFileItem fileItem: OAFileSettingsItem) {
guard fileItem.subtype == .subtypeColorPalette else { return }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class should not check types directly. Should be done via SettingsItem, like with needMd5Digest

return exists
}

static func updateCacheForItems(_ items: [OASettingsItem]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be better name? What is cache in this method?

@@ -179,6 +180,7 @@ - (void)onSettingsImportFinished:(BOOL)succeed items:(NSArray<OASettingsItem *>
{
if (succeed)
{
[BackupUtils updateCacheForItems:items];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double call?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cloud sync between two freshly installed apps on iPadOS and iOS produces sync errors / warnings
3 participants