Updates to the operation

3 files changed, 106 insertions, 94 deletions

diff --git a/SmartDeviceLink/SDLTextAndGraphicManager.m b/SmartDeviceLink/SDLTextAndGraphicManager.m
index 624068602..285e43355 100644
--- a/SmartDeviceLink/SDLTextAndGraphicManager.m
+++ b/SmartDeviceLink/SDLTextAndGraphicManager.m
@@ -172,8 +172,18 @@ NS_ASSUME_NONNULL_BEGIN
     }
 
     SDLTextAndGraphicUpdateOperation *updateOperation = [[SDLTextAndGraphicUpdateOperation alloc] initWithConnectionManager:self.connectionManager fileManager:self.fileManager currentCapabilities:self.windowCapability currentScreenData:self.currentScreenData newState:[self currentState] updateCompletionHandler:handler];
+
+    __weak typeof(self) weakSelf = self;
+    __weak typeof(updateOperation) weakOp = updateOperation;
     updateOperation.completionBlock = ^{
-        // Check for error and `sentShow` and update our own current state, then update other pending transactions
+        // TODO: Update other pending transactions
+        if (weakOp.sentShow != nil) {
+            weakSelf.currentScreenData = weakOp.sentShow;
+        }
+
+        if (weakOp.error != nil) {
+            SDLLogE(@"Update operation failed with error: %@", weakOp.error);
+        }
     };
     [self.transactionQueue addOperation:updateOperation];
 }
@@ -184,45 +194,18 @@ NS_ASSUME_NONNULL_BEGIN
     return [[SDLTextAndGraphicState alloc] initWithTextField1:_textField1 textField2:_textField2 textField3:_textField3 textField4:_textField4 mediaText:_mediaTrackTextField title:_title primaryGraphic:_primaryGraphic secondaryGraphic:_secondaryGraphic alignment:_alignment textField1Type:_textField1Type textField2Type:_textField2Type textField3Type:_textField3Type textField4Type:_textField4Type];
 }
 
-#pragma mark - Extraction
-
-- (SDLShow *)sdl_extractImageFromShow:(SDLShow *)show {
-    SDLShow *newShow = [[SDLShow alloc] init];
-    newShow.graphic = show.graphic;
-    newShow.secondaryGraphic = show.secondaryGraphic;
-
-    return newShow;
-}
-
-- (nullable SDLShow *)sdl_createImageOnlyShowWithPrimaryArtwork:(nullable SDLArtwork *)primaryArtwork secondaryArtwork:(nullable SDLArtwork *)secondaryArtwork  {
-    SDLShow *newShow = [[SDLShow alloc] init];
-    newShow.graphic = ![self sdl_artworkNeedsUpload:primaryArtwork] ? primaryArtwork.imageRPC : nil;
-    newShow.secondaryGraphic = ![self sdl_artworkNeedsUpload:secondaryArtwork] ? secondaryArtwork.imageRPC : nil;
+#pragma mark - Helpers
 
-    if (newShow.graphic == nil && newShow.secondaryGraphic == nil) {
-        SDLLogV(@"No graphics to upload");
-        return nil;
-    }
+- (NSArray<NSString *> *)sdl_findNonNilTextFields {
+    NSMutableArray *array = [NSMutableArray array];
+    (self.textField1.length > 0) ? [array addObject:self.textField1] : nil;
+    (self.textField2.length > 0) ? [array addObject:self.textField2] : nil;
+    (self.textField3.length > 0) ? [array addObject:self.textField3] : nil;
+    (self.textField4.length > 0) ? [array addObject:self.textField4] : nil;
 
-    return newShow;
+    return [array copy];
 }
 
-- (void)sdl_updateCurrentScreenDataFromShow:(SDLShow *)show {
-    // If the items are nil, they were not updated, so we can't just set it directly
-    self.currentScreenData.mainField1 = show.mainField1 ?: self.currentScreenData.mainField1;
-    self.currentScreenData.mainField2 = show.mainField2 ?: self.currentScreenData.mainField2;
-    self.currentScreenData.mainField3 = show.mainField3 ?: self.currentScreenData.mainField3;
-    self.currentScreenData.mainField4 = show.mainField4 ?: self.currentScreenData.mainField4;
-    self.currentScreenData.mediaTrack = show.mediaTrack ?: self.currentScreenData.mediaTrack;
-    self.currentScreenData.templateTitle = show.templateTitle ?: self.currentScreenData.templateTitle;
-    self.currentScreenData.metadataTags = show.metadataTags ?: self.currentScreenData.metadataTags;
-    self.currentScreenData.alignment = show.alignment ?: self.currentScreenData.alignment;
-    self.currentScreenData.graphic = show.graphic ?: self.currentScreenData.graphic;
-    self.currentScreenData.secondaryGraphic = show.secondaryGraphic ?: self.currentScreenData.secondaryGraphic;
-}
-
-#pragma mark - Helpers
-
 - (BOOL)sdl_hasData {
     BOOL hasTextFields = ([self sdl_findNonNilTextFields].count > 0) || (self.title.length > 0) || (self.mediaTrackTextField.length > 0);
     BOOL hasImageFields = (self.primaryGraphic != nil) || (self.secondaryGraphic != nil);
@@ -230,24 +213,8 @@ NS_ASSUME_NONNULL_BEGIN
     return hasTextFields || hasImageFields;
 }
 
-#pragma mark - Equality
-
-- (BOOL)sdl_showImages:(SDLShow *)show isEqualToShowImages:(SDLShow *)show2 {
-    BOOL same = NO;
-    same = ((show.graphic.value == nil && show.graphic.value == nil)
-            || [show.graphic.value isEqualToString:show2.graphic.value]);
-    if (!same) { return NO; }
-
-    same = ((show.secondaryGraphic.value == nil && show.secondaryGraphic.value == nil)
-            || [show.secondaryGraphic.value isEqualToString:show2.secondaryGraphic.value]);
-
-    return same;
-}
-
 #pragma mark - Getters / Setters
 
-#pragma mark - Setters
-
 - (void)setTextField1:(nullable NSString *)textField1 {
     _textField1 = textField1;
     _isDirty = YES;
@@ -353,10 +320,6 @@ NS_ASSUME_NONNULL_BEGIN
     }
 }
 
-- (BOOL)hasQueuedUpdate {
-    return (_hasQueuedUpdate || _queuedUpdateHandler != nil);
-}
-
 - (nullable SDLArtwork *)blankArtwork {
     if (_blankArtwork != nil) {
         return _blankArtwork;
diff --git a/SmartDeviceLink/SDLTextAndGraphicUpdateOperation.h b/SmartDeviceLink/SDLTextAndGraphicUpdateOperation.h
index 99f59de31..aca862e96 100644
--- a/SmartDeviceLink/SDLTextAndGraphicUpdateOperation.h
+++ b/SmartDeviceLink/SDLTextAndGraphicUpdateOperation.h
@@ -24,6 +24,10 @@ typedef void(^SDLTextAndGraphicUpdateCompletionHandler)(NSError *__nullable erro
 
 @interface SDLTextAndGraphicUpdateOperation : SDLAsynchronousOperation
 
+/// The current state of the screen in Show form. This is passed as a dependency in the init but it may need to be updated if a previous operation updated the state of the screen.
+@property (strong, nonatomic) SDLShow *currentScreenData;
+
+/// An "output" property with the data sent by this operation.
 @property (strong, nonatomic, nullable, readonly) SDLShow *sentShow;
 
 /// Initialize the operation with its dependencies
diff --git a/SmartDeviceLink/SDLTextAndGraphicUpdateOperation.m b/SmartDeviceLink/SDLTextAndGraphicUpdateOperation.m
index 179cf2603..3942da871 100644
--- a/SmartDeviceLink/SDLTextAndGraphicUpdateOperation.m
+++ b/SmartDeviceLink/SDLTextAndGraphicUpdateOperation.m
@@ -24,7 +24,6 @@
 @property (weak, nonatomic) id<SDLConnectionManagerType> connectionManager;
 @property (weak, nonatomic) SDLFileManager *fileManager;
 @property (strong, nonatomic) SDLWindowCapability *currentCapabilities;
-@property (strong, nonatomic) SDLShow *currentScreenData;
 @property (strong, nonatomic) SDLTextAndGraphicState *updatedState;
 
 @property (copy, nonatomic, nullable) SDLTextAndGraphicUpdateCompletionHandler updateCompletionHandler;
@@ -35,7 +34,7 @@
 
 @implementation SDLTextAndGraphicUpdateOperation
 
-- (instancetype)initWithConnectionManager:(id<SDLConnectionManagerType>)connectionManager fileManager:(SDLFileManager *)fileManager currentCapabilities:(SDLWindowCapability *)currentCapabilities currentScreenData:(SDLShow *)currentData newState:(nonnull SDLTextAndGraphicState *)newState {
+- (instancetype)initWithConnectionManager:(id<SDLConnectionManagerType>)connectionManager fileManager:(SDLFileManager *)fileManager currentCapabilities:(SDLWindowCapability *)currentCapabilities currentScreenData:(SDLShow *)currentData newState:(nonnull SDLTextAndGraphicState *)newState updateCompletionHandler:(nullable SDLTextAndGraphicUpdateCompletionHandler)updateCompletionHandler {
     self = [self init];
     if (!self) { return nil; }
 
@@ -44,6 +43,7 @@
     _currentCapabilities = currentCapabilities;
     _currentScreenData = currentData;
     _updatedState = newState;
+    _updateCompletionHandler = updateCompletionHandler;
 
     return self;
 }
@@ -59,74 +59,85 @@
     fullShow = [self sdl_assembleShowText:fullShow];
     fullShow = [self sdl_assembleShowImages:fullShow];
 
-    SDLShow *showToSend = nil;
-
+    __weak typeof(self) weakSelf = self;
     if (!([self sdl_shouldUpdatePrimaryImage] || [self sdl_shouldUpdateSecondaryImage])) {
         SDLLogV(@"No images to send, sending text");
         // If there are no images to update, just send the text
-        showToSend = [self sdl_extractTextFromShow:fullShow];
+        [self sdl_sendShow:[self sdl_extractTextFromShow:fullShow] withHandler:^(NSError * _Nullable error) {
+            __strong typeof(weakSelf) strongSelf = weakSelf;
+            if (error != nil) {
+                strongSelf.internalError = error;
+            }
+            [strongSelf finishOperation];
+        }];
     } else if (![self sdl_artworkNeedsUpload:self.updatedState.primaryGraphic] && ![self sdl_artworkNeedsUpload:self.updatedState.secondaryGraphic]) {
         SDLLogV(@"Images already uploaded, sending full update");
         // The files to be updated are already uploaded, send the full show immediately
-        showToSend = fullShow;
+        [self sdl_sendShow:fullShow withHandler:^(NSError * _Nullable error) {
+            __strong typeof(weakSelf) strongSelf = weakSelf;
+            if (error != nil) {
+                strongSelf.internalError = error;
+            }
+            [strongSelf finishOperation];
+        }];
     } else {
         SDLLogV(@"Images need to be uploaded, sending text and uploading images");
 
         // We need to upload or queue the upload of the images
         // Send the text immediately
-        showToSend = [self sdl_extractTextFromShow:fullShow];
+        [self sdl_sendShow:[self sdl_extractTextFromShow:fullShow] withHandler:^(NSError * _Nullable error) {
+            __strong typeof(weakSelf) strongSelf = weakSelf;
+            if (self.cancelled) {
+                [strongSelf finishOperation];
+            }
+
+            [strongSelf sdl_uploadImagesAndSendWhenDone:^(NSError * _Nullable error) {
+                __strong typeof(weakSelf) strongSelf = weakSelf;
+                if (error != nil) {
+                    strongSelf.internalError = error;
+                }
+                [strongSelf finishOperation];
+            }];
+        }];
     }
+}
+
+#pragma mark - Send Show
 
-    // Send the initial, and potentially only, Show request
+- (void)sdl_sendShow:(SDLShow *)show withHandler:(void (^)(NSError *_Nullable error))handler {
     __weak typeof(self)weakSelf = self;
-    [self.connectionManager sendConnectionRequest:showToSend withResponseHandler:^(__kindof SDLRPCRequest * _Nullable request, __kindof SDLRPCResponse * _Nullable response, NSError * _Nullable error) {
+    [self.connectionManager sendConnectionRequest:show withResponseHandler:^(__kindof SDLRPCRequest * _Nullable request, __kindof SDLRPCResponse * _Nullable response, NSError * _Nullable error) {
         __strong typeof(weakSelf) strongSelf = weakSelf;
         SDLLogD(@"Text and Graphic update completed");
 
-        // TODO: Monitor and delete old images when space is low?
         if (response.success) {
             [strongSelf sdl_updateCurrentScreenDataFromShow:(SDLShow *)request];
-            [self sdl_uploadImagesAndSendWhenDone];
-        } else {
-            // It failed, store the error and pass it along
-            self.internalError = error;
         }
+
+        handler(error);
     }];
 }
 
-- (void)sdl_uploadImagesAndSendWhenDone {
-    // Start uploading the images
+#pragma mark - Uploading Images
+
+- (void)sdl_uploadImagesAndSendWhenDone:(void (^)(NSError *_Nullable error))handler {
     __weak typeof(self)weakSelf = self;
     [self sdl_uploadImagesWithCompletionHandler:^(NSError *_Nullable error) {
         __strong typeof(weakSelf) strongSelf = weakSelf;
 
-        if (error != nil) {
-            SDLShow *showWithGraphics = [self sdl_createImageOnlyShowWithPrimaryArtwork:self.newState.primaryGraphic secondaryArtwork:self.newState.secondaryGraphic];
-            if (showWithGraphics != nil) {
-                SDLLogW(@"Some images failed to upload. Sending update with the successfully uploaded images");
-                self.inProgressUpdate = showWithGraphics;
-            } else {
-                SDLLogE(@"All images failed to upload. No graphics to show, skipping update.");
-                self.inProgressUpdate = nil;
-            }
-            return;
-        }
-
-        // Check if queued image update still matches our images (there could have been a new Show in the meantime) and send a new update if it does. Since the images will already be on the head unit, the whole show will be sent
-        // TODO: Send delete if it doesn't?
-        if ([strongSelf sdl_showImages:thisUpdate isEqualToShowImages:strongSelf.queuedImageUpdate]) {
-            SDLLogV(@"Queued image update matches the images we need, sending update");
-            return [strongSelf sdl_updateWithCompletionHandler:strongSelf.inProgressHandler];
+        SDLShow *showWithGraphics = [self sdl_createImageOnlyShowWithPrimaryArtwork:self.updatedState.primaryGraphic secondaryArtwork:self.updatedState.secondaryGraphic];
+        if (showWithGraphics != nil) {
+            SDLLogD(@"Sending update with the successfully uploaded images");
+            [strongSelf sdl_sendShow:showWithGraphics withHandler:^(NSError * _Nullable error) {
+                return handler(error);
+            }];
         } else {
-            SDLLogV(@"Queued image update does not match the images we need, skipping update");
+            SDLLogW(@"All images failed to upload. No graphics to show, skipping update.");
+            return handler(error);
         }
     }];
-    // When the images are done uploading, send another show with the images
-    self.queuedImageUpdate = fullShow;
 }
 
-#pragma mark - Uploading Images
-
 - (void)sdl_uploadImagesWithCompletionHandler:(void (^)(NSError *_Nullable error))handler {
     NSMutableArray<SDLArtwork *> *artworksToUpload = [NSMutableArray array];
     if ([self sdl_shouldUpdatePrimaryImage] && !self.updatedState.primaryGraphic.isStaticIcon) {
@@ -141,6 +152,7 @@
         return handler(nil);
     }
 
+    // TODO: Use progress handler
     [self.fileManager uploadArtworks:artworksToUpload completionHandler:^(NSArray<NSString *> * _Nonnull artworkNames, NSError * _Nullable error) {
         if (error != nil) {
             SDLLogW(@"Text and graphic manager artwork failed to upload with error: %@", error.localizedDescription);
@@ -170,6 +182,19 @@
     return show;
 }
 
+- (nullable SDLShow *)sdl_createImageOnlyShowWithPrimaryArtwork:(nullable SDLArtwork *)primaryArtwork secondaryArtwork:(nullable SDLArtwork *)secondaryArtwork  {
+    SDLShow *newShow = [[SDLShow alloc] init];
+    newShow.graphic = ![self sdl_artworkNeedsUpload:primaryArtwork] ? primaryArtwork.imageRPC : nil;
+    newShow.secondaryGraphic = ![self sdl_artworkNeedsUpload:secondaryArtwork] ? secondaryArtwork.imageRPC : nil;
+
+    if (newShow.graphic == nil && newShow.secondaryGraphic == nil) {
+        SDLLogV(@"No graphics to upload");
+        return nil;
+    }
+
+    return newShow;
+}
+
 #pragma mark Text
 
 - (SDLShow *)sdl_assembleShowText:(SDLShow *)show {
@@ -424,4 +449,24 @@
     return [array copy];
 }
 
+#pragma mark - Operation Overrides
+
+- (void)finishOperation {
+    SDLLogV(@"Finishing text and graphic update operation");
+    self.updateCompletionHandler(self.error);
+    [super finishOperation];
+}
+
+- (nullable NSString *)name {
+    return @"com.sdl.textandgraphic.update";
+}
+
+- (NSOperationQueuePriority)queuePriority {
+    return NSOperationQueuePriorityNormal;
+}
+
+- (nullable NSError *)error {
+    return self.internalError;
+}
+
 @end