You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ponymail.apache.org by sebb <se...@gmail.com> on 2017/08/14 09:50:20 UTC
Re: incubator-ponymail git commit: Address null entries in favorites list
On 14 August 2017 at 10:43, <hu...@apache.org> wrote:
> Repository: incubator-ponymail
> Updated Branches:
> refs/heads/master a5c98e735 -> f09765481
>
>
> Address null entries in favorites list
>
> This fixes #392 by both using the proper call to remove
> favorite entries when needed, and also pruning existing
> corrupted entries when an account is loaded.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/repo
> Commit: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/commit/f0976548
> Tree: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/tree/f0976548
> Diff: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/diff/f0976548
>
> Branch: refs/heads/master
> Commit: f09765481bb35c94b005c42f70a28894462a9430
> Parents: a5c98e7
> Author: Daniel Gruno <hu...@apache.org>
> Authored: Mon Aug 14 11:43:10 2017 +0200
> Committer: Daniel Gruno <hu...@apache.org>
> Committed: Mon Aug 14 11:43:10 2017 +0200
>
> ----------------------------------------------------------------------
> CHANGELOG.md | 1 +
> site/api/lib/user.lua | 13 +++++++++++++
> site/api/preferences.lua | 2 +-
> 3 files changed, 15 insertions(+), 1 deletion(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/f0976548/CHANGELOG.md
> ----------------------------------------------------------------------
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index e3a6b10..05fab5d 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -1,4 +1,5 @@
> ## CHANGES in 0.10:
> +- Fixed an issue where favorites could contain null entries due to missing GC (#392)
> - Added sample configs for Pony Mail (#374)
> - Renamed ll.py to list-lists.py (#378)
> - ID generators have now been split into a separate library (generators.py)
>
> http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/f0976548/site/api/lib/user.lua
> ----------------------------------------------------------------------
> diff --git a/site/api/lib/user.lua b/site/api/lib/user.lua
> index 57c65ac..e441476 100644
> --- a/site/api/lib/user.lua
> +++ b/site/api/lib/user.lua
> @@ -17,6 +17,7 @@
>
> local elastic = require 'lib/elastic'
> local config = require 'lib/config'
> +local JSON = require 'cjson'
>
> -- allow local override of secure cookie attribute
> -- Note: the config item is named to make it more obvious that enabling it is not recommended
> @@ -32,6 +33,18 @@ local function getUser(r, override)
> if override or (cookie and #cookie >= 40 and cid) then
> local js = elastic.get('account', r:sha1(override or cid), true)
> if js and js.credentials and (override or (cookie == js.internal.cookie)) then
> +
> + -- Issue #392: favorites may contain null entries, cull them.
> + if type(js.favorites) == "table" then
> + local jsfav = {}
> + for k, v in pairs(js.favorites) do
> + if v and v ~= JSON.null then
> + table.insert(jsfav, v)
> + end
> + end
> + js.favorites = jsfav
> + end
> +
The above is a temporary fix and should be documented as such.
It's not needed for new installations, nor ones where the erroneous
null entries have been manually removed from the database.
> login = {
> credentials = {
> email = js.credentials.email,
>
> http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/f0976548/site/api/preferences.lua
> ----------------------------------------------------------------------
> diff --git a/site/api/preferences.lua b/site/api/preferences.lua
> index 384cf1e..7f20f65 100644
> --- a/site/api/preferences.lua
> +++ b/site/api/preferences.lua
> @@ -253,7 +253,7 @@ Pony Mail - Email for Ponies and People.
> -- ensure it's here....
> for k, v in pairs(favs) do
> if v == rem then
> - favs[k] = nil
> + table.remove(favs, k)
> break
> end
> end
>
Re: incubator-ponymail git commit: Address null entries in favorites list
Posted by sebb <se...@gmail.com>.
On 14 August 2017 at 10:58, Daniel Gruno <hu...@apache.org> wrote:
> On 08/14/2017 11:50 AM, sebb wrote:
>> On 14 August 2017 at 10:43, <hu...@apache.org> wrote:
>>> Repository: incubator-ponymail
>>> Updated Branches:
>>> refs/heads/master a5c98e735 -> f09765481
>>>
>>>
>>> Address null entries in favorites list
>>>
>>> This fixes #392 by both using the proper call to remove
>>> favorite entries when needed, and also pruning existing
>>> corrupted entries when an account is loaded.
>>>
>>>
>>> Project: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/repo
>>> Commit: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/commit/f0976548
>>> Tree: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/tree/f0976548
>>> Diff: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/diff/f0976548
>>>
>>> Branch: refs/heads/master
>>> Commit: f09765481bb35c94b005c42f70a28894462a9430
>>> Parents: a5c98e7
>>> Author: Daniel Gruno <hu...@apache.org>
>>> Authored: Mon Aug 14 11:43:10 2017 +0200
>>> Committer: Daniel Gruno <hu...@apache.org>
>>> Committed: Mon Aug 14 11:43:10 2017 +0200
>>>
>>> ----------------------------------------------------------------------
>>> CHANGELOG.md | 1 +
>>> site/api/lib/user.lua | 13 +++++++++++++
>>> site/api/preferences.lua | 2 +-
>>> 3 files changed, 15 insertions(+), 1 deletion(-)
>>> ----------------------------------------------------------------------
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/f0976548/CHANGELOG.md
>>> ----------------------------------------------------------------------
>>> diff --git a/CHANGELOG.md b/CHANGELOG.md
>>> index e3a6b10..05fab5d 100644
>>> --- a/CHANGELOG.md
>>> +++ b/CHANGELOG.md
>>> @@ -1,4 +1,5 @@
>>> ## CHANGES in 0.10:
>>> +- Fixed an issue where favorites could contain null entries due to missing GC (#392)
>>> - Added sample configs for Pony Mail (#374)
>>> - Renamed ll.py to list-lists.py (#378)
>>> - ID generators have now been split into a separate library (generators.py)
>>>
>>> http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/f0976548/site/api/lib/user.lua
>>> ----------------------------------------------------------------------
>>> diff --git a/site/api/lib/user.lua b/site/api/lib/user.lua
>>> index 57c65ac..e441476 100644
>>> --- a/site/api/lib/user.lua
>>> +++ b/site/api/lib/user.lua
>>> @@ -17,6 +17,7 @@
>>>
>>> local elastic = require 'lib/elastic'
>>> local config = require 'lib/config'
>>> +local JSON = require 'cjson'
>>>
>>> -- allow local override of secure cookie attribute
>>> -- Note: the config item is named to make it more obvious that enabling it is not recommended
>>> @@ -32,6 +33,18 @@ local function getUser(r, override)
>>> if override or (cookie and #cookie >= 40 and cid) then
>>> local js = elastic.get('account', r:sha1(override or cid), true)
>>> if js and js.credentials and (override or (cookie == js.internal.cookie)) then
>>> +
>>> + -- Issue #392: favorites may contain null entries, cull them.
>>> + if type(js.favorites) == "table" then
>>> + local jsfav = {}
>>> + for k, v in pairs(js.favorites) do
>>> + if v and v ~= JSON.null then
>>> + table.insert(jsfav, v)
>>> + end
>>> + end
>>> + js.favorites = jsfav
>>> + end
>>> +
>>
>> The above is a temporary fix and should be documented as such.
>
> Temporary for how long though? We don't control when people upgrade, so
> the issue could be present tomorrow, a year from now, whenever. I'd hold
> off on calling it temporary till there's something that can actually
> replace this. I don't currently know what a more permanent solution
> would look like, save maybe a python script to clean up the database,
> which admins would then have to run manually on the box.
It's a temporary fix because:
- it does not affect new installations
- it only affects a small percentage of existing users in current installations
- it only requires a once-off admin job to tidy up any existing entries
Yes, it's a bit more work to create such a script.
But that's sometimes necessary to tidy up buggy data.
If it were really difficult to write a script to fix the database
entries, then I agree that a work-round such as this might be worth
considering.
But that is not the case here.
So I think I am actually now -1 on the work-round.
> We can document in the code that it's not the _optimal_ fix, perhaps -
> something for people to look at if they wanna try out contributing to PM...
>> It's not needed for new installations, nor ones where the erroneous
>> null entries have been manually removed from the database.
>>
>>> login = {
>>> credentials = {
>>> email = js.credentials.email,
>>>
>>> http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/f0976548/site/api/preferences.lua
>>> ----------------------------------------------------------------------
>>> diff --git a/site/api/preferences.lua b/site/api/preferences.lua
>>> index 384cf1e..7f20f65 100644
>>> --- a/site/api/preferences.lua
>>> +++ b/site/api/preferences.lua
>>> @@ -253,7 +253,7 @@ Pony Mail - Email for Ponies and People.
>>> -- ensure it's here....
>>> for k, v in pairs(favs) do
>>> if v == rem then
>>> - favs[k] = nil
>>> + table.remove(favs, k)
>>> break
>>> end
>>> end
>>>
>
Re: incubator-ponymail git commit: Address null entries in favorites
list
Posted by Daniel Gruno <hu...@apache.org>.
On 08/14/2017 11:50 AM, sebb wrote:
> On 14 August 2017 at 10:43, <hu...@apache.org> wrote:
>> Repository: incubator-ponymail
>> Updated Branches:
>> refs/heads/master a5c98e735 -> f09765481
>>
>>
>> Address null entries in favorites list
>>
>> This fixes #392 by both using the proper call to remove
>> favorite entries when needed, and also pruning existing
>> corrupted entries when an account is loaded.
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/commit/f0976548
>> Tree: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/tree/f0976548
>> Diff: http://git-wip-us.apache.org/repos/asf/incubator-ponymail/diff/f0976548
>>
>> Branch: refs/heads/master
>> Commit: f09765481bb35c94b005c42f70a28894462a9430
>> Parents: a5c98e7
>> Author: Daniel Gruno <hu...@apache.org>
>> Authored: Mon Aug 14 11:43:10 2017 +0200
>> Committer: Daniel Gruno <hu...@apache.org>
>> Committed: Mon Aug 14 11:43:10 2017 +0200
>>
>> ----------------------------------------------------------------------
>> CHANGELOG.md | 1 +
>> site/api/lib/user.lua | 13 +++++++++++++
>> site/api/preferences.lua | 2 +-
>> 3 files changed, 15 insertions(+), 1 deletion(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/f0976548/CHANGELOG.md
>> ----------------------------------------------------------------------
>> diff --git a/CHANGELOG.md b/CHANGELOG.md
>> index e3a6b10..05fab5d 100644
>> --- a/CHANGELOG.md
>> +++ b/CHANGELOG.md
>> @@ -1,4 +1,5 @@
>> ## CHANGES in 0.10:
>> +- Fixed an issue where favorites could contain null entries due to missing GC (#392)
>> - Added sample configs for Pony Mail (#374)
>> - Renamed ll.py to list-lists.py (#378)
>> - ID generators have now been split into a separate library (generators.py)
>>
>> http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/f0976548/site/api/lib/user.lua
>> ----------------------------------------------------------------------
>> diff --git a/site/api/lib/user.lua b/site/api/lib/user.lua
>> index 57c65ac..e441476 100644
>> --- a/site/api/lib/user.lua
>> +++ b/site/api/lib/user.lua
>> @@ -17,6 +17,7 @@
>>
>> local elastic = require 'lib/elastic'
>> local config = require 'lib/config'
>> +local JSON = require 'cjson'
>>
>> -- allow local override of secure cookie attribute
>> -- Note: the config item is named to make it more obvious that enabling it is not recommended
>> @@ -32,6 +33,18 @@ local function getUser(r, override)
>> if override or (cookie and #cookie >= 40 and cid) then
>> local js = elastic.get('account', r:sha1(override or cid), true)
>> if js and js.credentials and (override or (cookie == js.internal.cookie)) then
>> +
>> + -- Issue #392: favorites may contain null entries, cull them.
>> + if type(js.favorites) == "table" then
>> + local jsfav = {}
>> + for k, v in pairs(js.favorites) do
>> + if v and v ~= JSON.null then
>> + table.insert(jsfav, v)
>> + end
>> + end
>> + js.favorites = jsfav
>> + end
>> +
>
> The above is a temporary fix and should be documented as such.
Temporary for how long though? We don't control when people upgrade, so
the issue could be present tomorrow, a year from now, whenever. I'd hold
off on calling it temporary till there's something that can actually
replace this. I don't currently know what a more permanent solution
would look like, save maybe a python script to clean up the database,
which admins would then have to run manually on the box.
We can document in the code that it's not the _optimal_ fix, perhaps -
something for people to look at if they wanna try out contributing to PM...
>
> It's not needed for new installations, nor ones where the erroneous
> null entries have been manually removed from the database.
>
>> login = {
>> credentials = {
>> email = js.credentials.email,
>>
>> http://git-wip-us.apache.org/repos/asf/incubator-ponymail/blob/f0976548/site/api/preferences.lua
>> ----------------------------------------------------------------------
>> diff --git a/site/api/preferences.lua b/site/api/preferences.lua
>> index 384cf1e..7f20f65 100644
>> --- a/site/api/preferences.lua
>> +++ b/site/api/preferences.lua
>> @@ -253,7 +253,7 @@ Pony Mail - Email for Ponies and People.
>> -- ensure it's here....
>> for k, v in pairs(favs) do
>> if v == rem then
>> - favs[k] = nil
>> + table.remove(favs, k)
>> break
>> end
>> end
>>