Refactor usage of ASCollectionViewLayoutInspecting

- Fix not using itemSize of UICollectionViewFlowLayout
- Move automatic constrained size calculation to the ASCollectionViewFlowLayoutInspector
- Provide a null layout inspector for throwing exceptions if a custom
  UICollectionView is given but no ASCollectionViewLayoutInspecting
- Fix not checking for optional layout inspecting data source methods
  are implemented or not
- Improving tests around ASCollectionViewLayoutInspecting

AsyncDisplayKit/ASCollectionView.mm

@@ -38,7 +38,6 @@ typedef NS_ENUM(NSUInteger, ASCollectionViewInvalidationStyle) {
 };
 
 static const NSUInteger kASCollectionViewAnimationNone = UITableViewRowAnimationNone;
-static const ASSizeRange kInvalidSizeRange = {CGSizeZero, CGSizeZero};
 static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 
 #pragma mark -
@@ -95,7 +94,7 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
   ASCollectionDataController *_dataController;
   ASRangeController *_rangeController;
   ASCollectionViewLayoutController *_layoutController;
-  ASCollectionViewFlowLayoutInspector *_flowLayoutInspector;
+  id<ASCollectionViewLayoutInspecting> _defaultLayoutInspector;
   NSMutableSet *_cellsForVisibilityUpdates;
   id<ASCollectionViewLayoutFacilitatorProtocol> _layoutFacilitator;
   
@@ -246,11 +245,19 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
   // and should not trigger a relayout.
   _ignoreMaxSizeChange = CGSizeEqualToSize(_maxSizeForNodesConstrainedSize, CGSizeZero);
   
-  // Register the default layout inspector delegate for flow layouts only, custom layouts
-  // will need to roll their own ASCollectionViewLayoutInspecting implementation and set a layout delegate
   if ([layout asdk_isFlowLayout]) {
-    _layoutInspector = [self flowLayoutInspector];
+    // Register the default layout inspector delegate for flow layouts only
+    UICollectionViewFlowLayout *layout = (UICollectionViewFlowLayout *)self.collectionViewLayout;
+    ASDisplayNodeAssertNotNil(layout, @"Collection view layout must be a flow layout to use the built-in inspector");
+    _defaultLayoutInspector = [[ASCollectionViewFlowLayoutInspector alloc] initWithCollectionView:self flowLayout:layout];
+  } else {
+    // Custom layouts will need to roll their own ASCollectionViewLayoutInspecting implementation and set a layout
+    // delegate. In the meantime ASDK provides a null layout inspector that does not provide any implementation
+    // and throws an exception for methods that should be implemented in the <ASCollectionViewLayoutInspecting>
+    _defaultLayoutInspector = [[ASCollectionViewNullLayoutInspector alloc] init];
   }
+  _layoutInspector = _defaultLayoutInspector;
+  
   _layoutFacilitator = layoutFacilitator;
   
   _proxyDelegate = [[ASCollectionViewProxy alloc] initWithTarget:nil interceptor:self];
@@ -281,19 +288,6 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
   [self setAsyncDataSource:nil];
 }
 
-/**
- * A layout inspector implementation specific for the sizing behavior of UICollectionViewFlowLayouts
- */
-- (ASCollectionViewFlowLayoutInspector *)flowLayoutInspector
-{
-  if (_flowLayoutInspector == nil) {
-    UICollectionViewFlowLayout *layout = (UICollectionViewFlowLayout *)self.collectionViewLayout;
-    ASDisplayNodeAssertNotNil(layout, @"Collection view layout must be a flow layout to use the built-in inspector");
-    _flowLayoutInspector = [[ASCollectionViewFlowLayoutInspector alloc] initWithCollectionView:self flowLayout:layout];
-  }
-  return _flowLayoutInspector;
-}
-
 #pragma mark -
 #pragma mark Overrides.
 
@@ -379,6 +373,10 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
   }
   
   super.dataSource = (id<UICollectionViewDataSource>)_proxyDataSource;
+    
+  if ([_layoutInspector respondsToSelector:@selector(didChangeCollectionViewDataSource:)]) {
+    [_layoutInspector didChangeCollectionViewDataSource:asyncDataSource];
+  }
 }
 
 - (void)setAsyncDelegate:(id<ASCollectionViewDelegate>)asyncDelegate
