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

Album splitting logic doesn't seem to be correct #1000

Open
seehamrun opened this issue Aug 17, 2021 · 0 comments
Open

Album splitting logic doesn't seem to be correct #1000

seehamrun opened this issue Aug 17, 2021 · 0 comments

Comments

@seehamrun
Copy link
Collaborator

The ensureAlbumSize function in the PhotosContainerResource doesn't look entirely correct

https://github.com/google/data-transfer-project/blob/master/portability-types-common/src/main/java/org/datatransferproject/types/common/models/photos/PhotosContainerResource.java#L102-L139

A couple of notable things:

  • The split assumes that all the images corresponding to the albums are present in the same resource
  • Also assumes that the albums that images belong in are also in the same resource (which they may not be if the album was created in the prior resource).

I found this while trying to port this code to the media container resource and incorporate videos, in the meantime im going to not port this over since we have our own handling of this on the Google end and i don't think anyone else relies on this.

jzacsh added a commit that referenced this issue Apr 18, 2022
backfills missing unit tests of MedaiContainerResourceTest and fixes a bug in the process

This change forks test code (`{Photos,Media}ContainerResourceTest`) in line with MediaContainerResource's existing creation, then deletes some transmogrification stuff that we're not doing (per issue #1000) and adds a TODO(#1060) atop the new test to finish the tests more fully for video logic.
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

No branches or pull requests

1 participant