-
Notifications
You must be signed in to change notification settings - Fork 50
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
Standardize/dedupe external-request functions,leveraging caching where worthwhile #107
Comments
This is a list of all the "external resources" that the script needs to fetch (through various different calls):
Additionally there are two other "resources" that can cause troubles when "accessed" incorrectly; they are not properly "external resources", but they still need to be dealt with:
Lastly, there is Having established this I concur that we need a way to standardize these external calls (and remove all the redundant try/null checks). I'd propose to:
Something like this: // Enum
var extResType = {
WEBPAGE: 'URL',
G_CONTACT: 'GCONT',
...
};
function fetchExternalResource(resourceIdentifier, type) {
switch (type) {
case extResType.G_CONTACT: // returns null on failure
result = ContactsApp.getContactById(resourceIdentifier);
if (result === null) { throw Error(); }
return result;
case extResType.WEBPAGE// throws error on failure
return UrlFetchApp.fetch(resourceIdentifier);
case extResType.XYZ:
...
}
}
LocalCache.prototype.fetch = function (resourceIdentifier, type, retry) {
// Use fetchExternalResource() instead of UrlFetchApp.fetch().
uniqueId = type + ':' + resourceIdentifier;
this.cache[uniqueId] = response;
return this.cache[uniqueId];
}
LocalCache.prototype.isCached = function (resourceIdentifier, type) {
uniqueId = type + ':' + resourceIdentifier;
return !!this.cache[uniqueId];
};
LocalCache.prototype.retrieve= function (resourceIdentifier, type, retry) {
...
}
// Needs caching.
try {
cache.retrieve('http://example.org', extResType.WEBPAGE);
} ...
// Does not need caching.
try {
fetchExternalResource('http://example.org', extResType.WEBPAGE);
}...
// Not an external request.
try {
JSON.parse(str);
}... |
I'm opening this issue to continue the thread started about here on this now-closed pull-request. I won't copy-paste the existing thread here, it is easy enough to click through to read it.
The text was updated successfully, but these errors were encountered: