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

Duplicate of asyncEntity #1387

Merged
merged 4 commits into from
Apr 10, 2024
Merged

Duplicate of asyncEntity #1387

merged 4 commits into from
Apr 10, 2024

Conversation

github-actions[bot]
Copy link

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

@Robert-Costello
Copy link
Contributor

@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.
One note on this is that I re-ran tests on these changes and the jenkins build failed, but that looks to be a result of there being no new changes.

Copy link
Contributor

@avazirna avazirna left a 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

Comment on lines 126 to 162
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() + ">";
}
}
Copy link
Contributor

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?

@shubham1g5
Copy link
Contributor

duplicate this PR

@shubham1g5 shubham1g5 merged commit cfe3f9b into master Apr 10, 2024
2 checks passed
@shubham1g5 shubham1g5 deleted the copy_of_asyncEntity branch April 10, 2024 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants