Skip to content

Commit 7b266fa

Browse files
committed
Fix serialisation of isBeta and isExternal properties
The isBeta and isExternal properties weren't being serialised as part of `NavigatorItem`. These properties are used to initialise to `RenderIndex.Node` during the conversion to `index.json` [1] and must be preserved when navigator indexes are written to disk [2] so that when they are read [3], we don't drop beta and external information. This serialisation happens during the `finalize` step [4] and the deserialisation can be invoked via `NavigatorIndex.readNavigatorIndex` [5]. Otherwise, the values can be lost on serialisation roundtrip. [1]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Indexing/RenderIndexJSON/RenderIndex.swift#L329 [2]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Indexing/Navigator/NavigatorTree.swift#L195 [3]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Indexing/Navigator/NavigatorTree.swift#L266 [4]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift#L1193 [5]: https://github.com/swiftlang/swift-docc/blob/65aaf926ec079ddbd40f29540d4180a70af99e5e/Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift#L157
1 parent b7d9ecb commit 7b266fa

File tree

2 files changed

+86
-2
lines changed

2 files changed

+86
-2
lines changed

Sources/SwiftDocC/Indexing/Navigator/NavigatorItem.swift

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,27 @@ public final class NavigatorItem: Serializable, Codable, Equatable, CustomString
146146

147147
let pathData = data[cursor..<cursor + Int(pathLength)]
148148
self.path = String(data: pathData, encoding: .utf8)!
149+
cursor += Int(pathLength)
149150

150-
assert(cursor+Int(pathLength) == data.count)
151+
// isBeta and isExternal should be encoded because they are relevant when creating a RenderIndex node.
152+
// Without proper serialization, these indicators would be lost when navigator indexes are loaded from disk.
153+
154+
length = MemoryLayout<UInt8>.stride
155+
// To ensure backwards compatibility, handle both when `isBeta` has been encoded and when it hasn't
156+
if cursor < data.count {
157+
let betaValue: UInt8 = unpackedValueFromData(data[cursor..<cursor + length])
158+
cursor += length
159+
self.isBeta = betaValue != 0
160+
}
161+
162+
// To ensure backwards compatibility, handle both when `isExternal` has been encoded and when it hasn't
163+
if cursor < data.count {
164+
let externalValue: UInt8 = unpackedValueFromData(data[cursor..<cursor + length])
165+
cursor += length
166+
self.isExternal = externalValue != 0
167+
}
168+
169+
assert(cursor == data.count)
151170
}
152171

153172
/// Returns the `Data` representation of the current `NavigatorItem` instance.
@@ -164,6 +183,9 @@ public final class NavigatorItem: Serializable, Codable, Equatable, CustomString
164183
data.append(Data(title.utf8))
165184
data.append(Data(path.utf8))
166185

186+
data.append(packedDataFromValue(isBeta ? UInt8(1) : UInt8(0)))
187+
data.append(packedDataFromValue(isExternal ? UInt8(1) : UInt8(0)))
188+
167189
return data
168190
}
169191

@@ -176,7 +198,9 @@ public final class NavigatorItem: Serializable, Codable, Equatable, CustomString
176198
languageID: \(languageID),
177199
title: \(title),
178200
platformMask: \(platformMask),
179-
availabilityID: \(availabilityID)
201+
availabilityID: \(availabilityID),
202+
isBeta: \(isBeta),
203+
isExternal: \(isExternal)
180204
}
181205
"""
182206
}

Tests/SwiftDocCTests/Indexing/NavigatorIndexTests.swift

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,66 @@ Root
138138
XCTAssertEqual(item, fromData)
139139
}
140140

141+
func testNavigatorEquality() {
142+
// Test for equal
143+
var item1 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024, isExternal: true, isBeta: true)
144+
var item2 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024, isExternal: true, isBeta: true)
145+
XCTAssertEqual(item1, item2)
146+
147+
// Tests for not equal
148+
item1 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024, isBeta: true)
149+
item2 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024, isBeta: false)
150+
XCTAssertNotEqual(item1, item2)
151+
152+
item1 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024, isExternal: true)
153+
item2 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024, isExternal: false)
154+
XCTAssertNotEqual(item1, item2)
155+
156+
item1 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024)
157+
item2 = NavigatorItem(pageType: 2, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024)
158+
XCTAssertNotEqual(item1, item2)
159+
160+
item1 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024)
161+
item2 = NavigatorItem(pageType: 1, languageID: 5, title: "My Title", platformMask: 256, availabilityID: 1024)
162+
XCTAssertNotEqual(item1, item2)
163+
164+
item1 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024)
165+
item2 = NavigatorItem(pageType: 1, languageID: 4, title: "My Other Title", platformMask: 256, availabilityID: 1024)
166+
XCTAssertNotEqual(item1, item2)
167+
168+
item1 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024)
169+
item2 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 257, availabilityID: 1024)
170+
XCTAssertNotEqual(item1, item2)
171+
172+
item1 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024)
173+
item2 = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1025)
174+
XCTAssertNotEqual(item1, item2)
175+
}
176+
177+
func testNavigatorItemRawDumpWithExtraProperties() {
178+
let item = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024, isExternal: true, isBeta: true)
179+
let data = item.rawValue
180+
let fromData = NavigatorItem(rawValue: data)
181+
XCTAssertEqual(item, fromData)
182+
}
183+
184+
func testNavigatorItemRawDumpBackwardCompatibility() {
185+
let item = NavigatorItem(pageType: 1, languageID: 4, title: "My Title", platformMask: 256, availabilityID: 1024)
186+
var data = Data()
187+
data.append(packedDataFromValue(item.pageType))
188+
data.append(packedDataFromValue(item.languageID))
189+
data.append(packedDataFromValue(item.platformMask))
190+
data.append(packedDataFromValue(item.availabilityID))
191+
data.append(packedDataFromValue(UInt64(item.title.utf8.count)))
192+
data.append(packedDataFromValue(UInt64(item.path.utf8.count)))
193+
data.append(Data(item.title.utf8))
194+
data.append(Data(item.path.utf8))
195+
// Note: NOT adding isBeta and isExternal flags to simulate when they were not supported
196+
197+
let fromData = NavigatorItem(rawValue: data)
198+
XCTAssertEqual(item, fromData)
199+
}
200+
141201
func testObjCLanguage() {
142202
let root = generateLargeTree()
143203
var objcFiltered: Node?

0 commit comments

Comments
 (0)