Skip to content

Commit

Permalink
Fix Demeter violations in ZencoderVideoOutputDefinition
Browse files Browse the repository at this point in the history
Introduce `VideoFile#encode_highdef?` method to remove duplication of
feature name.
  • Loading branch information
tf committed Aug 16, 2017
1 parent a672e75 commit 81ccc6b
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 6 deletions.
4 changes: 4 additions & 0 deletions app/models/pageflow/video_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ def attachment_s3_url
"s3://#{File.join(attachment_on_s3.bucket_name, attachment_on_s3.path)}"
end

def encode_highdef?
entry.feature_state('highdef_video_encoding')
end

def mp4_4k
ZencoderAttachment.new(self, '4k.mp4')
end
Expand Down
12 changes: 6 additions & 6 deletions lib/pageflow/zencoder_video_output_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def outputs
private

def mp4_highdef_definitions
return [] unless video_file.entry.feature_state('highdef_video_encoding')
return [] unless video_file.encode_highdef?
[transferable(mp4_4k_definition), transferable(mp4_fullhd_definition)]
end

Expand Down Expand Up @@ -128,7 +128,7 @@ def dash_definitions
end

def dash_highdef_definitions
return [] unless video_file.entry.feature_state('highdef_video_encoding')
return [] unless video_file.encode_highdef?

[
non_transferable(dash_fullhd_definition),
Expand All @@ -149,7 +149,7 @@ def hls_definitions
end

def hls_highdef_definitions
return [] unless video_file.entry.feature_state('highdef_video_encoding')
return [] unless video_file.encode_highdef?

[
non_transferable(hls_fullhd_definition),
Expand Down Expand Up @@ -257,7 +257,7 @@ def dash_stream_definitions
end

def dash_highdef_stream_definitions
return [] unless video_file.entry.feature_state('highdef_video_encoding')
return [] unless video_file.encode_highdef?
[
{
source: 'dash-fullhd',
Expand Down Expand Up @@ -367,7 +367,7 @@ def hls_stream_definitions
end

def hls_highdef_stream_definitions
return [] unless video_file.entry.feature_state('highdef_video_encoding')
return [] unless video_file.encode_highdef?
[
{
source: 'hls-fullhd',
Expand Down Expand Up @@ -404,7 +404,7 @@ def smil_definitions
# these cases the input file is just a tiny bit larger than the
# next lower resolution, so we do not really care if the SMIL
# file which does not include the higher quality does not win.
if video_file.entry.feature_state('highdef_video_encoding')
if video_file.encode_highdef?
[
non_transferable(smil_definition.merge(streams: smil_default_stream_definitions,
skip: {
Expand Down
14 changes: 14 additions & 0 deletions spec/models/pageflow/video_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,19 @@ module Pageflow
expect(video_file.present_outputs).to include(:'hls-playlist')
end
end

describe '#encode_highdef?' do
it 'returns false by default' do
video_file = build(:video_file)

expect(video_file.encode_highdef?).to eq(false)
end

it 'returns true if entry has highdef_video_encoding feature enabled' do
video_file = build(:video_file, :with_highdef_encoding)

expect(video_file.encode_highdef?).to eq(true)
end
end
end
end

4 comments on commit 81ccc6b

@tilsammans
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not part of 12.0.4. Could 12.0.5 be released pretty soon? We just encountered a bug in code that was rightfully removed here.

@tf
Copy link
Member Author

@tf tf commented on 81ccc6b Oct 21, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tilsammans
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the bug you are seeing?

NoMethodError: undefined method `feature_state' for nil:NilClass.

Fyi, a file upload got stuck and this happened.

Just cherry pick what you need and prepare a PR against 12-0-stable.

Will do, but might not happen tomorrow.

@tilsammans
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, the error happened in pageflow/zencoder_video_output_definition.rb in mp4_highdef_definitions at line 37

Please sign in to comment.