Add external media support in documentation

[Kyle-Ye]

Bug/issue #, if applicable: SR-15839
Forum discussion: https://forums.swift.org/t/can-we-link-and-render-external-images-that-is-from-urls

Summary

Add a new ExternalMediaResolver to support external media reference.

Dependencies

None

Testing

Test external image and video support in docs and articles.

Checklist

• Added tests
• Ran the ./bin/test script and it succeeded
• Updated documentation if necessary

TODO

• Currently this PR does not support external media in tutorials. Will solve it in a follow-up PR.

[Kyle-Ye]

Implementation is almost done. Will update test and documentation later

[d-ronnqvist]

I'm wondering if there's a way to implement this without new render reference types. Ideally the solution could be easily shared to also add support for external videos and external downloads (if that's something that we want to support)

[franklinsch]

Hi @Kyle-Ye, are you still working on this? Let @d-ronnqvist or I know we can help!

[Kyle-Ye]

Hi @Kyle-Ye, are you still working on this? Let @d-ronnqvist or I know we can help!

Yes. Sorry for the late update (Busy with my full-time iOS RD internship recently)

I have a few question here in order to better finish this PR and your review.

After performing "docc preview" action to the xx.docc file, it will produce a xx.docc/.docc-build/data/documentation/swiftdoccrender.json file

For example if I have this markdown file in my xx.docc folder

# Swift-DocC-Render

@Metadata {
  @TechnologyRoot
}

A web renderer for DocC Archives produced by DocC.

## Overview

Local image

![swift icon](swift.svg)

External image

![swift icon](https://www.swift.org/assets/images/swift.svg)

DocC-Render displays documentation produced by [DocC](https://www.swift.org/documentation/docc) on the web and is included in DocC Archives.

It will produce something like this in reference part

{
    "https://www.swift.org/documentation/docc": {
        "title": "DocC",
        "titleInlineContent": [
            {
                "type": "text",
                "text": "DocC"
            }
        ],
        "type": "link",
        "identifier": "https://www.swift.org/documentation/docc",
        "url": "https://www.swift.org/documentation/docc"
    },
    // MARK: - This is the current behavior. Without the PR, this item will not exist.
    "https://www.swift.org/assets/images/swift.svg": {
        "alt": "swift icon",
        "type": "externalImage",
        "identifier": "https://www.swift.org/assets/images/swift.svg",
        "variants": [
            {
                "url": "https://www.swift.org/assets/images/swift.svg",
                "traits": [
                    "1x",
                    "light"
                ]
            }
        ]
    },
    "swift.svg": {
        "alt": "swift icon",
        "type": "image",
        "identifier": "swift.svg",
        "variants": [
            {
                "url": "/images/swift.svg",
                "traits": [
                    "1x",
                    "light"
                ]
            }
        ]
    }
}

I think "d-ronnqvist" is suggesting me to generate an item like this without introducing a new externalImage type, right?

    "https://www.swift.org/assets/images/swift.svg": {
        "alt": "swift icon",
        "type": "image",
        "identifier": "https://www.swift.org/assets/images/swift.svg",
        "variants": [
            {
                // MARK: Problem here, since in current design, for image type it'll do some magical logic here, how can we identity this is actually an external image? Do we need introduce a new field or just use host == nil?
                "url": "https://www.swift.org/assets/images/swift.svg",
                "traits": [
                    "1x",
                    "light"
                ]
            }
        ]
    },

[Kyle-Ye]

Currently, I was stuck by the "display" and "download" context in DataAsset. Should externalImage a "display" context or "download" context? And what is "download" context mean in docc?

The current implementation will throw an error due to "copyAsset" API calling FileManager.copyItem(at:to:) where "at" is a https schema file. Will fix this once I figure what "download" context really means.
error: The file “swift.svg” couldn’t be opened because URL type https isn’t supported.

swift-docc/Sources/SwiftDocCUtilities/Action/Actions/Convert/ConvertFileWritingConsumer.swift

Lines 70 to 101 in 0d61dc5

	// Create images directory if needed.
	let imagesDirectory = targetFolder.appendingPathComponent("images", isDirectory: true)
	if !fileManager.directoryExists(atPath: imagesDirectory.path) {
	try fileManager.createDirectory(at: imagesDirectory, withIntermediateDirectories: false, attributes: nil)
	}
	// Copy all registered images to the output directory.
	for imageAsset in context.registeredImageAssets(forBundleID: bundleIdentifier) {
	try copyAsset(imageAsset, to: imagesDirectory)
	}
	// Create videos directory if needed.
	let videosDirectory = targetFolder.appendingPathComponent("videos", isDirectory: true)
	if !fileManager.directoryExists(atPath: videosDirectory.path) {
	try fileManager.createDirectory(at: videosDirectory, withIntermediateDirectories: false, attributes: nil)
	}
	// Copy all registered videos to the output directory.
	for videoAsset in context.registeredVideoAssets(forBundleID: bundleIdentifier) {
	try copyAsset(videoAsset, to: videosDirectory)
	}
	// Create downloads directory if needed.
	let downloadsDirectory = targetFolder.appendingPathComponent(DownloadReference.locationName, isDirectory: true)
	if !fileManager.directoryExists(atPath: downloadsDirectory.path) {
	try fileManager.createDirectory(at: downloadsDirectory, withIntermediateDirectories: false, attributes: nil)
	}
	// Copy all downloads into the output directory.
	for downloadAsset in context.registeredDownloadsAssets(forBundleID: bundleIdentifier) {
	try copyAsset(downloadAsset, to: downloadsDirectory)
	}

Appreciate for your help and clarification @franklinsch

[Kyle-Ye]

The old (working but rejected) implementation is at https://github.com/Kyle-Ye/swift-docc/tree/external-image-support_v1
The new (WIP) implementation is at https://github.com/Kyle-Ye/swift-docc/tree/external-image-support_v2 (Which is currently in sync with external-image-support branch)

[d-ronnqvist]

I’ll look at this tomorrow

[d-ronnqvist]

Currently, I was stuck by the "display" and "download" context in DataAsset. Should externalImage a "display" context or "download" context? And what is "download" context mean in docc?

From what I recall, this is an indication to Swift-DocC Render that has some impact on how the request for that access is made. @mportiz08 May be able to provide more information about how this information is used.

A "download" context is used for zip archives and similar files that are downloaded but not displayed on the page and a "display" context is used for images and video that are displayed on the page. So in this case I would expect a "display" context.

[mportiz08]

Currently, I was stuck by the "display" and "download" context in DataAsset. Should externalImage a "display" context or "download" context? And what is "download" context mean in docc?

From what I recall, this is an indication to Swift-DocC Render that has some impact on how the request for that access is made. @mportiz08 May be able to provide more information about how this information is used.

A "download" context is used for zip archives and similar files that are downloaded but not displayed on the page and a "display" context is used for images and video that are displayed on the page. So in this case I would expect a "display" context.

"display" sounds right to me. It seems that this enum is used to determine whether a particular asset should be turned into a media or download reference in the Render JSON:

swift-docc/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift

Line 1288 in 0661e96

if assetContext == DataAsset.Context.download, let resolvedDownload = resolveAsset() {

This will better indicate the type of reference and how it should be displayed or linked to for any renderers, including DocC-Render. The "download" value might be used for something like a zip file to download sample code, etc. Note that DocC-Render might support rendering content that DocC itself is not yet generating at the moment.

[d-ronnqvist]

The current implementation will throw an error due to "copyAsset" API calling FileManager.copyItem(at:to:) where "at" is a https schema file. Will fix this once I figure what "download" context really means.
error: The file “swift.svg” couldn’t be opened because URL type https isn’t supported.

I think this could work by skipping copying assets that aren't local. There are a few places that inspect the URL to determine if it's a local asset or an external asset so maybe that could be used here as well.

[Kyle-Ye]

This PR adds external media support in documentation(docs and articles) but not in tutorials.

Should we add support external media support for tutorials? And if we should maybe we can do it in a follow-up PR? cc @franklinsch

[franklinsch]

Supporting external images in tutorials in a separate PR sounds good to me. Please make sure to file a ticket for it on bugs.swift.org to make sure we don't lose track of it :)

[d-ronnqvist]

Thank you. This looks good to me.

I left a few comments and suggestions about some of the details.

[Kyle-Ye]

@swift-ci please test

[Kyle-Ye]

URL.isAbsoluteWebURL delete the host check, otherwise it will fail on some test due to the case like 'docc-media:///path/a.png'. We can fix it by not using the URL.isAbsoluteWebURL API here. And I think it's better to use one uniformed API, so I delete the host check in URL.isAbsoluteWebURL. Any suggestion is welcomed.

[d-ronnqvist]

I agree that it's better to use the same API everywhere.

We can revisit the other tests' test data in the future to see if anything should be updated but I don't think that needs to happen now.

[Kyle-Ye]

Finally fix all the test cases. Ready for the final review. @d-ronnqvist

[Kyle-Ye]

Supporting external images in tutorials in a separate PR sounds good to me. Please make sure to file a ticket for it on bugs.swift.org to make sure we don't lose track of it :)

Update the original SR's title to "Add external media support for DocC documentation" and file up a new ticket to track the issue. https://bugs.swift.org/browse/SR-16026

[d-ronnqvist]

Thank you! This looks great to me.

[d-ronnqvist]

I agree that it's better to use the same API everywhere.

We can revisit the other tests' test data in the future to see if anything should be updated but I don't think that needs to happen now.

[Kyle-Ye]

@swift-ci please test