@@ -411,7 +409,9 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 
   super.delegate = (id<UICollectionViewDelegate>)_proxyDelegate;
   
-  [_layoutInspector didChangeCollectionViewDelegate:asyncDelegate];
+  if ([_layoutInspector respondsToSelector:@selector(didChangeCollectionViewDelegate:)]) {
+    [_layoutInspector didChangeCollectionViewDelegate:asyncDelegate];
+  }
 }
 
 - (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeType:(ASLayoutRangeType)rangeType
@@ -943,30 +943,7 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 
 - (ASSizeRange)dataController:(ASDataController *)dataController constrainedSizeForNodeAtIndexPath:(NSIndexPath *)indexPath
 {
-  ASSizeRange constrainedSize = kInvalidSizeRange;
-  if (_layoutInspector) {
-    constrainedSize = [_layoutInspector collectionView:self constrainedSizeForNodeAtIndexPath:indexPath];
-  }
-  
-  if (!ASSizeRangeEqualToSizeRange(constrainedSize, kInvalidSizeRange)) {
-    return constrainedSize;
-  }
-  
-  // TODO: Move this logic into the flow layout inspector. Create a simple inspector for non-flow layouts that don't
-  // implement a custom inspector.
-  if (_asyncDataSourceFlags.asyncDataSourceConstrainedSizeForNode) {
-    constrainedSize = [_asyncDataSource collectionView:self constrainedSizeForNodeAtIndexPath:indexPath];
-  } else {
-    CGSize maxSize = CGSizeEqualToSize(_maxSizeForNodesConstrainedSize, CGSizeZero) ? self.bounds.size : _maxSizeForNodesConstrainedSize;
-    if (ASScrollDirectionContainsHorizontalDirection([self scrollableDirections])) {
-      maxSize.width = FLT_MAX;
-    } else {
-      maxSize.height = FLT_MAX;
-    }
-    constrainedSize = ASSizeRangeMake(CGSizeZero, maxSize);
-  }
-  
-  return constrainedSize;
+  return [_layoutInspector collectionView:self constrainedSizeForNodeAtIndexPath:indexPath];
 }
 
 - (NSUInteger)dataController:(ASDataController *)dataController rowsInSection:(NSUInteger)section
@@ -1006,19 +983,16 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 
 - (ASSizeRange)dataController:(ASCollectionDataController *)dataController constrainedSizeForSupplementaryNodeOfKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath
 {
-  ASDisplayNodeAssert(_layoutInspector != nil, @"To support supplementary nodes in ASCollectionView, it must have a layoutDelegate for layout inspection. (See ASCollectionViewFlowLayoutInspector for an example.)");
   return [_layoutInspector collectionView:self constrainedSizeForSupplementaryNodeOfKind:kind atIndexPath:indexPath];
 }
 
 - (NSUInteger)dataController:(ASCollectionDataController *)dataController supplementaryNodesOfKind:(NSString *)kind inSection:(NSUInteger)section
 {
-  ASDisplayNodeAssert(_layoutInspector != nil, @"To support supplementary nodes in ASCollectionView, it must have a layoutDelegate for layout inspection. (See ASCollectionViewFlowLayoutInspector for an example.)");
   return [_layoutInspector collectionView:self supplementaryNodesOfKind:kind inSection:section];
 }
 
 - (NSUInteger)dataController:(ASCollectionDataController *)dataController numberOfSectionsForSupplementaryNodeOfKind:(NSString *)kind;
 {
-  ASDisplayNodeAssert(_layoutInspector != nil, @"To support supplementary nodes in ASCollectionView, it must have a layoutDelegate for layout inspection. (See ASCollectionViewFlowLayoutInspector for an example.)");
   return [_layoutInspector collectionView:self numberOfSectionsForSupplementaryNodeOfKind:kind];
 }
 

AsyncDisplayKit/Details/ASCollectionViewFlowLayoutInspector.h

@@ -14,12 +14,15 @@
 #import <AsyncDisplayKit/ASDimension.h>
 
 @class ASCollectionView;
+@protocol ASCollectionDataSource;
 @protocol ASCollectionDelegate;
 
+NS_ASSUME_NONNULL_BEGIN
+
 @protocol ASCollectionViewLayoutInspecting <NSObject>
 
 /**
- * Provides the size range needed to measure the collection view's item.
+ * Asks the inspector to provide a constarained size range for the given collection view node.
  */
 - (ASSizeRange)collectionView:(ASCollectionView *)collectionView constrainedSizeForNodeAtIndexPath:(NSIndexPath *)indexPath;
 
@@ -47,12 +50,32 @@
  */
 - (void)didChangeCollectionViewDelegate:(id<ASCollectionDelegate>)delegate;
 
+/**
+ * Allow the inspector to respond to dataSource changes.
+ *
+ * @discussion A great time to update perform selector caches!
+ */
+- (void)didChangeCollectionViewDataSource:(id<ASCollectionDataSource>)dataSource;
+
 @end
 
+/**
+ * Simple "Null Object" inspector for non-flow layouts that does throws exceptions if methods are called
+ * from <ASCollectionViewLayoutInspecting>
+ */
+@interface ASCollectionViewNullLayoutInspector : NSObject <ASCollectionViewLayoutInspecting>
+
+@end
+
+/**
+ * A layout inspector implementation specific for the sizing behavior of UICollectionViewFlowLayouts
+ */
 @interface ASCollectionViewFlowLayoutInspector : NSObject <ASCollectionViewLayoutInspecting>
 
-@property (nonatomic, weak) UICollectionViewFlowLayout *layout;
+@property (nonatomic, weak, readonly) UICollectionViewFlowLayout *layout;
 
 - (instancetype)initWithCollectionView:(ASCollectionView *)collectionView flowLayout:(UICollectionViewFlowLayout *)flowLayout;
 
 @end
+
+NS_ASSUME_NONNULL_END

AsyncDisplayKit/Details/ASCollectionViewFlowLayoutInspector.m

@@ -8,29 +8,72 @@
 //  of patent rights can be found in the PATENTS file in the same directory.
 //
 
-#import <UIKit/UIKit.h>
-
 #import "ASCollectionViewFlowLayoutInspector.h"
 #import "ASCollectionView.h"
 #import "ASAssert.h"
 #import "ASEqualityHelpers.h"
 
+#define kDefaultItemSize CGSizeMake(50, 50)
+
+#pragma mark - ASCollectionViewNullLayoutInspector
+
+@implementation ASCollectionViewNullLayoutInspector
+
+- (ASSizeRange)collectionView:(ASCollectionView *)collectionView constrainedSizeForNodeAtIndexPath:(NSIndexPath *)indexPath
+{
+  ASDisplayNodeAssert(NO, @"To support a custom collection view layout in ASCollectionView, it must have a layoutInspector for layout inspection. (See ASCollectionViewFlowLayoutInspector for an example.)");
+  return ASSizeRangeMake(CGSizeZero, CGSizeZero);
+}
+
+- (ASSizeRange)collectionView:(ASCollectionView *)collectionView constrainedSizeForSupplementaryNodeOfKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath
+{
+  ASDisplayNodeAssert(NO, @"To support supplementary nodes in ASCollectionView, it must have a layoutInspector for layout inspection. (See ASCollectionViewFlowLayoutInspector for an example.)");
+  return ASSizeRangeMake(CGSizeZero, CGSizeZero);
+}
+
+- (NSUInteger)collectionView:(ASCollectionView *)collectionView numberOfSectionsForSupplementaryNodeOfKind:(NSString *)kind
+{
+  ASDisplayNodeAssert(NO, @"To support supplementary nodes in ASCollectionView, it must have a layoutInspector for layout inspection. (See ASCollectionViewFlowLayoutInspector for an example.)");
+  return 0;
+}
+
+- (NSUInteger)collectionView:(ASCollectionView *)collectionView supplementaryNodesOfKind:(NSString *)kind inSection:(NSUInteger)section
+{
+  ASDisplayNodeAssert(NO, @"To support supplementary nodes in ASCollectionView, it must have a layoutInspector for layout inspection. (See ASCollectionViewFlowLayoutInspector for an example.)");
+  return 0;
+}
+
+@end
+
+
+#pragma mark - ASCollectionViewFlowLayoutInspector
+
+@interface ASCollectionViewFlowLayoutInspector ()
+@property (nonatomic, weak) UICollectionViewFlowLayout *layout;
+@end
+ 
 @implementation ASCollectionViewFlowLayoutInspector {
-  BOOL _delegateImplementsReferenceSizeForHeader;
-  BOOL _delegateImplementsReferenceSizeForFooter;
+  struct {
+    unsigned int implementsReferenceSizeForHeader:1;
+    unsigned int implementsReferenceSizeForFooter:1;
+  } _delegateFlags;
+  
+  struct {
+    unsigned int implementsConstrainedSizeForNodeAtIndexPath:1;
+    unsigned int implementsNumberOfSectionsInCollectionView:1;
+  } _dataSourceFlags;
 }
 
 #pragma mark - Accessors
 
 - (instancetype)initWithCollectionView:(ASCollectionView *)collectionView flowLayout:(UICollectionViewFlowLayout *)flowLayout;
 {
+  NSParameterAssert(collectionView);
+  NSParameterAssert(flowLayout);
+  
   self = [super init];
-
-  if (flowLayout == nil) {
-    ASDisplayNodeAssert(NO, @"Should never create a layout inspector without a layout");
-  }
-
   if (self != nil) {
+    [self didChangeCollectionViewDataSource:collectionView.asyncDataSource];
     [self didChangeCollectionViewDelegate:collectionView.asyncDelegate];
     _layout = flowLayout;
   }
@@ -40,11 +83,20 @@
 - (void)didChangeCollectionViewDelegate:(id<ASCollectionDelegate>)delegate;
 {
   if (delegate == nil) {
-    _delegateImplementsReferenceSizeForHeader = NO;
-    _delegateImplementsReferenceSizeForFooter = NO;
+    memset(&_delegateFlags, 0, sizeof(_delegateFlags));
   } else {
-    _delegateImplementsReferenceSizeForHeader = [delegate respondsToSelector:@selector(collectionView:layout:referenceSizeForHeaderInSection:)];
-    _delegateImplementsReferenceSizeForFooter = [delegate respondsToSelector:@selector(collectionView:layout:referenceSizeForFooterInSection:)];
+    _delegateFlags.implementsReferenceSizeForHeader = [delegate respondsToSelector:@selector(collectionView:layout:referenceSizeForHeaderInSection:)];
+    _delegateFlags.implementsReferenceSizeForFooter = [delegate respondsToSelector:@selector(collectionView:layout:referenceSizeForFooterInSection:)];
+  }
+}
+
+- (void)didChangeCollectionViewDataSource:(id<ASCollectionDataSource>)dataSource
+{
+  if (dataSource == nil) {
+    memset(&_dataSourceFlags, 0, sizeof(_dataSourceFlags));
+  } else {
+    _dataSourceFlags.implementsConstrainedSizeForNodeAtIndexPath = [dataSource respondsToSelector:@selector(collectionView:constrainedSizeForNodeAtIndexPath:)];
+    _dataSourceFlags.implementsNumberOfSectionsInCollectionView = [dataSource respondsToSelector:@selector(numberOfSectionsInCollectionView:)];
   }
 }
 
@@ -52,8 +104,25 @@
 
 - (ASSizeRange)collectionView:(ASCollectionView *)collectionView constrainedSizeForNodeAtIndexPath:(NSIndexPath *)indexPath
 {
-  // TODO: Provide constrained size for flow layout item nodes
-  return ASSizeRangeMake(CGSizeZero, CGSizeZero);
+  // First check if delegate provides a constrained size
+  if (_dataSourceFlags.implementsConstrainedSizeForNodeAtIndexPath) {
+    return [collectionView.asyncDataSource collectionView:collectionView constrainedSizeForNodeAtIndexPath:indexPath];
+  }
+  
+  // Check if item size as constrained size is given
+  CGSize itemSize = _layout.itemSize;
+  if (!CGSizeEqualToSize(itemSize, kDefaultItemSize)) {
+    return ASSizeRangeMake(itemSize, itemSize);
+  }
+  
+  // No constrained size is given try to let the cells layout itself as far as possible based on the scrollable direction
+  CGSize maxSize = collectionView.bounds.size;
+  if (ASScrollDirectionContainsHorizontalDirection([collectionView scrollableDirections])) {
+    maxSize.width = FLT_MAX;
+  } else {
+    maxSize.height = FLT_MAX;
+  }
+  return ASSizeRangeMake(CGSizeZero, maxSize);
 }
 
 - (ASSizeRange)collectionView:(ASCollectionView *)collectionView constrainedSizeForSupplementaryNodeOfKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath
@@ -61,16 +130,16 @@
   CGSize constrainedSize;
   CGSize supplementarySize = [self sizeForSupplementaryViewOfKind:kind inSection:indexPath.section collectionView:collectionView];
   if (_layout.scrollDirection == UICollectionViewScrollDirectionVertical) {
-    constrainedSize = CGSizeMake(collectionView.bounds.size.width, supplementarySize.height);
+    constrainedSize = CGSizeMake(CGRectGetWidth(collectionView.bounds), supplementarySize.height);
   } else {
-    constrainedSize = CGSizeMake(supplementarySize.height, collectionView.bounds.size.height);
+    constrainedSize = CGSizeMake(supplementarySize.height, CGRectGetHeight(collectionView.bounds));
   }
   return ASSizeRangeMake(CGSizeZero, constrainedSize);
 }
 
 - (NSUInteger)collectionView:(ASCollectionView *)collectionView numberOfSectionsForSupplementaryNodeOfKind:(NSString *)kind
 {
-  if ([collectionView.asyncDataSource respondsToSelector:@selector(numberOfSectionsInCollectionView:)]) {
+  if (_dataSourceFlags.implementsNumberOfSectionsInCollectionView) {
     return [collectionView.asyncDataSource numberOfSectionsInCollectionView:collectionView];
   } else {
     return 1;
@@ -87,13 +156,13 @@
 - (CGSize)sizeForSupplementaryViewOfKind:(NSString *)kind inSection:(NSUInteger)section collectionView:(ASCollectionView *)collectionView
 {
   if (ASObjectIsEqual(kind, UICollectionElementKindSectionHeader)) {
-    if (_delegateImplementsReferenceSizeForHeader) {
+    if (_delegateFlags.implementsReferenceSizeForHeader) {
       return [[self delegateForCollectionView:collectionView] collectionView:collectionView layout:_layout referenceSizeForHeaderInSection:section];
     } else {
       return [self.layout headerReferenceSize];
     }
   } else if (ASObjectIsEqual(kind, UICollectionElementKindSectionFooter)) {
-    if (_delegateImplementsReferenceSizeForFooter) {
+    if (_delegateFlags.implementsReferenceSizeForFooter) {
       return [[self delegateForCollectionView:collectionView] collectionView:collectionView layout:_layout referenceSizeForFooterInSection:section];
     } else {
       return [self.layout footerReferenceSize];

AsyncDisplayKitTests/ASCollectionViewTests.m

@@ -128,7 +128,10 @@
 {
   UICollectionViewLayout *layout = [[UICollectionViewLayout alloc] init];
   ASCollectionView *collectionView = [[ASCollectionView alloc] initWithFrame:CGRectZero collectionViewLayout:layout];
-  XCTAssert(collectionView.layoutInspector == nil, @"should not set a layout delegate for custom layouts");
+  XCTAssert(collectionView.layoutInspector != nil, @"should automatically set a layout delegate for custom layouts");
+  XCTAssert([collectionView.layoutInspector isKindOfClass:[ASCollectionViewNullLayoutInspector class]], @"should have a null layout inspector by default if no layout inspector is given for a custom layout");
+  XCTAssertThrows([collectionView.layoutInspector collectionView:collectionView constrainedSizeForNodeAtIndexPath:[NSIndexPath indexPathForItem:0 inSection:0]], @"should throw an exception for <ASCollectionViewLayoutInspecting> methods");
+  XCTAssertThrows([collectionView.layoutInspector collectionView:collectionView supplementaryNodesOfKind:UICollectionElementKindSectionHeader inSection:0], @"should throw an exception for <ASCollectionViewLayoutInspecting> methods");
 }
 
 - (void)testThatRegisteringASupplementaryNodeStoresItForIntrospection