Possible issue when requesting a coverdoc directly

[jesusbagpuss]

I'm investigating this on the OpenOffice version of coversheets - a quick skim of this versions' code looks like it may also be an issue here.

$docA (https://repo.com/1/1/doc.pdf) has a covered version $docB. $docB can be directly accessed via e.g. https://repo.com/1/2/doc.pdf (pos=2)

When the DOC_REWRITE_TRIGGER is fired for a request directly to $docB (the covered version), it returns early (from 'isVolatileVersion' test) - before any regeneration tests have occurred.

If a document was OA, but subsequently made non-public, $docB may be accessible via the 'direct' route - at least until a request via $docA is made (and subsequent regeneration has occured).

Is there code that I have overlooked that hooks into a $doc->commit to update e.g. security data of the coversheeted document (the line: $coverdoc->set_value( "security", $doc->get_value( "security" ) ); in the DOC_URL_REWRITE trigger isn't enough in the case above.

One possible resolution to the issue is to add this at the start of the DOC_URL_REWRITE trigger - although I haven't thoroughly tested this yet (I'll add a PR if this does resolve it for the OO coversheet plugin):

my $requested_coverdoc;
if( $doc->has_relation( undef, "isCoversheetVersionOf" ) )
{
    $requested_coverdoc = $doc;
    my @owning_docs = $doc->related_dataobjs( EPrints::Utils::make_relation( "isCoversheetVersionOf" ) );
    if( scalar @owning_docs > 1 )
    {
        $doc->{session}->log( "Coversheet trigger: multiple owning docs returned" );
    }
    $doc = $owning_docs[0];
}

I have not got this extention enabled on a repository - so the above may not be an issue - but I think it's worth someone checking!

[jesusbagpuss]

@cziaarm / @mpbraendle there may be a more fundamental question here:

should coversheeted versions be directly addressable?

The above addition to the rewrite trigger could return a redirect to the owning document, or some other http response (and not the coversheet).

[mpbraendle]

Thank you for pointing out the issue. I will check with @pjwest.

[jesusbagpuss]

@mpbraendle - thanks. If it helps @pjwest, this is the doc commit trigger I'm experimenting with (alongside the change outlined above):

### detect change in security flag, and a coveresheet existing ###
$c->add_dataset_trigger( 'document', EPrints::Const::EP_TRIGGER_AFTER_COMMIT, sub
{
        my( %args ) = @_;
        my( $repo, $doc, $changed ) = @args{qw( repository dataobj changed )};

        my $coverdoc = EPrints::DataObj::Coversheet->get_coversheet_doc( $doc );

        # NB $changed contains the old values for the field.
        if( defined $coverdoc && $changed->{security} ){
                $coverdoc->set_value( "security", $doc->get_value( "security" ) );
                $coverdoc->commit;
        }

}, priority => 100 );

[pjwest]

Hi

There are a couple of potential modifications that could be made such as:

1. extend the status change trigger (latex version) to act on commit.
2. utilise an extended version of the "regeneration tests" before simply returning if the relation is "isCoversheetVersionOf"

Of the two I think I would favour adding an extended regeneration test (comparing modification time of the eprint with the generation time of the coversheet version of the document. The code for the commit trigger version is potentially simpler but I would be concerned about the frequency at which such a trigger would be called.

For the specific case of the Latex version there is another trigger that operates on status changes and the coversheet should be updated on metadata changes but possibly not on document security changes. The changes to metadata would, however, not be picked up if the user has requested the coversheet version directly. However, there are also scripts provided in the Latex version to remove or apply coversheets to whole archives or individual items. When items in the live archive are edited these scripts should be used as part of the procedure to update the coversheet for the modified item.

On 20/03/18 07:21, jesusbagpuss wrote:

@mpbraendle https://github.com/mpbraendle - thanks. If it helps @pjwest https://github.com/pjwest, this is the doc commit trigger I'm experimenting with:

detect change in security flag, and a coveresheet existing

$c->add_dataset_trigger( 'document', EPrints::Const::EP_TRIGGER_AFTERCOMMIT, sub { my( %args ) = @; my( $repo, $doc, $changed ) = @args{qw( repository dataobj changed )};

    my $coverdoc = EPrints::DataObj::Coversheet->get_coversheet_doc( $doc );

    # NB $changed contains the old values for the field.
    if( defined $coverdoc && $changed->{security} ){
            $coverdoc->set_value( "security", $doc->get_value( "security" ) );
            $coverdoc->commit;
    }

}, priority => 100 );

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eprintsug/coversheets-latex/issues/2#issuecomment-374499533, or mute the thread https://github.com/notifications/unsubscribe-auth/ACyVS9eW-VaG6d-YO7_IyPP8YGMRMjKNks5tgK4VgaJpZM4Swexz.