Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add counts to MusicMediaContainer #1383

Merged
merged 2 commits into from
Aug 28, 2024
Merged

Add counts to MusicMediaContainer #1383

merged 2 commits into from
Aug 28, 2024

Conversation

seehamrun
Copy link
Collaborator

We were trying to figure out why no counts were written for MUSIC jobs even though the AppleMusicImporter sets the counts here:

final Map<String, Integer> counts =
new ImmutableMap.Builder<String, Integer>()
.put(AppleMusicConstants.PLAYLISTS_COUNT_DATA_NAME, playlistsCount)
.put(AppleMusicConstants.PLAYLIST_ITEMS_COUNT_DATA_NAME, playlistItemsCount)
.build();
return ImportResult.OK
.copyWithCounts(counts);

And this should be preserved the same way photos counts are preserved

Upon investigating it looks like we completely disregard the counts returned from the importer in favor of the counts that are with the original exported data here:

Which works for Media jobs because the MediaContainerResource overrides the getCounts method here:

public Map<String, Integer> getCounts() {
return new ImmutableMap.Builder<String, Integer>()
.put(ALBUMS_COUNT_DATA_NAME, albums.size())
.put(PHOTOS_COUNT_DATA_NAME, photos.size())
.put(VIDEOS_COUNT_DATA_NAME, videos.size())
.build();
}

This PR adds a getCounts method to the MusicMediaContainer and updates the strings that are used in the AppleMusicImporter to be the central strings instead (this prevents a mismatch in the strings that are being used)

This PR just makes things operate as they are operating for photos/media, however the larger question is in the callable importer which counts we want to use (the ones from the result of the import or the ones from the data) and that is likely a larger discussion to be had.

This also means that Apple->YTM transfers now have count information as well since the GoogleMusicImporter does not return counts

@seehamrun seehamrun requested a review from jzacsh August 27, 2024 22:09
@jzacsh jzacsh merged commit c92b344 into master Aug 28, 2024
5 checks passed
@jzacsh jzacsh deleted the counts branch August 28, 2024 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants