diff --git a/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift b/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift index 1673f7b33d..05a2bc9b77 100644 --- a/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift +++ b/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift @@ -100,16 +100,7 @@ package enum ConvertActionConverter { let resultsSyncQueue = DispatchQueue(label: "Convert Serial Queue", qos: .unspecified, attributes: []) let resultsGroup = DispatchGroup() - - // Consume external links and add them into the sidebar. - for externalLink in context.externalCache { - // Here we're associating the external node with the **current** bundle's bundle ID. - // This is needed because nodes are only considered children if the parent and child's bundle ID match. - // Otherwise, the node will be considered as a separate root node and displayed separately. - let externalRenderNode = ExternalRenderNode(externalEntity: externalLink.value, bundleIdentifier: context.inputs.id) - try outputConsumer.consume(externalRenderNode: externalRenderNode) - } - + let renderSignpostHandle = signposter.beginInterval("Render", id: signposter.makeSignpostID(), "Render \(context.knownPages.count) pages") var conversionProblems: [Problem] = context.knownPages.concurrentPerform { identifier, results in @@ -172,7 +163,27 @@ package enum ConvertActionConverter { signposter.endInterval("Render", renderSignpostHandle) guard !Task.isCancelled else { return [] } - + + // Consumes all external links and adds them into the sidebar. + // This consumes all external links referenced across all content, and indexes them so they're available for reference in the navigator. + // This is not ideal as it means that links outside of the Topics section can impact the content of the navigator. + // TODO: It would be more correct to only index external links which have been curated as part of the Topics section. + // + // This has to run after all local nodes have been indexed because we're associating the external node with the **current** bundle's bundle ID, + // which makes it possible for there be clashes between local and external render nodes. + // When there are duplicate nodes, only the first one will be indexed, + // so in order to prefer local entities whenever there are any clashes, we have to index external nodes second. + // TODO: External render nodes should be associated with the correct bundle identifier. + try signposter.withIntervalSignpost("Index external links", id: signposter.makeSignpostID()) { + for externalLink in context.externalCache { + // Here we're associating the external node with the **current** bundle's bundle ID. + // This is needed because nodes are only considered children if the parent and child's bundle ID match. + // Otherwise, the node will be considered as a separate root node and displayed separately. + let externalRenderNode = ExternalRenderNode(externalEntity: externalLink.value, bundleIdentifier: context.inputs.id) + try outputConsumer.consume(externalRenderNode: externalRenderNode) + } + } + // Write various metadata if emitDigest { signposter.withIntervalSignpost("Emit digest", id: signposter.makeSignpostID()) { diff --git a/Tests/CommandLineTests/ConvertActionTests.swift b/Tests/CommandLineTests/ConvertActionTests.swift index ea32665a99..647fa2daec 100644 --- a/Tests/CommandLineTests/ConvertActionTests.swift +++ b/Tests/CommandLineTests/ConvertActionTests.swift @@ -3186,6 +3186,160 @@ class ConvertActionTests: XCTestCase { "/images/unit-test/image-name~dark@2x.png" ]) } + + func testExternalLinksInContentDontAffectNavigatorIndex() async throws { + let temporaryFolder = try createTemporaryDirectory() + + let resolver: OutOfProcessReferenceResolver + do { + let executableLocation = temporaryFolder.appendingPathComponent("link-resolver-executable") + + let externalBundleID = "com.external.test" + let externalSummary = LinkDestinationSummary( + kind: .class, + language: .swift, + relativePresentationURL: URL(string: "/documentation/TestBundle/SampleClass")!, + referenceURL: URL(string: "doc://\(externalBundleID)/documentation/TestBundle/SampleClass")!, + title: "SampleClass", + abstract: [.text("External abstract")], + availableLanguages: [.swift], + references: [], + variants: [] + ) + let encodedResponse = try String(decoding: JSONEncoder().encode(OutOfProcessReferenceResolver.ResponseV2.resolved(externalSummary)), as: UTF8.self) + + try """ + #!/bin/bash + echo '{"bundleIdentifier":"\(externalBundleID)","capabilities":0}' # Write this resolver's bundle identifier + read # Wait for docc to send a symbol USR + echo '\(encodedResponse)' # Respond with the resolved link summary + """.write(to: executableLocation, atomically: true, encoding: .utf8) + + // `0o0700` is `-rwx------` (read, write, & execute only for owner) + try FileManager.default.setAttributes([.posixPermissions: 0o0700], ofItemAtPath: executableLocation.path) + XCTAssert(FileManager.default.isExecutableFile(atPath: executableLocation.path)) + + resolver = try OutOfProcessReferenceResolver(processLocation: executableLocation, errorOutputHandler: { _ in }) + } + + let catalog = Folder(name: "unit-test.docc", content: [ + InfoPlist(displayName: "TestBundle", identifier: "com.test.example"), + TextFile(name: "Article.md", utf8Content: """ + # Article + + This is an internal article with an external link which clashes with the curated local link. + + External links in content should not affect the navigator. + + ## Topics + + - ``SampleClass`` + """), + TextFile(name: "SampleClass.md", utf8Content: """ + # ``SampleClass`` + + This extends the documentation for this symbol. + + ## Topics + + - + - + """), + TextFile(name: "ChildArticleA.md", utf8Content: """ + # ChildArticleA + + A child article. + """), + TextFile(name: "ChildArticleB.md", utf8Content: """ + # ChildArticleB + + A child article. + """), + // Symbol graph with a class that matches an external link path + JSONFile(name: "TestBundle.symbols.json", content: makeSymbolGraph(moduleName: "TestBundle", symbols: [ + makeSymbol(id: "some-symbol-id", language: .swift, kind: .class, pathComponents: ["SampleClass"]) + ])), + ]) + + let catalogURL = try catalog.write(inside: temporaryFolder) + let targetURL = temporaryFolder.appendingPathComponent("Output.doccarchive", isDirectory: true) + + let action = try ConvertAction( + documentationBundleURL: catalogURL, + outOfProcessResolver: resolver, + analyze: false, + targetDirectory: targetURL, + htmlTemplateDirectory: nil, + emitDigest: false, + currentPlatforms: nil, + fileManager: FileManager.default, + temporaryDirectory: createTemporaryDirectory() + ) + + let result = try await action.perform(logHandle: .none) + + XCTAssertEqual(result.outputs, [targetURL]) + + let renderIndexURL = targetURL.appendingPathComponent("index", isDirectory: true) + .appendingPathComponent("index.json", isDirectory: false) + + XCTAssertEqual( + try RenderIndex.fromURL(renderIndexURL), + try RenderIndex.fromString( + """ + { + "includedArchiveIdentifiers": [ + "com.test.example" + ], + "interfaceLanguages": { + "swift": [ + { + "children": [ + { + "title": "Articles", + "type": "groupMarker" + }, + { + "children": [ + { + "children": [ + { + "path": "/documentation/testbundle/childarticlea", + "title": "ChildArticleA", + "type": "article" + }, + { + "path": "/documentation/testbundle/childarticleb", + "title": "ChildArticleB", + "type": "article" + } + ], + "path": "/documentation/testbundle/sampleclass", + "title": "SampleClass", + "type": "class" + } + ], + "path": "/documentation/testbundle/article", + "title": "Article", + "type": "symbol" + } + ], + "path": "/documentation/testbundle", + "title": "TestBundle", + "type": "module" + } + ] + }, + "schemaVersion": { + "major": 0, + "minor": 1, + "patch": 2 + } + } + """ + ) + ) + } #endif } @@ -3280,3 +3434,15 @@ private extension ConvertAction { return try await perform(logHandle: &logHandle) } } + +extension RenderIndex { + static func fromString(_ string: String) throws -> RenderIndex { + let decoder = JSONDecoder() + return try decoder.decode(RenderIndex.self, from: Data(string.utf8)) + } + + static func fromURL(_ url: URL) throws -> RenderIndex? { + let data = try Data(contentsOf: url) + return try JSONDecoder().decode(RenderIndex.self, from: data) + } +}