Skip to content

Commit

Permalink
Added support for new page index formats (#795)
Browse files Browse the repository at this point in the history
  • Loading branch information
michalrentka authored Oct 27, 2023
1 parent 6cd9bed commit e320953
Show file tree
Hide file tree
Showing 13 changed files with 189 additions and 27 deletions.
4 changes: 4 additions & 0 deletions Zotero.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@
B34E3DEC28CF490700B747AE /* PDFExportSettingsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = B34E3DEB28CF490700B747AE /* PDFExportSettingsView.swift */; };
B34F3566275A46BC008B9C76 /* RedirectWebViewHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = B34F3565275A46BC008B9C76 /* RedirectWebViewHandler.swift */; };
B3501F5425139B40007961DB /* Rounding+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = B340692124A60D6A009ECE48 /* Rounding+Extensions.swift */; };
B3518DA72AEBCB5E00D983B4 /* SettingsResponseSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3518DA62AEBCB5E00D983B4 /* SettingsResponseSpec.swift */; };
B351BD0E25EF7E78000451E2 /* ItemAction.swift in Sources */ = {isa = PBXBuildFile; fileRef = B351BD0D25EF7E78000451E2 /* ItemAction.swift */; };
B353F1FB242E21880062EE24 /* ResetTranslatorsDbRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B353F1FA242E21880062EE24 /* ResetTranslatorsDbRequest.swift */; };
B353F1FC242E23680062EE24 /* ResetTranslatorsDbRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B353F1FA242E21880062EE24 /* ResetTranslatorsDbRequest.swift */; };
Expand Down Expand Up @@ -1554,6 +1555,7 @@
B34F3565275A46BC008B9C76 /* RedirectWebViewHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RedirectWebViewHandler.swift; sourceTree = "<group>"; };
B34F9FA223743C36004ED34C /* CreatorSummaryFormatterSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreatorSummaryFormatterSpec.swift; sourceTree = "<group>"; };
B34F9FA423743C42004ED34C /* ItemTitleFormatterSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ItemTitleFormatterSpec.swift; sourceTree = "<group>"; };
B3518DA62AEBCB5E00D983B4 /* SettingsResponseSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsResponseSpec.swift; sourceTree = "<group>"; };
B351BD0D25EF7E78000451E2 /* ItemAction.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ItemAction.swift; sourceTree = "<group>"; };
B353F1FA242E21880062EE24 /* ResetTranslatorsDbRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ResetTranslatorsDbRequest.swift; sourceTree = "<group>"; };
B353F202242E52610062EE24 /* Database.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Database.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2674,6 +2676,7 @@
B34F9FA423743C42004ED34C /* ItemTitleFormatterSpec.swift */,
B39C7BDD251237D600C2CCF1 /* ReadUpdatedItemUpdateParametersSpec.swift */,
B3B1EDF0250242F700D8BC1E /* SearchResponseSpec.swift */,
B3518DA62AEBCB5E00D983B4 /* SettingsResponseSpec.swift */,
B3B1C58526651E2A00883597 /* StorageSettingsActionHandlerSpec.swift */,
B307E55D22D4A87B00592B3C /* SyncActionsSpec.swift */,
B34F274E220E20370038B3B1 /* SyncControllerSpec.swift */,
Expand Down Expand Up @@ -5192,6 +5195,7 @@
6144B5D42A4ADCDE00914B3C /* CreatorSummaryFormatterSpec.swift in Sources */,
B3F4E4EC2A4DAA7D00820718 /* UpdatableObjectSpec.swift in Sources */,
6144B5E12A4AE95E00914B3C /* WebDavControllerSpec.swift in Sources */,
B3518DA72AEBCB5E00D983B4 /* SettingsResponseSpec.swift in Sources */,
6144B5D82A4ADDC400914B3C /* ItemTitleFormatterSpec.swift in Sources */,
6144B5DF2A4AE48F00914B3C /* SyncControllerSpec.swift in Sources */,
B3243BC82A5EB2740033A7D6 /* HtmlAttributedStringConverterSpec.swift in Sources */,
Expand Down
1 change: 1 addition & 0 deletions Zotero/Assets/en.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@
"errors.pdf.cant_update_annotation" = "Can't update annotation.";
"errors.pdf.cant_add_annotations" = "Can't add annotations.";
"errors.pdf.cant_delete_annotations" = "Can't delete annotations.";
"errors.pdf.page_index_not_int" = "Incorrect format of page stored for this document.";

