ImageMagickCommandFactory: only extract first layer for PDFs

[dunn]

Should fix black images; see samvera/hydra-derivatives#110 and samvera/hydra-derivatives#120

[dunn]

Mostly for brainstorming cc @jcoyne

[coveralls]

Coverage decreased (-3.2%) to 89.818% when pulling fc71e61 on dunn:flatten into 6acf20d on curationexperts:master.

[jcoyne]

I'd prefer we make ImageInformation have a dimensions attribute and add the format attribute. Is there a reason we can't do that?

[dunn]

Yeah, I think I was confused by the weird override of the initialize method. Updated now though.

[dunn]

Getting a bunch of failures like this:

  17) Riiif::Image#render quality when bitonal is specified converts to bitonal
      Failure/Error:
        height, width, format = Riiif::CommandRunner.execute(
          "#{external_command} -format '%h %w %m' #{@path}[0]"
        ).split(' ')

      NoMethodError:
        undefined method `split' for nil:NilClass
      # ./app/services/riiif/image_magick_info_extractor.rb:15:in `extract'
      # ./app/models/riiif/file.rb:53:in `info'
      # ./app/models/riiif/image.rb:19:in `block (2 levels) in <class:Image>'
      # ./app/models/riiif/image.rb:18:in `block in <class:Image>'
      # ./app/models/riiif/image.rb:53:in `info'
      # ./app/models/riiif/image.rb:47:in `block in render'
      # ./app/models/riiif/image.rb:46:in `render'
      # ./spec/models/riiif/image_spec.rb:282:in `block (4 levels) in <top (required)>'
      # ./spec/models/riiif/image_spec.rb:332:in `block (5 levels) in <top (required)>'

There must be something buggy with the CommandRunner, because when I run the same command directly with popen3 it works fine.

[coveralls]

Coverage decreased (-2.3%) to 90.468% when pulling f7ac03e on dunn:flatten into 3584485 on curationexperts:master.

[dunn]

Why do we benchmark the commands, anyway?

[jcoyne]

Thanks! Looks great.

[jcoyne]

Looks like there's a few tests to fix up.

[dunn]

@jcoyne yeah, that's the CommandRunner stuff that's confusing me.

[dunn]

If the benchmarking isn't required, we could just remove the CommandRunner entirely.

[dunn]

The bizarre thing is I set a byebug if out.nil? debug trigger in #execute; it's never tripped, but somehow the downstream method calling it gets a nil response.

I thought maybe a different #execute method was somehow getting called instead, but I changed everything to be explicitly Riiif::CommandRunner.execute and it still happens.

[dunn]

Messing around, I tried replacing #execute just with backticks and #strip and got encoding errors, but I don't know if that's indicative of anything.

E.g.:

  9) Riiif::Image happy path renders
     Failure/Error: `#{command_builder.command}`.strip

     ArgumentError:
       invalid byte sequence in UTF-8
     # ./app/transformers/riiif/abstract_transformer.rb:20:in `strip'
     # ./app/transformers/riiif/abstract_transformer.rb:20:in `transform'
     # ./app/transformers/riiif/abstract_transformer.rb:8:in `transform'
     # ./app/models/riiif/file.rb:41:in `extract'
     # ./app/models/riiif/image.rb:47:in `block in render'
     # ./app/models/riiif/image.rb:46:in `render'
     # ./spec/models/riiif/image_spec.rb:26:in `block (3 levels) in <top (required)>'

[coveralls]

Coverage decreased (-2.1%) to 90.631% when pulling 7d3d71f on dunn:flatten into 3584485 on curationexperts:master.

[coveralls]

Coverage decreased (-0.2%) to 92.509% when pulling aea6398 on dunn:flatten into 3584485 on curationexperts:master.

[jcoyne]

This no longer tests anything. Can you restore the Riiif::CommandRunner?

[jcoyne]

All the errors are a result of this line being removed.

[coveralls]

Coverage increased (+0.07%) to 92.806% when pulling feba4c4 on dunn:flatten into 3584485 on curationexperts:master.

[jcoyne]

Thanks, this looks great!