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

Ensure that bundle identifiers are valid URL host components #1069

Merged
merged 21 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
28b9522
Add bundle identifier type that ensures a valid URL host component
d-ronnqvist Oct 22, 2024
0e62ffc
Use new Identifier type in `DocumentationBundle/Info`
d-ronnqvist Oct 22, 2024
b46b5d3
Update code to not use deprecated `Info/identifier` property
d-ronnqvist Oct 22, 2024
bfa673b
Use new Identifier type in `DocumentationBundle`
d-ronnqvist Oct 22, 2024
98c6a58
Update code to not use deprecated `identifier` property
d-ronnqvist Oct 22, 2024
399ed9a
Use new Identifier type in `[Unresolved|Resolved]TopicReference`
d-ronnqvist Oct 22, 2024
a7310b7
Update code to not use deprecated topic reference `bundleIdentifier` …
d-ronnqvist Oct 22, 2024
221891a
Deprecate `BundleIdentifier` in favor of `DocumentationBundle/Identif…
d-ronnqvist Oct 22, 2024
890e323
Use new Identifier type in `BuildMetadata`
d-ronnqvist Oct 22, 2024
3c56a2d
Use new Identifier type in `AssetReference`
d-ronnqvist Oct 22, 2024
13b11fb
Use new Identifier type in `ResourceReference`
d-ronnqvist Oct 22, 2024
414ef9c
Use new Identifier type in `ConvertServiceFallbackResolver`
d-ronnqvist Oct 22, 2024
61cc5e2
Use new Identifier type in `SerializableLinkResolutionInformation`
d-ronnqvist Oct 22, 2024
5369b1c
Use new Identifier type in `ConvertAction/Indexer`
d-ronnqvist Oct 22, 2024
4805a05
Prefer `bundleID` over `id` for types that scoped inside bundle
d-ronnqvist Oct 22, 2024
0f2f8ef
Merge branch 'main' into validate-bundle-identifier
d-ronnqvist Oct 31, 2024
c2442bc
Merge branch 'main' into validate-bundle-identifier
d-ronnqvist Nov 11, 2024
71b27b6
Merge branch 'main' into validate-bundle-identifier
d-ronnqvist Nov 19, 2024
3d063ef
Merge branch 'main' into validate-bundle-identifier
d-ronnqvist Nov 21, 2024
f97e6d3
Use `bundleID` instead of `id` for property names outside the bundle
d-ronnqvist Nov 21, 2024
9478d47
Use "bundle id" in local variable in fallback resolver
d-ronnqvist Nov 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ public struct ConvertService: DocumentationService {
.compactMap { (value, isDocumentationExtensionContent) -> (ResolvedTopicReference, RenderReferenceStore.TopicContent)? in
let (topicReference, article) = value

guard let bundle = context.bundle, bundle.id.rawValue == topicReference.bundleIdentifier else { return nil }
guard let bundle = context.bundle, bundle.id == topicReference.id else { return nil }
let renderer = DocumentationContentRenderer(documentationContext: context, bundle: bundle)

let documentationNodeKind: DocumentationNode.Kind = isDocumentationExtensionContent ? .unknownSymbol : .article
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftDocC/Indexing/Navigator/NavigatorIndex.swift
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ extension ResolvedTopicReference {
let normalizedPath = NodeURLGenerator.fileSafeReferencePath(self, lowercased: true)

return NavigatorIndex.Identifier(
bundleIdentifier: bundleIdentifier.lowercased(),
bundleIdentifier: id.rawValue.lowercased(),
path: "/" + normalizedPath,
fragment: fragment,
languageIdentifier: languageIdentifier
Expand Down
6 changes: 3 additions & 3 deletions Sources/SwiftDocC/Infrastructure/DocumentationBundle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,9 @@ public struct DocumentationBundle {
self.customHeader = customHeader
self.customFooter = customFooter
self.themeSettings = themeSettings
self.rootReference = ResolvedTopicReference(bundleIdentifier: info.id.rawValue, path: "/", sourceLanguage: .swift)
self.documentationRootReference = ResolvedTopicReference(bundleIdentifier: info.id.rawValue, path: NodeURLGenerator.Path.documentationFolder, sourceLanguage: .swift)
self.tutorialTableOfContentsContainer = ResolvedTopicReference(bundleIdentifier: info.id.rawValue, path: NodeURLGenerator.Path.tutorialsFolder, sourceLanguage: .swift)
self.rootReference = ResolvedTopicReference(id: info.id, path: "/", sourceLanguage: .swift)
self.documentationRootReference = ResolvedTopicReference(id: info.id, path: NodeURLGenerator.Path.documentationFolder, sourceLanguage: .swift)
self.tutorialTableOfContentsContainer = ResolvedTopicReference(id: info.id, path: NodeURLGenerator.Path.tutorialsFolder, sourceLanguage: .swift)
self.tutorialsContainerReference = tutorialTableOfContentsContainer.appendingPath(urlReadablePath(info.displayName))
self.articlesDocumentationRootReference = documentationRootReference.appendingPath(urlReadablePath(info.displayName))
}
Expand Down
52 changes: 25 additions & 27 deletions Sources/SwiftDocC/Infrastructure/DocumentationContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public class DocumentationContext {
/// references for lookup.
var documentationCache = LocalCache()
/// The asset managers for each documentation bundle, keyed by the bundle's identifier.
var assetManagers = [BundleIdentifier: DataAssetManager]()
var assetManagers = [DocumentationBundle.Identifier: DataAssetManager]()
/// A list of non-topic links that can be resolved.
var nodeAnchorSections = [ResolvedTopicReference: AnchorSection]()

Expand Down Expand Up @@ -914,7 +914,7 @@ public class DocumentationContext {
let (url, analyzed) = analyzedDocument

let path = NodeURLGenerator.pathForSemantic(analyzed, source: url, bundle: bundle)
let reference = ResolvedTopicReference(bundleIdentifier: bundle.id.rawValue, path: path, sourceLanguage: .swift)
let reference = ResolvedTopicReference(id: bundle.id, path: path, sourceLanguage: .swift)

// Since documentation extensions' filenames have no impact on the URL of pages, there is no need to enforce unique filenames for them.
// At this point we consider all articles with an H1 containing link a "documentation extension."
Expand Down Expand Up @@ -1335,7 +1335,7 @@ public class DocumentationContext {

let symbolPath = NodeURLGenerator.Path.documentation(path: url.components.path).stringValue
let symbolReference = ResolvedTopicReference(
bundleIdentifier: reference.bundleIdentifier,
id: reference.id,
path: symbolPath,
fragment: nil,
sourceLanguages: reference.sourceLanguages
Expand Down Expand Up @@ -1776,11 +1776,11 @@ public class DocumentationContext {

private func registerMiscResources(from bundle: DocumentationBundle) throws {
let miscResources = Set(bundle.miscResourceURLs)
try assetManagers[bundle.id.rawValue, default: DataAssetManager()].register(data: miscResources)
try assetManagers[bundle.id, default: DataAssetManager()].register(data: miscResources)
}

private func registeredAssets(withExtensions extensions: Set<String>? = nil, inContexts contexts: [DataAsset.Context] = DataAsset.Context.allCases, forBundleID bundleIdentifier: BundleIdentifier) -> [DataAsset] {
guard let resources = assetManagers[bundleIdentifier]?.storage.values else {
private func registeredAssets(withExtensions extensions: Set<String>? = nil, inContexts contexts: [DataAsset.Context] = DataAsset.Context.allCases, forBundleID bundleID: DocumentationBundle.Identifier) -> [DataAsset] {
guard let resources = assetManagers[bundleID]?.storage.values else {
return []
}
return resources.filter { dataAsset in
Expand All @@ -1801,23 +1801,23 @@ public class DocumentationContext {
/// - Parameter bundleIdentifier: The identifier of the bundle to return image assets for.
/// - Returns: A list of all the image assets for the given bundle.
public func registeredImageAssets(forBundleID bundleIdentifier: BundleIdentifier) -> [DataAsset] {
return registeredAssets(withExtensions: DocumentationContext.supportedImageExtensions, forBundleID: bundleIdentifier)
return registeredAssets(withExtensions: DocumentationContext.supportedImageExtensions, forBundleID: DocumentationBundle.Identifier(rawValue: bundleIdentifier))
}

/// Returns a list of all the video assets that registered for a given `bundleIdentifier`.
///
/// - Parameter bundleIdentifier: The identifier of the bundle to return video assets for.
/// - Returns: A list of all the video assets for the given bundle.
public func registeredVideoAssets(forBundleID bundleIdentifier: BundleIdentifier) -> [DataAsset] {
return registeredAssets(withExtensions: DocumentationContext.supportedVideoExtensions, forBundleID: bundleIdentifier)
return registeredAssets(withExtensions: DocumentationContext.supportedVideoExtensions, forBundleID: DocumentationBundle.Identifier(rawValue: bundleIdentifier))
}

/// Returns a list of all the download assets that registered for a given `bundleIdentifier`.
///
/// - Parameter bundleIdentifier: The identifier of the bundle to return download assets for.
/// - Returns: A list of all the download assets for the given bundle.
public func registeredDownloadsAssets(forBundleID bundleIdentifier: BundleIdentifier) -> [DataAsset] {
return registeredAssets(inContexts: [DataAsset.Context.download], forBundleID: bundleIdentifier)
return registeredAssets(inContexts: [DataAsset.Context.download], forBundleID: DocumentationBundle.Identifier(rawValue: bundleIdentifier))
}

typealias Articles = [DocumentationContext.SemanticResult<Article>]
Expand Down Expand Up @@ -1932,7 +1932,7 @@ public class DocumentationContext {
let title = articleResult.source.deletingPathExtension().lastPathComponent
// Create a new root-looking reference
let reference = ResolvedTopicReference(
bundleIdentifier: bundle.id.rawValue,
id: bundle.id,
path: NodeURLGenerator.Path.documentation(path: title).stringValue,
sourceLanguages: [DocumentationContext.defaultLanguage(in: nil /* article-only content has no source language information */)]
)
Expand Down Expand Up @@ -1971,7 +1971,7 @@ public class DocumentationContext {
let path = NodeURLGenerator.Path.documentation(path: title).stringValue
let sourceLanguage = DocumentationContext.defaultLanguage(in: [])

let reference = ResolvedTopicReference(bundleIdentifier: bundle.id.rawValue, path: path, sourceLanguages: [sourceLanguage])
let reference = ResolvedTopicReference(id: bundle.id, path: path, sourceLanguages: [sourceLanguage])

let graphNode = TopicGraph.Node(reference: reference, kind: .module, source: .external, title: title)
topicGraph.addNode(graphNode)
Expand Down Expand Up @@ -2036,7 +2036,7 @@ public class DocumentationContext {
let defaultSourceLanguage = defaultLanguage(in: availableSourceLanguages)

let reference = ResolvedTopicReference(
bundleIdentifier: bundle.id.rawValue,
id: bundle.id,
path: path,
sourceLanguages: availableSourceLanguages
// FIXME: Pages in article-only catalogs should not be inferred as "Swift" as a fallback
Expand Down Expand Up @@ -2670,13 +2670,13 @@ public class DocumentationContext {
Unregister a documentation bundle with this context and clear any cached resources associated with it.
*/
private func unregister(_ bundle: DocumentationBundle) {
let referencesToRemove = topicGraph.nodes.keys.filter { reference in
return reference.bundleIdentifier == bundle.id.rawValue
let referencesToRemove = topicGraph.nodes.keys.filter {
$0.id == bundle.id
}

for reference in referencesToRemove {
topicGraph.edges[reference]?.removeAll(where: { $0.bundleIdentifier == bundle.id.rawValue })
topicGraph.reverseEdges[reference]?.removeAll(where: { $0.bundleIdentifier == bundle.id.rawValue })
topicGraph.edges[reference]?.removeAll(where: { $0.id == bundle.id })
topicGraph.reverseEdges[reference]?.removeAll(where: { $0.id == bundle.id })
topicGraph.nodes[reference] = nil
}
}
Expand All @@ -2694,7 +2694,7 @@ public class DocumentationContext {
*/
public func resource(with identifier: ResourceReference, trait: DataTraitCollection = .init()) throws -> Data {
guard let bundle,
let assetManager = assetManagers[identifier.bundleIdentifier],
let assetManager = assetManagers[DocumentationBundle.Identifier(rawValue: identifier.bundleIdentifier)],
let asset = assetManager.allData(named: identifier.path) else {
throw ContextError.notFound(identifier.url)
}
Expand All @@ -2706,7 +2706,7 @@ public class DocumentationContext {

/// Returns true if a resource with the given identifier exists in the registered bundle.
public func resourceExists(with identifier: ResourceReference, ofType expectedAssetType: AssetType? = nil) -> Bool {
guard let assetManager = assetManagers[identifier.bundleIdentifier] else {
guard let assetManager = assetManagers[DocumentationBundle.Identifier(rawValue: identifier.bundleIdentifier)] else {
return false
}

Expand All @@ -2722,7 +2722,7 @@ public class DocumentationContext {
}

private func externalEntity(with reference: ResolvedTopicReference) -> LinkResolver.ExternalEntity? {
return configuration.externalDocumentationConfiguration.sources[reference.bundleIdentifier].map({ $0.entity(with: reference) })
return configuration.externalDocumentationConfiguration.sources[reference.id.rawValue].map({ $0.entity(with: reference) })
?? configuration.convertServiceConfiguration.fallbackResolver?.entityIfPreviouslyResolved(with: reference)
}

Expand Down Expand Up @@ -2888,8 +2888,7 @@ public class DocumentationContext {
/// - asset: The new asset for this name.
/// - parent: The topic where the asset is referenced.
public func updateAsset(named name: String, asset: DataAsset, in parent: ResolvedTopicReference) {
let bundleIdentifier = parent.bundleIdentifier
assetManagers[bundleIdentifier]?.update(name: name, asset: asset)
assetManagers[parent.id]?.update(name: name, asset: asset)
}

/// Attempt to resolve an asset given its name and the topic it's referenced in.
Expand All @@ -2900,12 +2899,11 @@ public class DocumentationContext {
/// - type: A restriction for what type of asset to resolve.
/// - Returns: The data that's associated with an image asset if it was found, otherwise `nil`.
public func resolveAsset(named name: String, in parent: ResolvedTopicReference, withType type: AssetType? = nil) -> DataAsset? {
let bundleIdentifier = parent.bundleIdentifier
return resolveAsset(named: name, bundleIdentifier: bundleIdentifier, withType: type)
resolveAsset(named: name, bundleID: parent.id, withType: type)
}

func resolveAsset(named name: String, bundleIdentifier: String, withType expectedType: AssetType?) -> DataAsset? {
if let localAsset = assetManagers[bundleIdentifier]?.allData(named: name) {
func resolveAsset(named name: String, bundleID: DocumentationBundle.Identifier, withType expectedType: AssetType?) -> DataAsset? {
if let localAsset = assetManagers[bundleID]?.allData(named: name) {
if let expectedType {
guard localAsset.hasVariant(withAssetType: expectedType) else {
return nil
Expand All @@ -2917,7 +2915,7 @@ public class DocumentationContext {

if let fallbackAssetResolver = configuration.convertServiceConfiguration.fallbackResolver,
let externallyResolvedAsset = fallbackAssetResolver.resolve(assetNamed: name) {
assetManagers[bundleIdentifier, default: DataAssetManager()]
assetManagers[bundleID, default: DataAssetManager()]
.register(dataAsset: externallyResolvedAsset, forName: name)
return externallyResolvedAsset
}
Expand Down Expand Up @@ -2945,7 +2943,7 @@ public class DocumentationContext {
///
/// - Returns: The best matching storage key if it was found, otherwise `nil`.
public func identifier(forAssetName name: String, in parent: ResolvedTopicReference) -> String? {
if let assetManager = assetManagers[parent.bundleIdentifier] {
if let assetManager = assetManagers[parent.id] {
if let localName = assetManager.bestKey(forAssetName: name) {
return localName
} else if let fallbackAssetManager = configuration.convertServiceConfiguration.fallbackResolver {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ struct DocumentationCurator {
let sourceArticlePath = NodeURLGenerator.Path.article(bundleName: bundle.displayName, articleName: articleFilename).stringValue

let reference = ResolvedTopicReference(
bundleIdentifier: resolved.bundleIdentifier,
id: resolved.id,
path: sourceArticlePath,
sourceLanguages: resolved.sourceLanguages)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public class OutOfProcessReferenceResolver: ExternalDocumentationSource, GlobalE
return resolved

case let .unresolved(unresolvedReference):
guard unresolvedReference.bundleIdentifier == id.rawValue else {
guard unresolvedReference.id == id else {
fatalError("""
Attempted to resolve a local reference externally: \(unresolvedReference.description.singleQuoted).
DocC should never pass a reference to an external resolver unless it matches that resolver's bundle identifier.
Expand Down Expand Up @@ -147,7 +147,7 @@ public class OutOfProcessReferenceResolver: ExternalDocumentationSource, GlobalE
guard let resolvedInformation = try? resolveInformationForSymbolIdentifier(preciseIdentifier) else { return nil }

let reference = ResolvedTopicReference(
bundleIdentifier: "com.externally.resolved.symbol",
id: "com.externally.resolved.symbol",
path: "/\(preciseIdentifier)",
sourceLanguages: sourceLanguages(for: resolvedInformation)
)
Expand Down Expand Up @@ -255,7 +255,7 @@ public class OutOfProcessReferenceResolver: ExternalDocumentationSource, GlobalE

private func resolvedReference(for resolvedInformation: ResolvedInformation) -> ResolvedTopicReference {
return ResolvedTopicReference(
bundleIdentifier: id.rawValue,
id: id,
path: resolvedInformation.url.path,
fragment: resolvedInformation.url.fragment,
sourceLanguages: sourceLanguages(for: resolvedInformation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ final class ExternalPathHierarchyResolver {
else {
return nil
}
return ResolvedTopicReference(bundleIdentifier: bundleID, path: url.path, fragment: url.fragment, sourceLanguage: .swift)
return ResolvedTopicReference(id: .init(rawValue: bundleID), path: url.path, fragment: url.fragment, sourceLanguage: .swift)
}
let dependencies = RenderReferenceDependencies(
topicReferences: topicReferences,
Expand Down Expand Up @@ -126,7 +126,7 @@ final class ExternalPathHierarchyResolver {
symbols.reserveCapacity(linkDestinationSummaries.count)
for entity in linkDestinationSummaries {
let reference = ResolvedTopicReference(
bundleIdentifier: entity.referenceURL.host!,
id: .init(rawValue: entity.referenceURL.host!),
path: entity.referenceURL.path,
fragment: entity.referenceURL.fragment,
sourceLanguage: entity.language
Expand All @@ -150,7 +150,7 @@ final class ExternalPathHierarchyResolver {
continue
}
let identifier = identifiers[index]
self.resolvedReferences[identifier] = ResolvedTopicReference(bundleIdentifier: fileRepresentation.bundleID, path: url.path, fragment: url.fragment, sourceLanguage: .swift)
self.resolvedReferences[identifier] = ResolvedTopicReference(id: .init(rawValue: fileRepresentation.bundleID), path: url.path, fragment: url.fragment, sourceLanguage: .swift)
}
}
// Finally, the Identifier -> Symbol mapping can be constructed by iterating over the nodes and looking up the reference for each USR.
Expand Down
Loading