"accessibility.untitled" = "Untitled";
"accessibility.locked" = "Locked";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import Foundation
import RealmSwift

struct ReadDocumentDataDbRequest: DbResponseRequest {
typealias Response = Int
typealias Response = String

let attachmentKey: String
let libraryId: LibraryIdentifier

var needsWrite: Bool { return false }

func process(in database: Realm) throws -> Int {
guard let pageIndex = database.objects(RPageIndex.self).filter(.key(self.attachmentKey, in: self.libraryId)).first else { return 0 }
func process(in database: Realm) throws -> String {
guard let pageIndex = database.objects(RPageIndex.self).filter(.key(self.attachmentKey, in: self.libraryId)).first else { return "0" }
return pageIndex.index
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import RealmSwift
struct StorePageForItemDbRequest: DbRequest {
let key: String
let libraryId: LibraryIdentifier
let page: Int
let page: String

var needsWrite: Bool { return true }

Expand Down
2 changes: 2 additions & 0 deletions Zotero/Extensions/Localizable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,8 @@ internal enum L10n {
internal static let mergeTooBig = L10n.tr("Localizable", "errors.pdf.merge_too_big", fallback: "The combined annotation would be too large.")
/// Unable to merge annotations
internal static let mergeTooBigTitle = L10n.tr("Localizable", "errors.pdf.merge_too_big_title", fallback: "Unable to merge annotations")
/// Incorrect format of page stored for this document.
internal static let pageIndexNotInt = L10n.tr("Localizable", "errors.pdf.page_index_not_int", fallback: "Incorrect format of page stored for this document.")
}
internal enum Settings {
/// Could not collect storage data
Expand Down
16 changes: 14 additions & 2 deletions Zotero/Models/API/SettingsResponse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct PageIndicesResponse {

struct PageIndexResponse {
let key: String
let value: Int
let value: String
let version: Int
let libraryId: LibraryIdentifier

Expand All @@ -48,11 +48,23 @@ struct PageIndexResponse {
let (key, libraryId) = try PageIndexResponse.parse(key: key)

self.key = key
self.value = try Parsing.parse(key: "value", from: dictionary, caller: Self.self)
self.value = try Self.parseValue(from: dictionary)
self.version = try Parsing.parse(key: "version", from: dictionary, caller: Self.self)
self.libraryId = libraryId
}

private static func parseValue(from dictionary: [String: Any]) throws -> String {
// Value can be Int, Double or String, so we convert all values to String which can be stored in database
if let value = dictionary["value"] as? Int {
return "\(value)"
} else if let value = dictionary["value"] as? Double {
return "\(Decimal(value).rounded(to: 1))"
} else if let value = dictionary["value"] as? String {
return value
}
throw Parsing.Error.missingKey("value")
}

static func parse(key: String) throws -> (String, LibraryIdentifier) {
let parts = key.split(separator: "_")
guard parts.count == 3 else {
Expand Down
65 changes: 49 additions & 16 deletions Zotero/Models/Database/Database.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,46 +13,79 @@ import RealmSwift
import Network

struct Database {
private static let schemaVersion: UInt64 = 40
private static let schemaVersion: UInt64 = 41

static func mainConfiguration(url: URL, fileStorage: FileStorage) -> Realm.Configuration {
var config = Realm.Configuration(fileURL: url,
schemaVersion: schemaVersion,
migrationBlock: createMigrationBlock(fileStorage: fileStorage),
deleteRealmIfMigrationNeeded: false)
config.objectTypes = [RCollection.self, RCreator.self, RCustomLibrary.self, RGroup.self, RItem.self, RItemField.self, RLink.self, RPageIndex.self, RPath.self, RPathCoordinate.self, RRect.self,
RRelation.self, RSearch.self, RCondition.self, RTag.self, RTypedTag.self, RUser.self, RWebDavDeletion.self, RVersions.self, RObjectChange.self]
var config = Realm.Configuration(
fileURL: url,
schemaVersion: schemaVersion,
migrationBlock: createMigrationBlock(fileStorage: fileStorage),
deleteRealmIfMigrationNeeded: false
)
config.objectTypes = [
RCollection.self,
RCreator.self,
RCustomLibrary.self,
RGroup.self,
RItem.self,
RItemField.self,
RLink.self,
RPageIndex.self,
RPath.self,
RPathCoordinate.self,
RRect.self,
RRelation.self,
RSearch.self,
RCondition.self,
RTag.self,
RTypedTag.self,
RUser.self,
RWebDavDeletion.self,
RVersions.self,
RObjectChange.self
]
return config
}

static func bundledDataConfiguration(fileStorage: FileStorage) -> Realm.Configuration {
let url = Files.bundledDataDbFile.createUrl()
var config = Realm.Configuration(fileURL: url,
schemaVersion: schemaVersion,
migrationBlock: createMigrationBlock(fileStorage: fileStorage),
deleteRealmIfMigrationNeeded: false)
var config = Realm.Configuration(
fileURL: url,
schemaVersion: schemaVersion,
migrationBlock: createMigrationBlock(fileStorage: fileStorage),
deleteRealmIfMigrationNeeded: false
)
config.objectTypes = [RTranslatorMetadata.self, RStyle.self]
return config
}

private static func createMigrationBlock(fileStorage: FileStorage) -> MigrationBlock {
return { migration, schemaVersion in
if schemaVersion < 35 {
// Migrate to new object change model.
self.migrateObjectChange(migration: migration)
migrateObjectChange(migration: migration)
}
if schemaVersion < 36 {
self.migrateTagNames(migration: migration)
migrateTagNames(migration: migration)
}
if schemaVersion < 37 {
self.extractItemHtmlFreeContent(migration: migration)
extractItemHtmlFreeContent(migration: migration)
}
if schemaVersion < 39 {
self.addUuidsToCreators(migration: migration)
addUuidsToCreators(migration: migration)
}
if schemaVersion < 40 {
truncateAnnotationPageLabels(migration: migration)
}
if schemaVersion < 41 {
migratePageIndexToString(migration: migration)
}
}
}

private static func migratePageIndexToString(migration: Migration) {
migration.enumerateObjects(ofType: RPageIndex.className()) { oldObject, newObject in
guard let oldValue = oldObject?["index"] as? Int else { return }
newObject?["index"] = "\(oldValue)"
}
}

Expand Down
2 changes: 1 addition & 1 deletion Zotero/Models/Database/RPageIndex.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ extension RPageIndexChanges {

final class RPageIndex: Object {
@Persisted(indexed: true) var key: String
@Persisted var index: Int
@Persisted var index: String
@Persisted var changed: Bool
@Persisted var customLibraryKey: RCustomLibraryType?
@Persisted var groupKey: Int?
Expand Down
11 changes: 10 additions & 1 deletion Zotero/Models/UpdatableObject.swift
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,16 @@ extension RPageIndex: Updatable {
libraryPart = "g\(groupId)"
}

return ["lastPageIndex_\(libraryPart)_\(self.key)": ["value": self.index]]
let value: Any
if let _value = Int(index) {
value = _value
} else if let _value = Double(index) {
value = Decimal(_value).rounded(to: 1)
} else {
value = index
}

return ["lastPageIndex_\(libraryPart)_\(self.key)": ["value": value]]
}

var selfOrChildChanged: Bool {
Expand Down
1 change: 1 addition & 0 deletions Zotero/Scenes/Detail/PDF/Models/PDFReaderState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ struct PDFReaderState: ViewModelState {
case cantAddAnnotations
case cantUpdateAnnotation
case mergeTooBig
case pageNotInt
case unknown
}

Expand Down
4 changes: 4 additions & 0 deletions Zotero/Scenes/Detail/PDF/PDFCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,10 @@ extension PDFCoordinator: PdfReaderCoordinatorDelegate {
title = L10n.error
message = L10n.Errors.Pdf.cantUpdateAnnotation

case .pageNotInt:
title = L10n.error
message = L10n.Errors.Pdf.pageIndexNotInt

case .unknown:
title = L10n.error
message = L10n.Errors.unknown
Expand Down
10 changes: 7 additions & 3 deletions Zotero/Scenes/Detail/PDF/ViewModels/PDFReaderActionHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ final class PDFReaderActionHandler: ViewModelActionHandler, BackgroundDbProcessi
state.visiblePage = page
}

let request = StorePageForItemDbRequest(key: viewModel.state.key, libraryId: viewModel.state.library.identifier, page: page)
let request = StorePageForItemDbRequest(key: viewModel.state.key, libraryId: viewModel.state.library.identifier, page: "\(page)")
self.perform(request: request) { error in
guard let error = error else { return }
// TODO: - handle error
Expand Down Expand Up @@ -1669,13 +1669,17 @@ final class PDFReaderActionHandler: ViewModelActionHandler, BackgroundDbProcessi
private func loadAnnotationsAndPage(for key: String, library: Library) -> Result<(Results<RItem>, Int), Error> {
do {
var results: Results<RItem>!
var page = 0
var pageStr = "0"

try self.dbStorage.perform(on: .main, with: { coordinator in
page = try coordinator.perform(request: ReadDocumentDataDbRequest(attachmentKey: key, libraryId: library.identifier))
pageStr = try coordinator.perform(request: ReadDocumentDataDbRequest(attachmentKey: key, libraryId: library.identifier))
results = try coordinator.perform(request: ReadAnnotationsDbRequest(attachmentKey: key, libraryId: library.identifier))
})

guard let page = Int(pageStr) else {
return .failure(PDFReaderState.Error.pageNotInt)
}

return .success((results, page))
} catch let error {
return .failure(error)
Expand Down
92 changes: 92 additions & 0 deletions ZoteroTests/SettingsResponseSpec.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
//
// SettingsResponseSpec.swift
// ZoteroTests
//
// Created by Michal Rentka on 27.10.2023.
// Copyright © 2023 Corporation for Digital Scholarship. All rights reserved.
//

import Foundation

@testable import Zotero

import Nimble
import Quick

final class SettingsResponseSpec: QuickSpec {
override class func spec() {
describe("a JSON settings response") {
var jsonData: [String: Any]!

justBeforeEach {
let json = """
{
"lastPageIndex_u_ZYI76ILE": {
"value": 1,
"version": 62756
},
"lastPageIndex_u_ZYI76ILF": {
"value": 2.2,
"version": 62756
},
"lastPageIndex_u_ZYI76ILG": {
"value": 2.233412312,
"version": 62756
},
"lastPageIndex_g333_ZYI76ILH": {
"value": "asda",
"version": 62756
},
"tagColors": {
"value": [],
"version": 66099
}
}
"""
let data = json.data(using: .utf8)!
jsonData = try! JSONSerialization.jsonObject(with: data, options: .allowFragments) as! [String: Any]
}

context("with supported page index formats") {
it("parses all formats successfully") {
do {
let decoded = try SettingsResponse(response: jsonData!)
expect(decoded.pageIndices.indices.count).to(equal(4))

guard decoded.pageIndices.indices.count == 4 else { return }

if let index = decoded.pageIndices.indices.first(where: { $0.key == "ZYI76ILE" }) {
expect(index.libraryId).to(equal(.custom(.myLibrary)))
expect(index.value).to(equal("1"))
} else {
fail("Missing page index ZYI76ILE")
}

if let index = decoded.pageIndices.indices.first(where: { $0.key == "ZYI76ILF" }) {
expect(index.libraryId).to(equal(.custom(.myLibrary)))
expect(index.value).to(equal("2.2"))
} else {
fail("Missing page index ZYI76ILE")
}

if let index = decoded.pageIndices.indices.first(where: { $0.key == "ZYI76ILG" }) {
expect(index.libraryId).to(equal(.custom(.myLibrary)))
expect(index.value).to(equal("2.233"))
} else {
fail("Missing page index ZYI76ILE")
}

if let index = decoded.pageIndices.indices.first(where: { $0.key == "ZYI76ILH" }) {
expect(index.libraryId).to(equal(.group(333)))
expect(index.value).to(equal("asda"))
} else {
fail("Missing page index ZYI76ILE")
}
} catch let error {
fail(error.localizedDescription)
}
}
}
}
}
}

0 comments on commit e320953

Please sign in to comment.