From d0a0bef6006ade3db0bf33c853e21256a1df01e6 Mon Sep 17 00:00:00 2001 From: Jonathan Zacsh Date: Wed, 22 Jun 2022 15:40:57 -0500 Subject: [PATCH] start automating Media integration; stand as demo for future model refactorings changes (#1062) * fixes #1058: add MediaContainerResourceTest & more adds MediaContainerResourceTest and fixes bug said test newly reveals in automatic root album creation logic during transmogrification. nit: in nit-fixing doc bug (documentation was wrong on `MediaContainerResource#ensureRootAlbum`) also just lifts conditional logic being pushed down into helper function; taking liberty here with my opinion - that Single Responsibility (SOLID) would remove this complexity to begin with (`ensureRootAlbum` should not care about what a transmogrification is or what its config is). * noop(nit:docs): point TODO at a trackable issue number * noop(readability) run Goog style via clang-format * noop(new api) Media* APIs: generate lowlevel types adds MediaContainerResource (and accompanying `MediaAlbum` under the hood) static APIs that admit these classes just wrap or maintain 1:1 correspondence to Photo* types. Unfortunately only adds test coverage via the top-level usage (`MediaContainerResourceTest`) because there's no unit test coverage in place for MediaAlbum and I'm already spinning my wheels a bit adding missing test infra. But splitting some of the MediaAlbum==PhotoAlbum comparison unit test sanity-checking logic (into non-extant MediaAlbumTest.java) would be the correct thing to do here. motivation: this unblocks continued `MediaVertical` work without the need to have (for example) a `AcmecoPhotoExporter` manually forked copy-pasta style into `AcmecoMediaExporter` and then cause immediate techdebt for us (through code drift). longer-term: This change _does_ bring into question some of the intermediate type planning we're doing here, but we've determined this change is not going to be throw away work (eg: to unblock continued Media vertical expansion) while we revisit and discuss intermediate types a bit more. Those discussions will change _how_ this code is called but are very unlikely to _remove_ callers to this code. * WIP: experimental (not even compiling) draft of converter logic with some TODOs * WIP: nitfixes; cleanups & renames renames variables taht were backwards cleans up code since MediaContainer APIs[^1] were pulled out of this work stream to get committed in parallel. [^1]: commit 896342610b9c (of branch `kate-jon-pair-utils`) * WIP: fixes compilation errors with my generics setup * noop: revert accidental formatting during merge * noop(refactor) moves container copy logic into ExportResult; unfortunately there are no unit tests of this class * noop(refactor) clang-format & add javadoc * noop(refactor) pull ExportResult<...> construction out * noop(docs) point to a tracked TODO: #1065 * WIP: merge fix: bad merge kept both versions of the same code * adds Media* importer by conversion of Photo* one also some nitpicks on some javadoc. * fix: 2c2c4a34 = uncompiling copy-pasta * fix incorrect marking of Import auto-support for Media Co-authored-by: William Morland --- .../microsoft/MicrosoftTransferExtension.java | 14 ++++-- .../spi/transfer/provider/ExportResult.java | 9 +++- .../MediaToPhotoConversionImporter.java | 44 +++++++++++++++++ .../PhotoToMediaConversionExporter.java | 47 +++++++++++++++++++ 4 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/converter/MediaToPhotoConversionImporter.java create mode 100644 portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/converter/PhotoToMediaConversionExporter.java diff --git a/extensions/data-transfer/portability-data-transfer-microsoft/src/main/java/org/datatransferproject/transfer/microsoft/MicrosoftTransferExtension.java b/extensions/data-transfer/portability-data-transfer-microsoft/src/main/java/org/datatransferproject/transfer/microsoft/MicrosoftTransferExtension.java index a615c4a9a..1dfda53d7 100644 --- a/extensions/data-transfer/portability-data-transfer-microsoft/src/main/java/org/datatransferproject/transfer/microsoft/MicrosoftTransferExtension.java +++ b/extensions/data-transfer/portability-data-transfer-microsoft/src/main/java/org/datatransferproject/transfer/microsoft/MicrosoftTransferExtension.java @@ -15,6 +15,7 @@ import org.datatransferproject.spi.transfer.extension.TransferExtension; import org.datatransferproject.spi.transfer.provider.Exporter; import org.datatransferproject.spi.transfer.provider.Importer; +import org.datatransferproject.spi.transfer.provider.converter.PhotoToMediaConversionExporter; import org.datatransferproject.transfer.microsoft.calendar.MicrosoftCalendarExporter; import org.datatransferproject.transfer.microsoft.calendar.MicrosoftCalendarImporter; import org.datatransferproject.transfer.microsoft.common.MicrosoftCredentialFactory; @@ -36,11 +37,16 @@ public class MicrosoftTransferExtension implements TransferExtension { private static final String CONTACTS = "CONTACTS"; private static final String CALENDAR = "CALENDAR"; private static final String PHOTOS = "PHOTOS"; + private static final String MEDIA = "MEDIA"; private static final String OFFLINE_DATA = "OFFLINE-DATA"; + + // TODO(#1065) don't keep adding here - just have the converters invoked automatically when Media + // isn't supported on one or the other side of this equation; this is just a WIP prototype to show + // the concept of converters at play. private static final ImmutableList SUPPORTED_IMPORT_SERVICES = ImmutableList.of(CALENDAR, CONTACTS, PHOTOS); private static final ImmutableList SUPPORTED_EXPORT_SERVICES = - ImmutableList.of(CALENDAR, CONTACTS, PHOTOS, OFFLINE_DATA); + ImmutableList.of(CALENDAR, CONTACTS, PHOTOS, MEDIA, OFFLINE_DATA); private ImmutableMap importerMap; private ImmutableMap exporterMap; @@ -135,8 +141,10 @@ PHOTOS, new MicrosoftPhotosImporter(BASE_GRAPH_URL, client, mapper, jobStore, mo exporterBuilder.put( CALENDAR, new MicrosoftCalendarExporter(BASE_GRAPH_URL, client, mapper, transformerService)); - exporterBuilder.put( - PHOTOS, new MicrosoftPhotosExporter(credentialFactory, jsonFactory, monitor)); + // TODO(#1065) don't require this manual mapping; just do this automatically inside DTP + MicrosoftPhotosExporter photosExporter = new MicrosoftPhotosExporter(credentialFactory, jsonFactory, monitor); + exporterBuilder.put(PHOTOS, photosExporter); + exporterBuilder.put(MEDIA, new PhotoToMediaConversionExporter(photosExporter)); exporterBuilder.put( OFFLINE_DATA, new MicrosoftOfflineDataExporter(BASE_GRAPH_URL, client, mapper)); diff --git a/portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/ExportResult.java b/portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/ExportResult.java index 22a868f60..5d0e561c8 100644 --- a/portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/ExportResult.java +++ b/portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/ExportResult.java @@ -9,7 +9,6 @@ * The result of an item export operation, after retries. */ public class ExportResult { - public static final ExportResult CONTINUE = new ExportResult(ResultType.CONTINUE); public static final ExportResult END = new ExportResult(ResultType.CONTINUE); @@ -41,6 +40,14 @@ public ExportResult(ResultType type, T exportedData) { this.exportedData = exportedData; } + /** + * Builds a new ExportResult. copying the current one but replacing only the data with + * `replacementData`. + */ + public ExportResult copyWithExportedData(R replacementData) { + return new ExportResult<>(this.getType(), replacementData, this.getContinuationData()); + } + /** * Ctor. * diff --git a/portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/converter/MediaToPhotoConversionImporter.java b/portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/converter/MediaToPhotoConversionImporter.java new file mode 100644 index 000000000..84f3e12bf --- /dev/null +++ b/portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/converter/MediaToPhotoConversionImporter.java @@ -0,0 +1,44 @@ +package org.datatransferproject.spi.transfer.provider.converter; + +import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutor; +import org.datatransferproject.spi.transfer.provider.ImportResult; +import org.datatransferproject.spi.transfer.provider.Importer; +import org.datatransferproject.types.common.ExportInformation; +import org.datatransferproject.types.common.models.media.MediaContainerResource; +import org.datatransferproject.types.common.models.photos.PhotosContainerResource; +import org.datatransferproject.types.transfer.auth.AuthData; +import org.datatransferproject.types.transfer.auth.TokensAndUrlAuthData; + +import java.util.Optional; +import java.util.UUID; + +/** + * Automatically produces a "PHOTO" importer from a more sophisticated "MEDIA" importer. + * + * Produces an `Importer` that can handle import jobs for `PhotosContainerResource` (aka "PHOTOS" + * jobs) based on an existing importer that can handle the broader jobs for `MediaContainerResource` + * (aka "MEDIA" jobs). + * + * This is intended for providers who do not support "MEDIA" as a special case. + */ +// TODO(#1065) fix primitives-obession causing us to key Providers on "PHOTOS" string rather +// than underlying file types. +public class MediaToPhotoConversionImporter< + A extends AuthData, + PCR extends PhotosContainerResource, + WrappedImporter extends Importer> implements Importer { + private final WrappedImporter wrappedMediaImporter; + + public MediaToPhotoConversionImporter(WrappedImporter wrappedMediaImporter) { + this.wrappedMediaImporter = wrappedMediaImporter; + } + + public ImportResult importItem( + UUID jobId, + IdempotentImportExecutor idempotentExecutor, + A authData, + PCR photosContainerResource) throws Exception { + MediaContainerResource mediaContainerResource = MediaContainerResource.photoToMedia(photosContainerResource); + return wrappedMediaImporter.importItem(jobId, idempotentExecutor, authData, mediaContainerResource); + } +} diff --git a/portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/converter/PhotoToMediaConversionExporter.java b/portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/converter/PhotoToMediaConversionExporter.java new file mode 100644 index 000000000..8d670a2fc --- /dev/null +++ b/portability-spi-transfer/src/main/java/org/datatransferproject/spi/transfer/provider/converter/PhotoToMediaConversionExporter.java @@ -0,0 +1,47 @@ +package org.datatransferproject.spi.transfer.provider.converter; + +import java.util.Optional; +import java.util.UUID; +import org.datatransferproject.spi.transfer.provider.ExportResult; +import org.datatransferproject.spi.transfer.provider.Exporter; +import org.datatransferproject.types.common.ExportInformation; +import org.datatransferproject.types.common.models.media.MediaContainerResource; +import org.datatransferproject.types.common.models.photos.PhotosContainerResource; +import org.datatransferproject.types.transfer.auth.AuthData; +import org.datatransferproject.types.transfer.auth.TokensAndUrlAuthData; + +/** + * Automatically produces a "MEDIA" importer from a "PHOTO" importer. + * + * This is intended for providers who do not support video, but should be allowed to continue using + * the common model of "Photo" without rewriting their DTP integration. In such cases this class + * allows the provider to export Media objects with only the relevant photo fields populated and + * anything else (like the video fields) ignored. + * + * This is intended for providers who do not support "PHOTOS" as a special case. + */ +// TODO(#1065) fix primitives-obession causing us to key Providers on "PHOTOS" string rather +// than underlying file types. +public class PhotoToMediaConversionExporter< + A extends AuthData, + PCR extends PhotosContainerResource, + WrappedExporter extends Exporter + > implements Exporter { + private final WrappedExporter wrappedPhotoExporter; + + public PhotoToMediaConversionExporter(WrappedExporter wrappedPhotoExporter) { + this.wrappedPhotoExporter = wrappedPhotoExporter; + } + + public ExportResult export( + UUID jobId, + A authData, + Optional exportInformation) throws Exception { + ExportResult originalExportResult = + wrappedPhotoExporter.export(jobId, authData, exportInformation); + PCR photosContainerResource = originalExportResult.getExportedData(); + MediaContainerResource mediaContainerResource = + MediaContainerResource.photoToMedia(photosContainerResource); + return originalExportResult.copyWithExportedData(mediaContainerResource); + } +}