Skip to content

Commit

Permalink
Cache redirected gids
Browse files Browse the repository at this point in the history
If you call `get_by_gid` with a redirected gid, we'll cache the resolved gid of
the entity, but not the redirected one that was passed in. This fixes that.
  • Loading branch information
mwiencek committed Jan 14, 2024
1 parent 3c35766 commit b2d4b82
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 6 deletions.
3 changes: 2 additions & 1 deletion lib/MusicBrainz/Server/Data/Role/EntityCache.pm
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use Moose::Role;
use namespace::autoclean;
use List::AllUtils qw( any uniq );
use MusicBrainz::Server::Replication qw( :replication_type );
use MusicBrainz::Server::Validation qw( is_database_row_id );
use Readonly;

requires '_type';
Expand Down Expand Up @@ -159,7 +160,7 @@ sub _add_to_cache_impl {
my %possible_recently_invalidated_id_map = map {
my $key = $invalidated_prefix . $_;
($key => $_)
} @$ids;
} grep { is_database_row_id($_) } @$ids;
# Check which of these keys actually exist in our Redis store,
# indicating which entity IDs were recently-invalidated in the cache.
my @recently_invalidated_ids = map {
Expand Down
18 changes: 14 additions & 4 deletions lib/MusicBrainz/Server/Data/Role/GIDEntityCache.pm
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package MusicBrainz::Server::Data::Role::GIDEntityCache;

use Moose::Role;
use namespace::autoclean;
use Scalar::Util qw( blessed );

with 'MusicBrainz::Server::Data::Role::EntityCache';

Expand All @@ -20,7 +21,12 @@ around get_by_gid => sub {
} else {
$obj = $self->$orig($gid);
if (defined($obj)) {
$self->_add_to_cache($cache, { $obj->id => $obj }, [$obj->id]);
$id = $obj->id;
$self->_add_to_cache(
$cache,
{ $id => $obj, $gid => $id, $obj->gid => $id },
[$id, $gid, $obj->gid],
);
}
}
return $obj;
Expand All @@ -30,9 +36,13 @@ around _create_cache_entries => sub {
my ($orig, $self, $data, $ids) = @_;

my $prefix = $self->_cache_prefix;
my @orig_entries = $self->$orig($data, $ids);
my @entries = @orig_entries;
for my $entry (@orig_entries) {
my @entries = $self->$orig($data, $ids);
my @entity_entries = grep {
my $value = $_->[1];
blessed $value &&
$value->does('MusicBrainz::Server::Entity::Role::GID')
} @entries;
for my $entry (@entity_entries) {
my $entity = $entry->[1];
push @entries, [$prefix . $entity->gid, $entity->id];
}
Expand Down
34 changes: 33 additions & 1 deletion t/lib/t/MusicBrainz/Server/Data/Role/GIDEntityCache.pm
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ is ( $entity->id, 1 );
is ( $entity_data->get_by_gid_called, 1 );
is ( $entity_data->get_by_id_called, 0 );
is ( $test->c->cache->_orig->get_called, 1 );
is ( $test->c->cache->_orig->set_called, 2 );
is ( $test->c->cache->_orig->set_called, 4 );
ok ( $test->c->cache->_orig->data->{'my_cached_entity_data:1'} =~ '1' );
ok ( $test->c->cache->_orig->data->{"my_cached_entity_data:$gid1"} =~ '1' );

Expand Down Expand Up @@ -290,4 +290,36 @@ test 'Cache is transactional (MBS-7241)' => sub {
die $error if $error;
};

test 'Redirected gids are cached' => sub {
my $test = shift;
my $c = $test->cache_aware_c;

$c->sql->begin;
$c->sql->do(<<~'SQL');
INSERT INTO artist (id, gid, name, sort_name)
VALUES (3, '5809b63b-73d9-406c-b67d-f145a5a9b696', 'A', 'A');
INSERT INTO artist_gid_redirect
VALUES ('6322dacd-64c9-4ae8-a7ca-eada3f8abf2e', 3);
SQL
$c->model('Artist')->get_by_gid('6322dacd-64c9-4ae8-a7ca-eada3f8abf2e');
$c->sql->commit;

ok($c->cache->get('artist:3')->isa('MusicBrainz::Server::Entity::Artist'),
'id is cached');

is($c->cache->get('artist:5809b63b-73d9-406c-b67d-f145a5a9b696'),
3,
'gid is cached');

is($c->cache->get('artist:6322dacd-64c9-4ae8-a7ca-eada3f8abf2e'),
3,
'redirected gid is cached');

$c->sql->auto_commit(1);
$c->sql->do(<<~'SQL');
DELETE FROM artist_gid_redirect WHERE new_id = 3;
DELETE FROM artist WHERE id = 3;
SQL
};

1;

0 comments on commit b2d4b82

Please sign in to comment.