Skip to content

Commit

Permalink
Merge pull request #19762 from wordpress-mobile/feature/upload-servic…
Browse files Browse the repository at this point in the history
…e-logs

[Upload] Add UploadService and UploadWorker logs
  • Loading branch information
Thomas Horta authored Dec 11, 2023
2 parents 3a1777c + de66db5 commit b4bc8d0
Show file tree
Hide file tree
Showing 18 changed files with 117 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1094,7 +1094,7 @@ private void addMediaToUploadService(@NonNull ArrayList<MediaModel> mediaModels)
return;
}

UploadService.uploadMedia(this, mediaModels);
UploadService.uploadMedia(this, mediaModels, "MediaBrowserActivity#addMediaToUploadService");
AppRatingDialog.INSTANCE.incrementInteractions(APP_REVIEWS_EVENT_INCREMENTED_BY_UPLOADING_MEDIA);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,9 @@ class MySiteFragment : Fragment(R.layout.my_site_fragment),
)
showSnackbar(SnackbarMessageHolder(UiString.UiStringText(message)))
}
viewModel.onMediaUpload.observeEvent(viewLifecycleOwner) { UploadService.uploadMedia(requireActivity(), it) }
viewModel.onMediaUpload.observeEvent(viewLifecycleOwner) {
UploadService.uploadMedia(requireActivity(), it, "MySiteFragment onMediaUpload")
}
dialogViewModel.onInteraction.observeEvent(viewLifecycleOwner) { viewModel.onDialogInteraction(it) }
viewModel.onUploadedItem.observeEvent(viewLifecycleOwner) { handleUploadedItem(it) }
viewModel.onOpenJetpackInstallFullPluginOnboarding.observeEvent(viewLifecycleOwner) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class FeaturedImageHelper @Inject constructor(
private fun startUploadService(media: MediaModel) {
val mediaList = ArrayList<MediaModel>()
mediaList.add(media)
uploadServiceFacade.uploadMedia(mediaList)
uploadServiceFacade.uploadMedia(mediaList, "FeaturedImageHelper#startUploadService")
}

fun queueFeaturedImageForUpload(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ fun handlePostListAction(
val intent = UploadService.getRetryUploadServiceIntent(
activity,
action.post,
action.trackAnalytics
action.trackAnalytics,
"PostListAction.RetryUpload"
)
activity.startService(intent)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ class PreviewStateHelper @Inject constructor() {
PostInfoType.PostNoInfo
)
if (!UploadService.isPostUploadingOrQueued(post)) {
UploadService.uploadPost(activity, post.id, false)
UploadService.uploadPost(
activity, post.id, false,
"PreviewStateHelper#startUploading, isRemoteAutoSave: true"
)
} else {
AppLog.d(
AppLog.T.POSTS,
Expand All @@ -69,7 +72,10 @@ class PreviewStateHelper @Inject constructor() {
PostInfoType.PostNoInfo
)
if (!UploadService.isPostUploadingOrQueued(post)) {
UploadService.uploadPost(activity, post.id, false)
UploadService.uploadPost(
activity, post.id, false,
"PreviewStateHelper#startUploading, isRemoteAutoSave: false"
)
} else {
AppLog.d(
AppLog.T.POSTS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,10 @@ class StorePostViewModel
editPostRepository.getPost(),
requireNotNull(siteStore.getSiteByLocalId(editPostRepository.localSiteId))
)
uploadService.uploadPost(context, editPostRepository.id, isFirstTimePublish)
uploadService.uploadPost(
context, editPostRepository.id, isFirstTimePublish,
"StorePostViewModel#savePostOnline"
)
SAVED_ONLINE
} else {
SAVED_LOCALLY
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,10 @@ class StoryMediaSaveUploadBridge @Inject constructor(
editPostRepository.getPost(),
site
)
uploadService.uploadPost(appContext, editPostRepository.id, true)
uploadService.uploadPost(
appContext, editPostRepository.id, true,
"StoryMediaSaveUploadBridge#addNewMediaItemsInStoryFramesToPostAsync"
)
// SAVED_ONLINE
storiesTrackerHelper.trackStoryPostSavedEvent(uriList.size, site, false)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,8 @@ void updateNotificationSuccessForPost(@NonNull PostImmutableModel post, @NonNull

// add draft Publish action for drafts
if (PostStatus.fromPost(post) == PostStatus.DRAFT || PostStatus.fromPost(post) == PostStatus.PENDING) {
Intent publishIntent = UploadService.getPublishPostServiceIntent(mContext, post, isFirstTimePublish);
Intent publishIntent = UploadService.getPublishPostServiceIntent(mContext, post,
isFirstTimePublish, "draft success notification publish action");
PendingIntent pendingIntent = PendingIntent.getService(mContext, 0, publishIntent,
PendingIntent.FLAG_CANCEL_CURRENT | PendingIntent.FLAG_IMMUTABLE);
notificationBuilder.addAction(R.drawable.ic_posts_white_24dp, mContext.getString(R.string.button_publish),
Expand Down Expand Up @@ -531,7 +532,7 @@ void updateNotificationErrorForPost(@NonNull PostModel post, @NonNull SiteModel
// Add RETRY action - only available on Aztec
if (AppPrefs.isAztecEditorEnabled()) {
Intent publishIntent = UploadService.getRetryUploadServiceIntent(mContext, post,
PostUtils.isFirstTimePublish(post));
PostUtils.isFirstTimePublish(post), "post error notification retry action");
PendingIntent actionPendingIntent = PendingIntent.getService(mContext, 0, publishIntent,
PendingIntent.FLAG_CANCEL_CURRENT | PendingIntent.FLAG_IMMUTABLE);
notificationBuilder.addAction(0, mContext.getString(R.string.retry),
Expand Down Expand Up @@ -624,7 +625,8 @@ void updateNotificationErrorForMedia(@NonNull List<MediaModel> mediaList, @NonNu
if (mediaList != null && !mediaList.isEmpty()) {
ArrayList<MediaModel> mediaListToRetry = new ArrayList<>();
mediaListToRetry.addAll(mediaList);
Intent publishIntent = UploadService.getUploadMediaServiceIntent(mContext, mediaListToRetry, true);
Intent publishIntent = UploadService.getUploadMediaServiceIntent(mContext, mediaListToRetry,
true, "media error notification retry action");
PendingIntent actionPendingIntent = PendingIntent.getService(mContext, 1, publishIntent,
PendingIntent.FLAG_CANCEL_CURRENT | PendingIntent.FLAG_IMMUTABLE);
notificationBuilder.addAction(0, mContext.getString(R.string.retry),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public class UploadService extends Service {
private static final String KEY_UPLOAD_MEDIA_FROM_EDITOR = "mediaFromEditor";
private static final String KEY_LOCAL_POST_ID = "localPostId";
private static final String KEY_SHOULD_TRACK_ANALYTICS = "shouldTrackPostAnalytics";
private static final String KEY_SOURCE_FOR_LOGGING = "sourceForLogging";

private static @Nullable UploadService sInstance;

Expand Down Expand Up @@ -150,6 +151,13 @@ public int onStartCommand(Intent intent, int flags, int startId) {
return START_NOT_STICKY;
}

if (intent.hasExtra(KEY_SOURCE_FOR_LOGGING)) {
String sourceForLogging = intent.getStringExtra(KEY_SOURCE_FOR_LOGGING);
if (sourceForLogging != null && !sourceForLogging.isEmpty()) {
AppLog.i(T.MAIN, "UploadService > Started from " + sourceForLogging);
}
}

if (intent.hasExtra(KEY_MEDIA_LIST)) {
unpackMediaIntent(intent);
}
Expand Down Expand Up @@ -336,11 +344,12 @@ private boolean isThisPostTotallyNewOrFailed(PostImmutableModel post) {
}

public static Intent getRetryUploadServiceIntent(Context context, @NonNull PostImmutableModel post,
boolean trackAnalytics) {
boolean trackAnalytics, String sourceForLogging) {
Intent intent = new Intent(context, UploadService.class);
intent.putExtra(KEY_LOCAL_POST_ID, post.getId());
intent.putExtra(KEY_SHOULD_TRACK_ANALYTICS, trackAnalytics);
intent.putExtra(KEY_SHOULD_RETRY, true);
intent.putExtra(KEY_SOURCE_FOR_LOGGING, sourceForLogging);
return intent;
}

Expand All @@ -353,48 +362,53 @@ public static Intent getRetryUploadServiceIntent(Context context, @NonNull PostI
* UploadUtils.publishPost(..) instead.
*/
public static Intent getPublishPostServiceIntent(Context context, @NonNull PostImmutableModel post,
boolean trackAnalytics) {
boolean trackAnalytics, String sourceForLogging) {
Intent intent = new Intent(context, UploadService.class);
intent.putExtra(KEY_LOCAL_POST_ID, post.getId());
intent.putExtra(KEY_SHOULD_TRACK_ANALYTICS, trackAnalytics);
intent.putExtra(KEY_CHANGE_STATUS_TO_PUBLISH, true);
intent.putExtra(KEY_SOURCE_FOR_LOGGING, sourceForLogging);
return intent;
}

public static Intent getUploadMediaServiceIntent(Context context, @NonNull ArrayList<MediaModel> mediaList,
boolean isRetry) {
boolean isRetry, String sourceForLogging) {
Intent intent = new Intent(context, UploadService.class);
intent.putExtra(UploadService.KEY_MEDIA_LIST, mediaList);
intent.putExtra(KEY_SHOULD_RETRY, isRetry);
intent.putExtra(KEY_SOURCE_FOR_LOGGING, sourceForLogging);
return intent;
}

/**
* Adds a post to the queue.
* @param postId
* @param isFirstTimePublish true when its status changes from local draft or remote draft to published.
* @param sourceForLogging the source of the request for logging purposes.
*/
public static void uploadPost(Context context, int postId, boolean isFirstTimePublish) {
public static void uploadPost(Context context, int postId, boolean isFirstTimePublish, String sourceForLogging) {
Intent intent = new Intent(context, UploadService.class);
intent.putExtra(KEY_LOCAL_POST_ID, postId);
intent.putExtra(KEY_SHOULD_TRACK_ANALYTICS, isFirstTimePublish);
intent.putExtra(KEY_SOURCE_FOR_LOGGING, sourceForLogging);
context.startService(intent);
}

public static void uploadMedia(Context context, @NonNull MediaModel media) {
public static void uploadMedia(Context context, @NonNull MediaModel media, String sourceForLogging) {
ArrayList<MediaModel> list = new ArrayList<>();
list.add(media);

uploadMedia(context, list);
uploadMedia(context, list, sourceForLogging);
}

public static void uploadMedia(Context context, @NonNull ArrayList<MediaModel> mediaList) {
public static void uploadMedia(Context context, @NonNull ArrayList<MediaModel> mediaList, String sourceForLogging) {
if (context == null) {
return;
}

Intent intent = new Intent(context, UploadService.class);
intent.putExtra(UploadService.KEY_MEDIA_LIST, mediaList);
intent.putExtra(KEY_SOURCE_FOR_LOGGING, sourceForLogging);
context.startService(intent);
}

Expand All @@ -406,6 +420,7 @@ public static void uploadMediaFromEditor(Context context, @NonNull ArrayList<Med
Intent intent = new Intent(context, UploadService.class);
intent.putExtra(UploadService.KEY_MEDIA_LIST, mediaList);
intent.putExtra(UploadService.KEY_UPLOAD_MEDIA_FROM_EDITOR, true);
intent.putExtra(KEY_SOURCE_FOR_LOGGING, "UploadService#uploadMediaFromEditor");
context.startService(intent);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ import javax.inject.Inject
* contain any static methods.
*/
class UploadServiceFacade @Inject constructor(private val appContext: Context) {
fun uploadPost(context: Context, post: PostModel, trackAnalytics: Boolean) {
val intent = UploadService.getRetryUploadServiceIntent(context, post, trackAnalytics)
fun uploadPost(context: Context, post: PostModel, trackAnalytics: Boolean, sourceForLogging: String = "") {
val intent = UploadService.getRetryUploadServiceIntent(context, post, trackAnalytics, sourceForLogging)
context.startService(intent)
}

fun uploadPost(context: Context, postId: Int, isFirstTimePublish: Boolean) =
UploadService.uploadPost(context, postId, isFirstTimePublish)
fun uploadPost(context: Context, postId: Int, isFirstTimePublish: Boolean, sourceForLogging: String = "") =
UploadService.uploadPost(context, postId, isFirstTimePublish, sourceForLogging)

fun isPostUploadingOrQueued(post: PostImmutableModel) =
UploadService.isPostUploadingOrQueued(post)
Expand All @@ -31,8 +31,8 @@ class UploadServiceFacade @Inject constructor(private val appContext: Context) {
fun cancelFinalNotificationForMedia(site: SiteModel) =
UploadService.cancelFinalNotificationForMedia(appContext, site)

fun uploadMedia(mediaList: ArrayList<MediaModel>) =
UploadService.uploadMedia(appContext, mediaList)
fun uploadMedia(mediaList: ArrayList<MediaModel>, sourceForLogging: String = "") =
UploadService.uploadMedia(appContext, mediaList, sourceForLogging)

fun getPendingOrInProgressFeaturedImageUploadForPost(post: PostImmutableModel): MediaModel? =
UploadService.getPendingOrInProgressFeaturedImageUploadForPost(post)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ class UploadStarter @Inject constructor(
uploadServiceFacade.uploadPost(
context = context,
post = post,
trackAnalytics = false
trackAnalytics = false,
sourceForLogging = "UploadStarter#upload"
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ public static void publishPost(Activity activity, final PostModel post, SiteMode
dispatcher.dispatch(PostActionBuilder.newUpdatePostAction(post));

if (NetworkUtils.isNetworkAvailable(activity)) {
UploadService.uploadPost(activity, post.getId(), isFirstTimePublish);
UploadService.uploadPost(activity, post.getId(), isFirstTimePublish, "UploadUtils#publishPost");
if (onPublishingCallback != null) {
onPublishingCallback.onPublishing(isFirstTimePublish);
}
Expand Down Expand Up @@ -486,14 +486,15 @@ public static void onPostUploadedSnackbarHandler(final Activity activity, View s
// RETRY only available for Aztec
if (AppPrefs.isAztecEditorEnabled()) {
UploadUtils.showSnackbarError(snackbarAttachView, errorMessage, R.string.retry,
new View.OnClickListener() {
@Override
public void onClick(View view) {
Intent intent = UploadService.getRetryUploadServiceIntent(
activity, post, false);
activity.startService(intent);
}
}, sequencer);
new View.OnClickListener() {
@Override
public void onClick(View view) {
Intent intent = UploadService.getRetryUploadServiceIntent(
activity, post, false,
"post error snack bar retry action");
activity.startService(intent);
}
}, sequencer);
} else {
UploadUtils.showSnackbarError(snackbarAttachView, errorMessage, sequencer);
}
Expand Down Expand Up @@ -567,17 +568,18 @@ public static void onMediaUploadedSnackbarHandler(final Activity activity, View
// RETRY only available for Aztec
if (mediaList != null && !mediaList.isEmpty()) {
UploadUtils.showSnackbarError(snackbarAttachView, messageForUser, R.string.retry,
new View.OnClickListener() {
@Override
public void onClick(View view) {
ArrayList<MediaModel> mediaListToRetry = new ArrayList<>();
mediaListToRetry.addAll(mediaList);
Intent retryIntent = UploadService
.getUploadMediaServiceIntent(activity,
mediaListToRetry, true);
activity.startService(retryIntent);
}
}, sequencer);
new View.OnClickListener() {
@Override
public void onClick(View view) {
ArrayList<MediaModel> mediaListToRetry = new ArrayList<>();
mediaListToRetry.addAll(mediaList);
Intent retryIntent = UploadService
.getUploadMediaServiceIntent(activity,
mediaListToRetry, true,
"media error snack bar retry action");
activity.startService(retryIntent);
}
}, sequencer);
} else {
UploadUtils.showSnackbarError(snackbarAttachView, messageForUser, sequencer);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,15 @@ class UploadWorker(
}

override fun doWork(): Result {
AppLog.i(AppLog.T.MAIN, "UploadWorker started")
runBlocking {
val job = when (val localSiteId = inputData.getInt(WordPress.LOCAL_SITE_ID, UPLOAD_FROM_ALL_SITES)) {
UPLOAD_FROM_ALL_SITES -> uploadStarter.queueUploadFromAllSites()
else -> siteStore.getSiteByLocalId(localSiteId)?.let { uploadStarter.queueUploadFromSite(it) }
}
job?.join()
}
AppLog.i(AppLog.T.MAIN, "UploadWorker finished")
return Result.success()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class FeaturedImageHelperTest {
// Act
val mediaModel = featuredImageHelper.retryFeaturedImageUpload(mock(), mock())
// Assert
verify(uploadServiceFacade).uploadMedia(argThat { this[0] == mediaModel })
verify(uploadServiceFacade).uploadMedia(argThat { this[0] == mediaModel }, any())
}

@Test
Expand Down Expand Up @@ -189,7 +189,7 @@ class FeaturedImageHelperTest {
// Act
featuredImageHelper.queueFeaturedImageForUpload(0, createSiteModel(), mock(), "")
// Assert
verify(uploadServiceFacade).uploadMedia(argThat { this[0] == mediaModel })
verify(uploadServiceFacade).uploadMedia(argThat { this[0] == mediaModel }, any())
}

@Test
Expand Down
Loading

0 comments on commit b4bc8d0

Please sign in to comment.