Feature Proposal: Deprecate afterAttachmentSaveHandler

Motivation

During development of SolrPlugin I've found out that the afterAttachmentSaveHandler is actually called too early: while the attachment has already been saved, the corresponding topic was NOT.

From an outer perspective the uploaded file hasn't arrived at the topic it yet as it does not appear as FILEATTACHMENT on the topic. This basically is an inconsistent storage state.

For SolrPlugin, this renders the afterAttachmentSaveHandler useless when indexing topcis and attachemts and part of the properties of the topic - like tags and categories stored in formfields - are propagated to the attachments and part of the properties of attachments go back to the topic - like the attachment name. These information can't be imported as they haven't yet arrived in the store.

Description and Documentation

In theory there are two solutions:

1. fix afterAttachmentSaveHandler
2. introduce a new handler at the correct position of the code path

CDot proposed the latter on IRC. Let's call it afterUploadHandler. It basically takes the same arguments as the afterAttachmentSaveHandler: attrHash, topic, web but is called a few lines later in Foswiki::Meta::attach() right after the point where the topic meta data has been committed to the store.

The job goes along the following lines:

1. add afterUploadHandler to Foswiki::Plugin and mark it for API version 2.1
2. flag afterAttachmentSaveHandler as deprecated
3. apply this patch to Foswiki::Meta (trunk)

   --- lib/Foswiki/Meta.pm (revision 5472)
   +++ lib/Foswiki/Meta.pm (working copy)
   @@ -2218,6 +2218,11 @@
                $opts{name}, $this->{_session}->{user}
            );
        }
   +
   +    if ( $plugins->haveHandlerFor('afterUploadHandler') ) {
   +        $plugins->dispatch( 'afterUploadHandler', $attrs, $this);
   +    }
    }

4. add it to ReleaseHistory
5. add it to the EmptyPlugin documentation

Impact

WhatDoesItAffect:

%WHATDOESITAFFECT%

edit

Implementation

-- Contributors: MichaelDaum - 16 Nov 2009

Discussion

Please don't pass $topic, $web unless there is a really powerful reason for it. Pass the $topicObject instead to any new handler. And if you must pass $topic, $web then pass $web, $topic.

-- CrawfordCurrie - 16 Nov 2009

True.

-- MichaelDaum - 16 Nov 2009

For consistency the beforeAttachmentSaveHandle(attrs, topic, web) should become beforeUploadHandler(attrs, topicObj)

Btw. DBCacheContrib / DBCachePlugin fail to update the cache incrementally if a new file is attached: Tasks.Item8328.

-- MichaelDaum - 17 Nov 2009

AcceptedBy14DayRule ?

-- MichaelDaum - 30 Nov 2009

imo yes - I've used it, but I also find the legacy handlers a problem for similar reasons.

-- SvenDowideit - 01 Dec 2009

See also Tasks.Item572 and Tasks.Item2529 for other issues with the attachment plugin API before finalizing your design. These all should be addressed at once... e.g. it's not obvious from your description that you're fixing the locking problem described in 572.

-- TimotheLitt - 28 Dec 2009

Tasks.Item8749

-- MichaelDaum - 22 Mar 2010