Condition fails with 'Required argument "file_name" undefined' error

[netsensei]

Okay, trying to work with conditions in the ini file, but I get an error.

Here's what I do in my pipeline config:

[plugin_fixer_Fix]
condition = record.institution_name
fixers = MSK, GRO
id_path = 'lidoRecID.0._'

[plugin_fixer_MSK]
condition = 'Museum voor Schone Kunsten Gent'
file_name = '/Users/matthiasvandermaesen/Workspace/Datahub-Fixes/msk_oai_adlib.fix'

[plugin_fixer_GRO]
condition = 'Groeningemuseum Brugge'
file_name = '/Users/matthiasvandermaesen/Workspace/Datahub-Fixes/msk_oai_adlib.fix'

This isn't any different from what's in the transport.pm pod. But I do get this error + stack trace:

Use of uninitialized value in string eq at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Fixer/Condition.pm line 71.
Use of uninitialized value in string eq at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Fixer/Condition.pm line 71.
2017/05/17 16:57:44 [56432] - ERROR Catmandu::Error::BUILD /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Catmandu/Error.pm (22) time=25500 : Required argument "file_name" undefined

Trace begun at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Fixer/Fix.pm line 11
Datahub::Factory::Fixer::Fix::__ANON__(undef) called at (eval 801) line 58
eval {...} at (eval 801) line 57
Datahub::Factory::Fixer::Fix::new('Datahub::Factory::Fixer::Fix', 'file_name', undef) called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Fixer/Condition.pm line 74
Datahub::Factory::Fixer::Condition::_build_fix_module('Datahub::Factory::Fixer::Condition=HASH(0x7f9b43d85fe8)') called at (eval 386) line 20
Datahub::Factory::Fixer::Condition::fix_module('Datahub::Factory::Fixer::Condition=HASH(0x7f9b43d85fe8)') called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Command/transport.pm line 110
Datahub::Factory::Command::transport::try {...}  at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Try/Tiny.pm line 103
eval {...} at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Try/Tiny.pm line 94
Try::Tiny::try('CODE(0x7f9b45b48818)', 'Try::Tiny::Catch=REF(0x7f9b45b496b0)') called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Command/transport.pm line 120
Datahub::Factory::Command::transport::try {...}  at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Try/Tiny.pm line 101
eval {...} at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Try/Tiny.pm line 94
Try::Tiny::try('CODE(0x7f9b45b4a0e8)', 'Try::Tiny::Catch=REF(0x7f9b45b4a698)') called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Command/transport.pm line 133
Datahub::Factory::Command::transport::__ANON__('HASH(0x7f9b422cd8c0)') called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Catmandu/Iterable.pm line 88
Catmandu::Iterable::each('Catmandu::Importer::OAI=HASH(0x7f9b449839e0)', 'CODE(0x7f9b43d727c0)') called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Importer.pm line 24
Datahub::Factory::Importer::try {...}  at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Try/Tiny.pm line 103
eval {...} at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Try/Tiny.pm line 94
Try::Tiny::try('CODE(0x7f9b4171d3f0)', 'Try::Tiny::Catch=REF(0x7f9b4171d408)') called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Importer.pm line 34
Datahub::Factory::Importer::each('Datahub::Factory::Importer::OAI=HASH(0x7f9b44ec87a8)', 'CODE(0x7f9b43d727c0)') called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Command/transport.pm line 165
Datahub::Factory::Command::transport::execute('Datahub::Factory::Command::transport=HASH(0x7f9b43da8c38)', 'Getopt::Long::Descriptive::Opts::__OPT__::3=HASH(0x7f9b43d72508)', 'ARRAY(0x7f9b43bf9a48)') called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/App/Cmd.pm line 468
App::Cmd::execute_command('Datahub::Factory::CLI=HASH(0x7f9b43d20600)', 'Datahub::Factory::Command::transport=HASH(0x7f9b43da8c38)', 'Getopt::Long::Descriptive::Opts::__OPT__::3=HASH(0x7f9b43d72508)') called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/CLI.pm line 100
Datahub::Factory::CLI::run('Datahub::Factory::CLI') called at /Users/matthiasvandermaesen/.plenv/versions/5.22.0/bin/dhconveyor line 5
2017/05/17 16:27:02 [53978] - FATAL Datahub::Factory::Command::transport::catch {...}  /Users/matthiasvandermaesen/.plenv/versions/5.22.0/lib/perl5/site_perl/5.22.0/Datahub/Factory/Command/transport.pm (118) time=12398 : Required argument "file_name" undefined

which is weird because file_name IS defined in the config file. I updated Catmandu to the latest stable version (1.0506) but that doesn't change the output. Still getting that error.

Needs investigating...

[netsensei]

Okay, isolated the problem.

On line 69 in Condition.pm we see this code:

        if ($self->options->{sprintf('fixer_%s', $fixer)}->{'condition'} eq $self->condition) {
            $fix_file_name = $self->options->{sprintf('fixer_%s', $fixer)}->{'file_name'};
            last;
        }

Problem: $self->condition is never initialised. This causes $fix_file_name to stay empty which triggers the BadArg block in Fix.pm.

[pieterdp]

$self->condition should be initialised by $self->_build_condition() - I think this happens when $self->options->{sprintf('fixer_%s', $fixer)}->{'condition'} never equals $self->condition. Will investigate.

[netsensei]

I think $self->condition is empty when the path can't be found in the XML or doesn't yield a value. That's why it borks.

[pieterdp]

This has been partially fixed in 43157ec, but:

Currently, transport.pm does not allow us to distinguish between different kinds of error (e.g. the difference between Catmandu::BadArg and Catmandu::BadVal. So we are faced with the choice between die-ing on every error the Conditional fixer generates, or never.

Currently, the Conditional fixer generates two kinds of errors: errors in loading modules (missing arguments, missing modules etc.). This should be fatal (we can not recover from this). But, to fix #29, it now also generates a "Fixer not found for condition", which is recoverable (the following item could have a condition that matches one of the fixers).

I have elected never to die. Otherwise, if item x (about halfway the run) does not match any of the fixers, the run ends and you have to start over. We don't want that. Downside is that if we get a unrecoverable error, it will print errors for the entire run, one item at the time.

A solution for this could be the implementation of a Datahub::Factory::Error-module that would be able to handle two different kinds of error, with different actions.