From b6c44523164bbaf94e42df4e3bcdad764c28c20a Mon Sep 17 00:00:00 2001 From: Linas Valiukas Date: Tue, 29 Sep 2020 15:59:22 +0300 Subject: [PATCH] In attach_story_data_to_stories(), don't overwrite existing fields --- .../common/src/perl/MediaWords/DBI/Stories.pm | 28 ++- .../tests/perl/MediaWords/DBI/Stories.t | 177 ++++++++++++++++++ 2 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 apps/common/tests/perl/MediaWords/DBI/Stories.t diff --git a/apps/common/src/perl/MediaWords/DBI/Stories.pm b/apps/common/src/perl/MediaWords/DBI/Stories.pm index 3b574c7b03..7b1712c679 100644 --- a/apps/common/src/perl/MediaWords/DBI/Stories.pm +++ b/apps/common/src/perl/MediaWords/DBI/Stories.pm @@ -35,7 +35,21 @@ sub attach_story_data_to_stories { my ( $stories, $story_data, $list_field ) = @_; - map { $_->{ $list_field } = [] } @{ $stories } if ( $list_field ); + if ( $list_field ) { + for my $story ( @{ $stories } ) { + # This subroutine could be called for multiple "story_data" chunks + # with the same "stories" list to attach the chunks to, so the list + # with "list_field" key might already exist + if ( defined $story->{ $list_field } ) { + # Validate that it's actually an arrayref + unless ( ref( $story->{ $list_field } ) eq ref( [] ) ) { + die "One or more stories already have '$list_field' set which is not an arrayref"; + } + } else { + $story->{ $list_field } = []; + } + } + } unless ( scalar @{ $story_data } ) { @@ -65,7 +79,17 @@ sub attach_story_data_to_stories my $sid = $story->{ stories_id }; if ( my $sd = $story_data_lookup->{ $sid } ) { - map { $story->{ $_ } = $sd->{ $_ } } keys( %{ $sd } ); + foreach my $story_key ( keys( %{ $sd } ) ) { + if ( defined $list_field and $story_key eq $list_field ) { + $story->{ $story_key } //= []; + foreach my $story_field_value ( @{ $sd->{ $story_key } } ) { + push( @{ $story->{ $story_key } }, $story_field_value ); + } + } else { + $story->{ $story_key } = $sd->{ $story_key }; + } + } + TRACE "story matched: " . Dumper( $story ); } } diff --git a/apps/common/tests/perl/MediaWords/DBI/Stories.t b/apps/common/tests/perl/MediaWords/DBI/Stories.t new file mode 100644 index 0000000000..e4a43a220c --- /dev/null +++ b/apps/common/tests/perl/MediaWords/DBI/Stories.t @@ -0,0 +1,177 @@ +use strict; +use warnings; + +use Test::More tests => 2; +use Test::Deep; + +use Data::Dumper; + +use MediaWords::CommonLibs; +use MediaWords::DBI::Stories; + + +sub test_attach_story_data_to_stories() +{ + my $stories = [ + { + 'stories_id' => 1, + 'title' => 'Foo', + }, + { + 'stories_id' => 2, + 'title' => 'Bar', + }, + { + 'stories_id' => 3, + 'title' => 'Baz', + }, + ]; + + my $story_data = [ + { + 'stories_id' => 1, + 'description' => 'foo foo foo', + }, + { + 'stories_id' => 2, + 'description' => 'bar bar bar', + }, + { + 'stories_id' => 3, + 'description' => 'baz baz baz', + }, + ]; + + my $got_stories = MediaWords::DBI::Stories::attach_story_data_to_stories( $stories, $story_data ); + + my $expected_stories = [ + { + 'stories_id' => 1, + 'title' => 'Foo', + 'description' => 'foo foo foo', + }, + { + 'stories_id' => 2, + 'title' => 'Bar', + 'description' => 'bar bar bar', + }, + { + 'stories_id' => 3, + 'title' => 'Baz', + 'description' => 'baz baz baz', + } + ]; + + is_deeply( $got_stories, $expected_stories, "attach_story_data_to_stories()" ); +} + +sub test_attach_story_data_to_stories_list_field() +{ + my $stories = [ + { + 'stories_id' => 1, + 'title' => 'Foo', + }, + { + 'stories_id' => 2, + 'title' => 'Bar', + }, + { + 'stories_id' => 3, + 'title' => 'Baz', + }, + ]; + + # Run function with multiple inputs to confirm that existing "attached" data + # doesn't get overwritten + + my $story_data_1 = [ + { + 'stories_id' => 1, + 'description' => 'foo 1', + }, + { + 'stories_id' => 1, + 'description' => 'foo 2', + }, + { + 'stories_id' => 2, + 'description' => 'bar 1', + }, + ]; + my $story_data_2 = [ + { + 'stories_id' => 2, + 'description' => 'bar 2', + }, + { + 'stories_id' => 3, + 'description' => 'baz 1', + }, + { + 'stories_id' => 3, + 'description' => 'baz 2', + }, + ]; + + my $got_stories; + $got_stories = MediaWords::DBI::Stories::attach_story_data_to_stories( $stories, $story_data_1, 'attached' ); + $got_stories = MediaWords::DBI::Stories::attach_story_data_to_stories( $stories, $story_data_2, 'attached' ); + + my $expected_stories = [ + { + 'stories_id' => 1, + 'title' => 'Foo', + 'attached' => [ + { + 'description' => 'foo 1', + 'stories_id' => 1, + }, + { + 'description' => 'foo 2', + 'stories_id' => 1, + } + ], + }, + { + 'stories_id' => 2, + 'title' => 'Bar', + 'attached' => [ + { + 'stories_id' => 2, + 'description' => 'bar 1', + }, + { + 'stories_id' => 2, + 'description' => 'bar 2', + } + ], + }, + { + 'stories_id' => 3, + 'title' => 'Baz', + 'attached' => [ + { + 'stories_id' => 3, + 'description' => 'baz 1', + }, + { + 'stories_id' => 3, + 'description' => 'baz 2', + } + ], + }, + ]; + + is_deeply( $got_stories, $expected_stories, "attach_story_data_to_stories() with list_field" ); +} + +sub main +{ + test_attach_story_data_to_stories(); + test_attach_story_data_to_stories_list_field(); + + done_testing(); +} + +main();