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

Update latest folder with one index.html includes the redirect links #1298

Merged
4 commits merged into from
Jan 6, 2021
Merged
Changes from 2 commits
Commits
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
20 changes: 13 additions & 7 deletions eng/common/scripts/copy-docs-to-blobstorage.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -216,17 +216,23 @@ function Upload-Blobs

LogDebug "Uploading $($PkgName)/$($DocVersion) to $($DocDest)..."
& $($AzCopy) cp "$($DocDir)/**" "$($DocDest)/$($PkgName)/$($DocVersion)$($SASKey)" --recursive=true --cache-control "max-age=300, must-revalidate"

LogDebug "Handling versioning files under $($DocDest)/$($PkgName)/versioning/"
$versionsObj = (Update-Existing-Versions -PkgName $PkgName -PkgVersion $DocVersion -DocDest $DocDest)

# we can safely assume we have AT LEAST one version here. Reason being we just completed Update-Existing-Versions
$latestVersion = ($versionsObj.SortedVersionArray | Select-Object -First 1).RawVersion

if ($UploadLatest -and ($latestVersion -eq $DocVersion))
$latestVersion = $versionsObj.LatestGAPackage
if (!$latestVersion) {
$latestVersion = $versionsObj.LatestPreviewPackage
}
LogDebug "Fetching the latest version. $latestVersion"
# Prepare the index.html which can redirect to the latest GA whenever avaiable, otherwise point to latest preview.
New-Item -Path $DocDir -Name "latest" -ItemType "directory"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably also guard these New-Item creations behind the same uploadlatest and latestVersion condition.

New-Item -Path "$($DocDir)/latest" -Name "index.html" -ItemType "file" -Value "<meta http-equiv=`"refresh`" content=`"0; URL=$($DocDest)/$($PkgName)/$($latestVersion)/index.html`" />"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any downsides to using the meta refresh tag in terms of SEO or anything like that?

https://www.weboptimizers.com.au/google-warning-meta-refresh/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we care about our search indexing for these pages so I'm not worried about that. The reason we are using meta refresh for redirecting is because that is the only way we could figure out how to do a redirect while using blob storage as a web host. Ideally we would use server redirects of some sort but we don't have that support in blob storage hosting. If you have other ideas that might work I'd be interested in other ideas.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scbedd did you consider this option when you originally did started including the latest folder? Do you know of any potential issues based one your known usage of the latest folder?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think people will be linking directly to any /latest/<some subfolder>/blah.html anywhere. Given that, I can't think of any situations that this breaks.

if ($UploadLatest -and $latestVersion)
{
LogDebug "Uploading $($PkgName) to latest folder in $($DocDest)..."
& $($AzCopy) cp "$($DocDir)/**" "$($DocDest)/$($PkgName)/latest$($SASKey)" --recursive=true --cache-control "max-age=300, must-revalidate"
# Clean up existing folders, will remove this step once we have one or two release cycles.
& $($AzCopy) rm "$($DocDest)/$($PkgName)/latest$($SASKey)" --recursive=true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should see if we can do this as a one-time step for everything in the blob storage to avoid needing to delete and also to ensure we are consistent with our existing published docs.

& $($AzCopy) cp "$($DocDir)/latest/**" "$($DocDest)/$($PkgName)/latest$($SASKey)" --recursive=true --cache-control "max-age=300, must-revalidate"
}
}

Expand Down