| Summary: | Empty (undef) system preferences may cause some issues in combination with memcache | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Jacek Ablewicz <abl> |
| Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | brendan, jonathan.druart, mtj, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 11998 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 16070: Default value for sysprefs should be an empty string
[SIGNED-OFF] Bug 16070: Default value for sysprefs should be an empty string Bug 16070: Add atomic update file Bug 16070: Default value for sysprefs should be an empty string Bug 16070: Add atomic update file |
||
|
Description
Jacek Ablewicz
2016-03-14 17:32:05 UTC
bug 11998 comment 113: """ For sysprefs with undef value in the database - while testing with memcache - there are 2 warnings generated: Use of uninitialized value in subroutine entry at /home/koha/devkohaclone/Koha/Cache.pm line 287. Use of uninitialized value in subroutine entry at /home/koha/devkohaclone/Koha/Cache.pm line 287. This only happens when trying to fetch such syspref first time, from unpopulated cache. There are 9 such values in my test database: CoceHost CoceProviders DefaultLongOverdueChargeValue DefaultLongOverdueDays DefaultLongOverdueLostValue MembershipExpiryDaysNotice NovelistSelectPassword NovelistSelectProfile TagsExternalDictionary Non-existing preference fetch generates the same warning (but again only once). Note that subsequent C4::Context->preference() calls will return empty string for such preferences, instead of undef value - not sure if that may cause some problems or not, but it's a change from previous behaviour. Also it does not happen for Cache::Memory (= default caching system after this patch). """ In the case of sysprefs, it does not make sense to have them with an undefined value. They should be updated. Created attachment 49164 [details] [review] Bug 16070: Default value for sysprefs should be an empty string The default value for sysprefs should not be NULL but an empty string. When a pref is cleared, it's set to an empty string, so it does not make sense to create it with an undefined value. The main purpose of this patch is to remove the warning in logs when a pref is accessed for the first time and the cache is not yet populated. It also ensures that the behavior will be the same for the first access and the others. Test plan: SELECT COUNT(*) FROM systempreferences WHERE value IS NULL; Should not return any results after the update DB entry executed. Created attachment 49196 [details] [review] [SIGNED-OFF] Bug 16070: Default value for sysprefs should be an empty string The default value for sysprefs should not be NULL but an empty string. When a pref is cleared, it's set to an empty string, so it does not make sense to create it with an undefined value. The main purpose of this patch is to remove the warning in logs when a pref is accessed for the first time and the cache is not yet populated. It also ensures that the behavior will be the same for the first access and the others. Test plan: SELECT COUNT(*) FROM systempreferences WHERE value IS NULL; Should not return any results after the update DB entry executed. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Created attachment 49254 [details] [review] Bug 16070: Add atomic update file I forgot the atomic update file! Created attachment 49550 [details] [review] Bug 16070: Default value for sysprefs should be an empty string The default value for sysprefs should not be NULL but an empty string. When a pref is cleared, it's set to an empty string, so it does not make sense to create it with an undefined value. The main purpose of this patch is to remove the warning in logs when a pref is accessed for the first time and the cache is not yet populated. It also ensures that the behavior will be the same for the first access and the others. Test plan: SELECT COUNT(*) FROM systempreferences WHERE value IS NULL; Should not return any results after the update DB entry executed. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 49551 [details] [review] Bug 16070: Add atomic update file Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pushed to Master - Should be in the May 2016 release. Thanks! Added to Koha 16.05.00 release |