Review Request: Stop DataContainer from discarding multiple values for a single key.

Marco Martin notmart at gmail.com
Sat Jun 5 11:29:02 CEST 2010


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4235/#review5992
-----------------------------------------------------------

Ship it!


good catch.
could also have something to do wwith https://bugs.kde.org/show_bug.cgi?id=240462  ?

- Marco


On 2010-06-05 00:08:34, John Layt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4235/
> -----------------------------------------------------------
> 
> (Updated 2010-06-05 00:08:34)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> If a DataEngine implementation uses QHash::insertMulti() to try return multiple values for a key, the DataContainer discards the multiple values and only returns a single value to the client.
> 
> A simplified example showing where 2 holidays occurring on the same date need to be treated as separate data entities:
> 
>         Plasma::DataEngine::Data holidays;
>         holidays.insertMulti("2010-06-01", "A national public holiday");
>         holidays.insertMulti("2010-06-01", "A minor religious holiday");
>         setData(request, holidays);
> 
> What gets returned to the client is only the second value for the minor religious holiday, so the public holiday doesn't appear on the calendar.
> 
> This is because the call to setData() loops through each value in the passed in DataEngine::Data and calls DataContainer::setData(), which inserts each value into a new DataEngine::Data, in the process discarding any previous multiple values.
> 
> This patch uses insertMulti() in the DataContainer::setData() to fix this.  I can't think of anything this would break by changing the behaviour to retain any multiples, they can only get there in the first place by the DataEngine explicitly calling insertMulti() so I think it's safe to assume the coder wants them kept.
> 
> The alternative would be to create new DataEngine::setMultiData() and DataContainer::setMultiData() methods.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/datacontainer.cpp 1134380 
> 
> Diff: http://reviewboard.kde.org/r/4235/diff
> 
> 
> Testing
> -------
> 
> Without the patch, holidays disappear between the calendar data engine and the calendar plasmoid.  With the patch all the holidays make it and can be treated differently according to type.
> 
> 
> Thanks,
> 
> John
> 
>



More information about the Plasma-devel mailing list