-
Notifications
You must be signed in to change notification settings - Fork 14
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
Duplicate of asyncEntity #1387
Duplicate of asyncEntity #1387
Conversation
@avazirna @ctsims pinging for review. I'm hoping to get some of these duplicate PRs merged while Shubham is away. There are a few backlogged where one depends on the previous being merged. This is the first of those. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1g5 looks good, just left a small comment
synchronized (mAsyncLock) { | ||
if (sortData[i] == null) { | ||
// sort data not in search field cache; load and store it | ||
Text sortText = fields[i].getSort(); | ||
if (sortText == null) { | ||
mEntityStorageCache.releaseCache(); | ||
return null; | ||
} | ||
|
||
String cacheKey = mEntityStorageCache.getCacheKey(mDetailId, String.valueOf(i)); | ||
|
||
if (mCacheIndex != null) { | ||
//Check the cache! | ||
String value = mEntityStorageCache.retrieveCacheValue(mCacheIndex, cacheKey); | ||
if (value != null) { | ||
this.setSortData(i, value); | ||
mEntityStorageCache.releaseCache(); | ||
return sortData[i]; | ||
} | ||
} | ||
|
||
loadVariableContext(); | ||
try { | ||
sortText = fields[i].getSort(); | ||
if (sortText == null) { | ||
this.setSortData(i, getFieldString(i)); | ||
} else { | ||
this.setSortData(i, StringUtils.normalize(sortText.evaluate(context))); | ||
} | ||
|
||
mEntityStorageCache.cache(mCacheIndex, cacheKey, sortData[i]); | ||
} catch (XPathException xpe) { | ||
Logger.exception("Error while evaluating sort field", xpe); | ||
xpe.printStackTrace(); | ||
sortData[i] = "<invalid xpath: " + xpe.getMessage() + ">"; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code was previously wrapped around a try..finally
block, which would close the transaction no matter the outcome. How certain are we that this code will always be successful?
duplicate this PR |
Copy of PR #1386
GitHub Actions automatically ran duplicate_pr.py and used GitHub CLI to open this pull request.
Please close and reopen this pull request to have tests run.†
† Workaround for this GitHub Actions constraint