You are here: Foswiki>Tasks Web>Item11347 (23 Dec 2014, GeorgeClark)Edit Attach

Item11347: Some Foswiki::Meta::put code makes no sense and it doesn't behave as documented

pencil
Priority: Normal
Current State: Confirmed
Released In: n/a
Target Release: n/a
Applies To: Engine
Component: FoswikiMeta
Branches:
Reported By: GilmarSantosJr
Waiting For:
Last Change By: GeorgeClark
    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:

ObjectMethod put($type, \%args)

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 smile

-- 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 smile - 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
 

ItemTemplate edit

Summary Some Foswiki::Meta::put code makes no sense and it doesn't behave as documented
ReportedBy GilmarSantosJr
Codebase trunk
SVN Range
AppliesTo Engine
Component FoswikiMeta
Priority Normal
CurrentState Confirmed
WaitingFor
Checkins
TargetRelease n/a
ReleasedIn n/a
CheckinsOnBranches
trunkCheckins
masterCheckins
ItemBranchCheckins
Release01x01Checkins
Topic revision: r9 - 23 Dec 2014, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy