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

Langtag download issue 987 #1232

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [SIL.Core] ConsoleErrorReporter logs exception if available
- [SIL.Core, SIL.Windows.Forms] If WinFormsErrorReporter is set as the ErrorReporter, and ErrorReporter.NotifyUserOfProblem(IRepeatNoticePolicy, Exception, String, params object[]) is passed null for the exception, the "Details" button will no longer appear, making this consistent with the no-Exception overload of this method
- [SIL.WritingSystems] Changed behavior of IetfLanguageTag to better handle zh-TW.
- [SIL.WritingSystems] Download of langtag.json handled with etag instead of IF-MODIFIED-SINCE Header(#987)

### Fixed

Expand Down
90 changes: 64 additions & 26 deletions SIL.WritingSystems/Sldr.cs
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,9 @@ public static IReadOnlyKeyedCollection<string, SldrLanguageTagInfo> LanguageTags
}
}

public static void InitializeLanguageTags()
public static void InitializeLanguageTags(bool downloadLangTags = true)
{
if (downloadLangTags) DownloadLanguageTags();
LoadLanguageTagsIfNecessary();
}

Expand All @@ -470,55 +471,92 @@ private static void LoadLanguageTagsIfNecessary()
CreateSldrCacheDirectory();

cachedAllTagsPath = Path.Combine(SldrCachePath, "langtags.json");
string etagPath;
etagPath = Path.Combine(SldrCachePath, "langtags.json.etag");
var sinceTime = _embeddedAllTagsTime.ToUniversalTime();
if (File.Exists(cachedAllTagsPath))
{
var fileTime = File.GetLastWriteTimeUtc(cachedAllTagsPath);
if (sinceTime > fileTime)
{
// delete the old langtags.json file if a newer embedded one is available.
// this can happen if the application is upgraded to use a newer version of SIL.WritingSystems
// that has an updated embedded langtags.json file.
File.Delete(cachedAllTagsPath);
File.Delete(etagPath);
}
else
sinceTime = fileTime;
}
sinceTime += TimeSpan.FromSeconds(1);
try
{
if (_offlineMode)
throw new WebException("Test mode: SLDR offline so accessing cache", WebExceptionStatus.ConnectFailure);

}
_languageTags = new ReadOnlyKeyedCollection<string, SldrLanguageTagInfo>(ParseAllTagsJson(cachedAllTagsPath));
}

// get SLDR langtags.json from the SLDR api compressed
// it will throw WebException or have status HttpStatusCode.NotModified if file is unchanged or not get it
var langtagsUrl =
$"{SldrRepository}index.html?query=langtags&ext=json{StagingParameter}";
var webRequest = (HttpWebRequest) WebRequest.Create(Uri.EscapeUriString(langtagsUrl));
webRequest.UserAgent = UserAgent;
webRequest.IfModifiedSince = sinceTime;
webRequest.Timeout = 10000;
webRequest.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate;
using var webResponse = (HttpWebResponse) webRequest.GetResponse();
public static void DownloadLanguageTags()
{
CreateSldrCacheDirectory();
string cachedAllTagsPath;
cachedAllTagsPath = Path.Combine(SldrCachePath, "langtags.json");
string etagPath;
etagPath = Path.Combine(SldrCachePath, "langtags.json.etag");
string etag;
string currentEtag;
try
{
if (_offlineMode)
throw new WebException("Test mode: SLDR offline so accessing cache", WebExceptionStatus.ConnectFailure);
Copy link
Member

Choose a reason for hiding this comment

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

These two lines can go directly below CreateSldrCacheDirectory().

// get SLDR langtags.json from the SLDR api compressed
// it will throw WebException or have status HttpStatusCode.NotModified if file is unchanged or not get it
var langtagsUrl =
$"{SldrRepository}index.html?query=langtags&ext=json{StagingParameter}";
var webRequest = (HttpWebRequest)WebRequest.Create(Uri.EscapeUriString(langtagsUrl));
webRequest.UserAgent = UserAgent;
webRequest.Timeout = 10000;
webRequest.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate;
ermshiperete marked this conversation as resolved.
Show resolved Hide resolved
ermshiperete marked this conversation as resolved.
Show resolved Hide resolved
using var webResponse = (HttpWebResponse)webRequest.GetResponse();
if (File.Exists(etagPath))
Copy link
Member

Choose a reason for hiding this comment

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

You'll also have to check for the existence of langtags.json file - it's possible that langtags.json.etag exists but langtags.json doesn't (e.g. it gets renamed if it contains corrupt content).

{
etag = File.ReadAllText(etagPath);
webRequest.Headers.Set(etag, "If-None-Match");
Copy link
Member

Choose a reason for hiding this comment

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

This is backwards. etag is the value, "If-None-Match" the key.

Suggested change
webRequest.Headers.Set(etag, "If-None-Match");
webRequest.Headers.Set("If-None-Match", etag);

currentEtag = webResponse.Headers.Get("Etag");
if (!etag.Equals(currentEtag))
{
//File.WriteAllText(etagPath, currentEtag);
//webRequest.Headers.Set(etag, "If-None-Match");
if (webResponse.StatusCode != HttpStatusCode.NotModified)
Copy link
Member

@ermshiperete ermshiperete Nov 14, 2022

Choose a reason for hiding this comment

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

The status code should be checked directly after calling webRequest.GetResponse(). If we get a status code other than OK we can return right away: either it's NotModified in which case we don't have to do anything, or we get an error in which case the data we got is not valid, so we can't do anything with it either.

In all cases I saw in my testing we got a WebException instead of a non-OK status code, but since the code comment says we might get a NotModified status code it safer to check it.

{
File.WriteAllText(etagPath, currentEtag);
using Stream output = File.OpenWrite(cachedAllTagsPath);
using var input = webResponse.GetResponseStream();
input.CopyTo(output);
Copy link
Member

Choose a reason for hiding this comment

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

It's possible that webResponse.GetResponseStream() returns null, so you should check for null here. This can be easiest done as:

Suggested change
input.CopyTo(output);
input?.CopyTo(output);

}
}
}
else
{
currentEtag = webResponse.Headers.Get("Etag");
File.WriteAllText(etagPath, currentEtag);
if (webResponse.StatusCode != HttpStatusCode.NotModified)
{
using Stream output = File.OpenWrite(cachedAllTagsPath);
using var input = webResponse.GetResponseStream();
input.CopyTo(output);
}
}
catch (WebException)
{
}
catch (UnauthorizedAccessException)
{
}
catch (IOException)
{
}
}
_languageTags = new ReadOnlyKeyedCollection<string, SldrLanguageTagInfo>(ParseAllTagsJson(cachedAllTagsPath));
catch (WebException)
{
}
catch (UnauthorizedAccessException)
{
}
catch (IOException)
{
}
}


private static IKeyedCollection<string, SldrLanguageTagInfo> ParseAllTagsJson(string cachedAllTagsPath)
{
// read in the json file
Expand Down