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

[stable9.1] Add check for empty result in storage memcache #28229

Merged
merged 2 commits into from
Aug 10, 2017

Conversation

PVince81
Copy link
Contributor

Description

It was observed that sometimes the row data is empty which could be due to memcache returning empty arrays instead of null.

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@tomneedham

@PVince81 PVince81 added this to the development milestone Jul 4, 2017
@tomneedham tomneedham self-assigned this Jul 20, 2017
@tomneedham tomneedham self-requested a review July 20, 2017 10:06
@tomneedham tomneedham removed their assignment Jul 20, 2017
@tomneedham tomneedham force-pushed the storage-cache-more-than-null-check branch from fafe8f3 to 3d425db Compare July 20, 2017 10:08
Copy link
Contributor

@tomneedham tomneedham left a comment

Choose a reason for hiding this comment

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

Tested at customer where cache was returning empty arrays instead

@tomneedham
Copy link
Contributor

@PVince81 Forward port? Seems safe.

@PVince81
Copy link
Contributor Author

@tom go for it!

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 1, 2017

I'll take care of the forward ports

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 1, 2017

stable10: #28548
master: #28549

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 1, 2017

let's only merge the stable9.1 once master and co have been merged too

@PVince81 PVince81 self-assigned this Aug 7, 2017
@@ -109,7 +109,7 @@ public static function getStorageById($storageId) {
self::$localCache = new CappedMemoryCache();
}
$result = self::$localCache->get($storageId);
if ($result === null) {
if ($result === null || empty($result) || !isset($result['numeric_id'])) {
Copy link
Member

Choose a reason for hiding this comment

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

Remove empty($result)?

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 9, 2017

@jvillafanez removed here now ffed517

@PVince81
Copy link
Contributor Author

PVince81 commented Aug 9, 2017

rebased

@PVince81 PVince81 force-pushed the storage-cache-more-than-null-check branch from ffed517 to 23d28a1 Compare August 9, 2017 16:49
@PVince81 PVince81 merged commit af29e8f into stable9.1 Aug 10, 2017
@PVince81 PVince81 deleted the storage-cache-more-than-null-check branch August 10, 2017 07:03
@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants