Cherry pick: Remove accessors for prioritizer internals onto 6.22.0 (#5396)

* Remove accessors for prioritizer internals (#5352)

* Remove accessors for prioritizer internals

This effectively makes it impossible to access the event sets in an unsynchronized way.

Fixes #5312

* Fix typoo

* Update CHANGELOG and GDTCCT version

GoogleDataTransportCCTSupport.podspec

@@ -1,7 +1,7 @@
 
 Pod::Spec.new do |s|
   s.name             = 'GoogleDataTransportCCTSupport'
-  s.version          = '2.0.2'
+  s.version          = '2.0.3'
   s.summary          = 'Support library for the GoogleDataTransport CCT backend target.'
 
 

GoogleDataTransportCCTSupport/CHANGELOG.md

@@ -1,4 +1,7 @@
-# Unreleased
+# v2.0.3
+- Synchronize prioritizer property access using a new method. (#5312)
+
+# v2.0.2
 - Remove usage of memcpy and convert calls from malloc to calloc.
 
 # v2.0.1

GoogleDataTransportCCTSupport/GDTCCTLibrary/GDTCCTPrioritizer.m

@@ -39,6 +39,20 @@ static NSString *ArchivePath() {
   return archivePath;
 }
 
+/** This class extension is for declaring private properties. */
+@interface GDTCCTPrioritizer ()
+
+/** All CCT events that have been processed by this prioritizer. */
+@property(nonatomic) NSMutableSet<GDTCOREvent *> *CCTEvents;
+
+/** All FLL events that have been processed by this prioritizer. */
+@property(nonatomic) NSMutableSet<GDTCOREvent *> *FLLEvents;
+
+/** All CSH events that have been processed by this prioritizer. */
+@property(nonatomic) NSMutableSet<GDTCOREvent *> *CSHEvents;
+
+@end
+
 @implementation GDTCCTPrioritizer
 
 + (void)load {
@@ -72,6 +86,29 @@ static NSString *ArchivePath() {
   return self;
 }
 
+- (nullable NSSet *)eventsForTarget:(GDTCORTarget)target {
+  __block NSSet *events;
+  dispatch_sync(_queue, ^{
+    switch (target) {
+      case kGDTCORTargetCCT:
+        events = [self->_CCTEvents copy];
+        break;
+
+      case kGDTCORTargetFLL:
+        events = [self->_FLLEvents copy];
+        break;
+
+      case kGDTCORTargetCSH:
+        events = [self->_CSHEvents copy];
+        break;
+
+      default:
+        break;
+    }
+  });
+  return events;
+}
+
 #pragma mark - GDTCORPrioritizer Protocol
 
 - (void)prioritizeEvent:(GDTCOREvent *)event {

GoogleDataTransportCCTSupport/GDTCCTLibrary/GDTCCTUploader.m

@@ -276,13 +276,10 @@ NSNotificationName const GDTCCTUploadCompleteNotification = @"com.GDTCCTUploader
 
 - (BOOL)readyToUploadTarget:(GDTCORTarget)target conditions:(GDTCORUploadConditions)conditions {
   __block BOOL result = NO;
+  NSSet *CSHEvents = [[GDTCCTPrioritizer sharedInstance] eventsForTarget:kGDTCORTargetCSH];
   dispatch_sync(_uploaderQueue, ^{
     if (target == kGDTCORTargetCSH) {
-      if ([GDTCCTPrioritizer sharedInstance].CSHEvents.count > 0) {
-        result = YES;
-      } else {
-        result = NO;
-      }
+      result = CSHEvents.count > 0;
       return;
     }
 

GoogleDataTransportCCTSupport/GDTCCTLibrary/Private/GDTCCTPrioritizer.h

@@ -29,15 +29,6 @@ NS_ASSUME_NONNULL_BEGIN
 /** The queue on which this prioritizer operates. */
 @property(nonatomic) dispatch_queue_t queue;
 
-/** All CCT events that have been processed by this prioritizer. */
-@property(nonatomic) NSMutableSet<GDTCOREvent *> *CCTEvents;
-
-/** All FLL events that have been processed by this prioritizer. */
-@property(nonatomic) NSMutableSet<GDTCOREvent *> *FLLEvents;
-
-/** All CSH events that have been processed by this prioritizer. */
-@property(nonatomic) NSMutableSet<GDTCOREvent *> *CSHEvents;
-
 /** The most recent attempted upload of CCT daily uploaded logs. */
 @property(nonatomic) GDTCORClock *CCTTimeOfLastDailyUpload;
 
@@ -50,6 +41,13 @@ NS_ASSUME_NONNULL_BEGIN
  */
 + (instancetype)sharedInstance;
 
+/** Returns a set of events that have been prioritized for the given target.
+ *
+ * @param target The target to check. CCT, FLL, and CSH are currently supported by this class.
+ * @return The set of events prioritized so far.
+ */
+- (nullable NSSet *)eventsForTarget:(GDTCORTarget)target;
+
 NS_ASSUME_NONNULL_END
 
 @end

GoogleDataTransportCCTSupport/GDTCCTTests/Unit/GDTCCTPrioritizerTest.m

@@ -55,11 +55,9 @@
   [prioritizer prioritizeEvent:[_CCTGenerator generateEvent:GDTCOREventQosDefault]];
   [prioritizer prioritizeEvent:[_FLLGenerator generateEvent:GDTCOREventQosDefault]];
   [prioritizer prioritizeEvent:[_CSHGenerator generateEvent:GDTCOREventQosDefault]];
-  dispatch_sync(prioritizer.queue, ^{
-    XCTAssertEqual(prioritizer.CCTEvents.count, 1);
-    XCTAssertEqual(prioritizer.FLLEvents.count, 1);
-    XCTAssertEqual(prioritizer.CSHEvents.count, 1);
-  });
+  XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCCT].count, 1);
+  XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetFLL].count, 1);
+  XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCSH].count, 1);
 }
 
 /** Tests prioritizing multiple events. */
@@ -74,11 +72,9 @@
   [prioritizer prioritizeEvent:[_CCTGenerator generateEvent:GDTCOREventQosDefault]];
   [prioritizer prioritizeEvent:[_CSHGenerator generateEvent:GDTCOREventQosDefault]];
   [prioritizer prioritizeEvent:[_CCTGenerator generateEvent:GDTCOREventQosDefault]];
-  dispatch_sync(prioritizer.queue, ^{
-    XCTAssertEqual(prioritizer.CCTEvents.count, 5);
-    XCTAssertEqual(prioritizer.FLLEvents.count, 2);
-    XCTAssertEqual(prioritizer.CSHEvents.count, 2);
-  });
+  XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCCT].count, 5);
+  XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetFLL].count, 2);
+  XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCSH].count, 2);
 }
 
 /** Tests unprioritizing events. */
@@ -93,11 +89,9 @@
   [prioritizer prioritizeEvent:[_CCTGenerator generateEvent:GDTCOREventQosDefault]];
   [prioritizer prioritizeEvent:[_CSHGenerator generateEvent:GDTCOREventQosDefault]];
   [prioritizer prioritizeEvent:[_CCTGenerator generateEvent:GDTCOREventQosDefault]];
-  dispatch_sync(prioritizer.queue, ^{
-    XCTAssertEqual(prioritizer.CCTEvents.count, 5);
-    XCTAssertEqual(prioritizer.FLLEvents.count, 2);
-    XCTAssertEqual(prioritizer.CSHEvents.count, 2);
-  });
+  XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCCT].count, 5);
+  XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetFLL].count, 2);
+  XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCSH].count, 2);
   GDTCORUploadPackage *package =
       [prioritizer uploadPackageWithTarget:kGDTCORTargetFLL
                                 conditions:GDTCORUploadConditionWifiData];
@@ -190,7 +184,7 @@
   NSError *error;
   dispatch_sync(prioritizer.queue, ^{
                 });
-  XCTAssertEqual(prioritizer.CCTEvents.count, 1);
+  XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCCT].count, 1);
   NSData *prioritizerData = GDTCOREncodeArchive(prioritizer, nil, &error);
   XCTAssertNil(error);
   XCTAssertNotNil(prioritizerData);
@@ -201,7 +195,8 @@
   XCTAssertNil(error);
   XCTAssertNotNil(unarchivedPrioritizer);
   XCTAssertEqual([prioritizer hash], [prioritizer hash]);
-  XCTAssertEqualObjects(prioritizer.CCTEvents, unarchivedPrioritizer.CCTEvents);
+  XCTAssertEqualObjects([prioritizer eventsForTarget:kGDTCORTargetCCT],
+                        [unarchivedPrioritizer eventsForTarget:kGDTCORTargetCCT]);
 }
 
 @end