Fix double-callback and nil category in PostCategoryService create

Tests/KeystoneTests/Tests/Services/PostCategoryServiceTests.m

@@ -149,4 +149,81 @@ - (void)testSyncFailureShouldBeCalledOnce
     [self waitForExpectations:@[completion] timeout:1];
 }
 
+/// Regression: when the save context can't resolve the blog, `success` must not
+/// also fire. Previously the completion called `success(nil)` alongside `failure`
+/// (a double callback that passed nil into the non-null `PostCategory` block).
+/// The blog is intentionally left unsaved (see `setUp`), so its temporary
+/// objectID doesn't resolve in the background save context — the "no blog" path.
+- (void)testThatCreateCategoryDoesNotAlsoCallSuccessWhenBlogIsMissing
+{
+    TaxonomyServiceRemoteREST *remote = self.service.remoteForStubbing;
+
+    RemotePostCategory *received = [RemotePostCategory new];
+    received.categoryID = @123;
+    received.name = @"category name";
+    received.parentID = @0;
+
+    OCMStub([remote createCategory:[OCMArg any]
+                           success:([OCMArg invokeBlockWithArgs:received, nil])
+                           failure:[OCMArg any]]);
+
+    XCTestExpectation *failed = [self expectationWithDescription:@"failure is called"];
+    XCTestExpectation *successNotCalled = [self expectationWithDescription:@"success is not called"];
+    successNotCalled.inverted = YES;
+
+    [self.service createCategoryWithName:@"category name"
+                 parentCategoryObjectID:nil
+                        forBlogObjectID:self.blog.objectID
+                                success:^(PostCategory * _Nonnull __unused category) {
+        [successNotCalled fulfill];
+    } failure:^(NSError * _Nonnull __unused error) {
+        // Failure is delivered from the main-queue completion, not the background save context.
+        XCTAssertTrue([NSThread isMainThread]);
+        [failed fulfill];
+    }];
+
+    [self waitForExpectations:@[failed, successNotCalled] timeout:1];
+}
+
+/// Happy path: when the blog resolves in the save context, the created category
+/// is looked up and handed to `success` exactly once (on the main queue) and
+/// `failure` is not called. The blog is saved first — unlike the no-blog test
+/// above — so its permanent objectID resolves in the background save context.
+- (void)testThatCreateCategoryCallsSuccessWithTheCreatedCategory
+{
+    [self.manager saveContextAndWait:self.manager.mainContext];
+
+    TaxonomyServiceRemoteREST *remote = self.service.remoteForStubbing;
+
+    RemotePostCategory *received = [RemotePostCategory new];
+    received.categoryID = @123;
+    received.name = @"category name";
+    received.parentID = @0;
+
+    OCMStub([remote createCategory:[OCMArg any]
+                           success:([OCMArg invokeBlockWithArgs:received, nil])
+                           failure:[OCMArg any]]);
+
+    XCTestExpectation *succeeded = [self expectationWithDescription:@"success is called with the created category"];
+    XCTestExpectation *failureNotCalled = [self expectationWithDescription:@"failure is not called"];
+    failureNotCalled.inverted = YES;
+
+    [self.service createCategoryWithName:@"category name"
+                 parentCategoryObjectID:nil
+                        forBlogObjectID:self.blog.objectID
+                                success:^(PostCategory * _Nonnull category) {
+        // Success is delivered from the main-queue completion.
+        XCTAssertTrue([NSThread isMainThread]);
+        XCTAssertNotNil(category);
+        XCTAssertEqualObjects(category.categoryID, @123);
+        XCTAssertEqualObjects(category.categoryName, @"category name");
+        XCTAssertEqualObjects(category.parentID, @0);
+        [succeeded fulfill];
+    } failure:^(NSError * _Nonnull __unused error) {
+        [failureNotCalled fulfill];
+    }];
+
+    [self waitForExpectations:@[succeeded, failureNotCalled] timeout:1];
+}
+
 @end

WordPress/Classes/Services/PostCategoryService.h

@@ -8,7 +8,8 @@ NS_ASSUME_NONNULL_BEGIN
 @protocol CoreDataStack;
 
 typedef NS_ENUM(NSInteger, PostCategoryServiceErrors) {
-    PostCategoryServiceErrorsBlogNotFound
+    PostCategoryServiceErrorsBlogNotFound,
+    PostCategoryServiceErrorsCategoryNotFound
 };
 
 @interface PostCategoryService : NSObject
@@ -39,8 +40,8 @@ typedef NS_ENUM(NSInteger, PostCategoryServiceErrors) {
 - (void)createCategoryWithName:(NSString *)name
         parentCategoryObjectID:(nullable NSManagedObjectID *)parentCategoryObjectID
                forBlogObjectID:(NSManagedObjectID *)blogObjectID
-                       success:(nullable void (^)(PostCategory *category))success
-                       failure:(nullable void (^)(NSError *error))failure;
+                       success:(void (^)(PostCategory *category))success
+                       failure:(void (^)(NSError *error))failure;
 @end
 
 NS_ASSUME_NONNULL_END

WordPress/Classes/Services/PostCategoryService.m

@@ -22,6 +22,13 @@ - (NSError *)serviceErrorNoBlog
                            userInfo:nil];
 }
 
+- (NSError *)serviceErrorCategoryNotFound
+{
+    return [NSError errorWithDomain:NSStringFromClass([self class])
+                               code:PostCategoryServiceErrorsCategoryNotFound
+                           userInfo:nil];
+}
+
 - (void)syncCategoriesForBlog:(Blog *)blog
                       success:(nullable void (^)(void))success
                       failure:(nullable void (^)(NSError *error))failure
@@ -88,8 +95,8 @@ - (void)syncCategoriesForBlog:(Blog *)blog
 - (void)createCategoryWithName:(NSString *)name
         parentCategoryObjectID:(nullable NSManagedObjectID *)parentCategoryObjectID
                forBlogObjectID:(NSManagedObjectID *)blogObjectID
-                       success:(nullable void (^)(PostCategory *category))success
-                       failure:(nullable void (^)(NSError *error))failure
+                       success:(void (^)(PostCategory *category))success
+                       failure:(void (^)(NSError *error))failure
 {
     NSParameterAssert(name != nil);
     Blog * __block blog = nil;
@@ -108,12 +115,11 @@ - (void)createCategoryWithName:(NSString *)name
     id<TaxonomyServiceRemote> remote = [self remoteForBlog:blog];
     [remote createCategory:remoteCategory
                    success:^(RemotePostCategory *receivedCategory) {
+                       NSError * __block error = nil;
                        [self.coreDataStack performAndSaveUsingBlock:^(NSManagedObjectContext *context) {
                            Blog *blog = [context existingObjectWithID:blogObjectID error:nil];
                            if (!blog) {
-                               if (failure) {
-                                   failure([self serviceErrorNoBlog]);
-                               }
+                               error = [self serviceErrorNoBlog];
                                return;
                            }
                            PostCategory *newCategory = [PostCategory createWithBlogObjectID:blogObjectID inContext:context];
@@ -129,11 +135,23 @@ - (void)createCategoryWithName:(NSString *)name
                                newCategory.parentID = @0;
                            }
                        } completion:^{
-                           if (success) {
+                           // Deliver exactly one callback. `failure` is invoked from the
+                           // main-queue completion (not the background save block) so the
+                           // no-blog path matches the threading of the success path.
+                           if (error) {
+                               failure(error);
+                           } else {
                                PostCategory *newCategory = [PostCategory lookupWithBlogObjectID:blogObjectID
                                                                            categoryID:receivedCategory.categoryID
                                                                             inContext:[self.coreDataStack mainContext]];
-                               success(newCategory);
+                               if (newCategory) {
+                                   success(newCategory);
+                               } else {
+                                   // The category was created remotely but couldn't be
+                                   // resolved locally (e.g. the response had no usable ID).
+                                   // Report failure rather than dropping the callback.
+                                   failure([self serviceErrorCategoryNotFound]);
+                               }
                            }
                            if ([remote isKindOfClass:[TaxonomyServiceRemoteXMLRPC class]]) {
                                // XML-RPC only returns ID, let's fetch the new category as