diff --git a/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift b/Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift index 1673f7b33d..c18d64004b 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 **local** documentation's identifier, + // 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 documentation identifier. + try signposter.withIntervalSignpost("Index external links", id: signposter.makeSignpostID()) { + for externalLink in context.externalCache { + // Here we're associating the external node with the **local** documentation's identifier. + // This is needed because nodes are only considered children if the parent and child's identifier 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/SwiftDocCTests/Indexing/ExternalRenderNodeTests.swift b/Tests/SwiftDocCTests/Indexing/ExternalRenderNodeTests.swift index 1d5ed2b1cc..588c979ebd 100644 --- a/Tests/SwiftDocCTests/Indexing/ExternalRenderNodeTests.swift +++ b/Tests/SwiftDocCTests/Indexing/ExternalRenderNodeTests.swift @@ -507,4 +507,150 @@ class ExternalRenderNodeTests: XCTestCase { XCTAssertEqual(objcNavigatorExternalRenderNode.metadata.title, objcTitle) XCTAssertTrue(objcNavigatorExternalRenderNode.metadata.isBeta) } + + func testExternalLinksInContentDontAffectNavigatorIndex() async throws { + let externalResolver = ExternalReferenceResolverTests.TestExternalReferenceResolver() + externalResolver.expectedReferencePath = "/documentation/testbundle/sampleclass" + + let catalog = Folder(name: "unit-test.docc", content: [ + 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"]) + ])), + ]) + + var configuration = DocumentationContext.Configuration() + configuration.externalDocumentationConfiguration.sources[externalResolver.bundleID] = externalResolver + let (bundle, context) = try await loadBundle(catalog: catalog, configuration: configuration) + XCTAssert(context.problems.isEmpty, "Unexpectedly found problems: \(context.problems.map(\.diagnostic.summary))") + + let renderIndexFolder = try createTemporaryDirectory() + let indexBuilder = NavigatorIndex.Builder(outputURL: renderIndexFolder, bundleIdentifier: bundle.id.rawValue, sortRootChildrenByName: true, groupByLanguage: true) + indexBuilder.setup() + let outputConsumer = TestExternalRenderNodeOutputConsumer(indexBuilder: indexBuilder) + + let problems = try ConvertActionConverter.convert( + context: context, + outputConsumer: outputConsumer, + sourceRepository: nil, + emitDigest: false, + documentationCoverageOptions: .noCoverage + ) + XCTAssert(problems.isEmpty, "Unexpectedly found problems: \(DiagnosticConsoleWriter.formattedDescription(for: problems))") + indexBuilder.finalize(emitJSONRepresentation: true, emitLMDBRepresentation: false) + + XCTAssertEqual( + try RenderIndex.fromURL(renderIndexFolder.appendingPathComponent("index.json", isDirectory: false)), + try RenderIndex.fromString( + """ + { + "includedArchiveIdentifiers": [ + "unit-test" + ], + "interfaceLanguages": { + "swift": [ + { + "children": [ + { + "title": "Articles", + "type": "groupMarker" + }, + { + "children": [ + { + "children": [ + { + "path": "/documentation/unit-test/childarticlea", + "title": "ChildArticleA", + "type": "article" + }, + { + "path": "/documentation/unit-test/childarticleb", + "title": "ChildArticleB", + "type": "article" + } + ], + "path": "/documentation/testbundle/sampleclass", + "title": "SampleClass", + "type": "class" + } + ], + "path": "/documentation/unit-test/article", + "title": "Article", + "type": "symbol" + } + ], + "path": "/documentation/testbundle", + "title": "TestBundle", + "type": "module" + } + ] + }, + "schemaVersion": { + "major": 0, + "minor": 1, + "patch": 2 + } + } + """ + ) + ) + } +} + +private class TestExternalRenderNodeOutputConsumer: ConvertOutputConsumer, ExternalNodeConsumer { + let indexBuilder: Synchronized! + + init(indexBuilder: NavigatorIndex.Builder) { + self.indexBuilder = Synchronized(indexBuilder) + } + + func consume(externalRenderNode: ExternalRenderNode) throws { + try self.indexBuilder.sync { try $0.index(renderNode: externalRenderNode) } + } + + func consume(renderNode: RenderNode) throws { + try self.indexBuilder.sync { try $0.index(renderNode: renderNode) } + } + + func consume(assetsInBundle bundle: DocumentationBundle) throws { } + func consume(linkableElementSummaries: [LinkDestinationSummary]) throws { } + func consume(indexingRecords: [IndexingRecord]) throws { } + func consume(assets: [RenderReferenceType: [any RenderReference]]) throws { } + func consume(benchmarks: Benchmark) throws { } + func consume(documentationCoverageInfo: [CoverageDataEntry]) throws { } + func consume(renderReferenceStore: RenderReferenceStore) throws { } + func consume(buildMetadata: BuildMetadata) throws { } + func consume(linkResolutionInformation: SerializableLinkResolutionInformation) throws { } }