Priority: Normal
Current State: Confirmed
Released In: n/a
Target Release: n/a
unless ( $this->{$type} ) {
$this->{$type} = [];
$this->{_indices}->{$type} = {};
}
my $data = $this->{$type};
my $i = 0;
if ($data) {
# overwrite old single value
if ( scalar(@$data) && defined $data->[0]->{name} ) {
delete $this->{_indices}->{$type}->{ $data->[0]->{name} };
}
$data->[0] = $args;
}
else {
$i = push( @$data, $args ) - 1;
}
The
if
block
always is executed, since in worst case
$data
will be a reference to an empty array, what evaluates to
true. The consequence is:
Put a hash of key=value pairs into the given type set in this meta. This will not replace another value with the same name (for that see putKeyed)
is not really true. The problem here is that "fixing" it would probably break a lot of things.
WorkflowPlugin is an example.
So... should we fix the documentation? If so, what is the difference between the
put
and
putKeyed
methods?
I spent a lot of time trying to get
Item8002 done and I also saw an
IRC conversation, so I'm raising the discussion.
--
GilmarSantosJr - 10 Dec 2011
As I wrote on the IRC logs Gilmar linked above, I agree. This code is broken, and there is no easy way to fix it. I guess fixing the documentation is the easiest, but I would rather fix the code, which would break things...
Anyway, as I don't code plugins, I can't tell. For me, the right thing to do is to fix the code, but I'll be happy either way.
--
OlivierRaginel - 10 Dec 2011
I am hoping to get rid of the putKey crap in store2.
basically, someone somewhere in history decided to seriously pre-de-optimise twiki, by making all hash lookups be evaluated into an array - so as to support the idea of an ordered hash. 99% of the accesses are by name, and always have been, but the fast path is coded as array, and then we later hacked in a hash pointer into that array.
in Store2, I intend to replace this shamozzle so as to make serialisation and deserialisation
fast, named lookups hash-instant, and the order can become either an index stored in the elements, or some other hackjob to support the few reasons its used (mostly to allow an implicit ordering of attachments).
so - thank you for pointing out a huge risk in my intention, and I do look forward to being re-reminded when i break lots
--
SvenDowideit - 10 Dec 2011
I don't really understand why recoding the fastpath to be an index by name is such a big risk. The existing "API" (direct access) can be coded using a tie. There is no reason I can think of that any plugin would break.
--
CrawfordCurrie - 18 Dec 2011
excellent
- yes, I'll continue to be Henny-Penny - as it helps me focus when I change ancient code.
--
SvenDowideit - 19 Dec 2011
I'd like to see this fixed but in respect to the IRC conversation noted above I was able to get around it by performing a single invoke of the method.
--
PadraigLennon - 20 Dec 2011
Deferring this to 1.2.
--
GeorgeClark - 28 Feb 2012
Deferring to uinknown...
--
GeorgeClark - 23 Dec 2014