Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add storage behavior for property keys #210

Merged
merged 20 commits into from Mar 14, 2014
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 35 additions & 11 deletions Mantle/MTLModel.h
Expand Up @@ -8,6 +8,24 @@

#import <Foundation/Foundation.h>

#import "EXTRuntimeExtensions.h"

// Defines a property's storage behavior, which affects how it will be copied,
// compared, and persisted.
typedef enum : NSUInteger {
// This property is not included in -description, -hash, or anything else.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bleh, just remembered that TomDoc style would put this at the top of the enum.

MTLPropertyStorageNone,

// This property is included in one-off operations like -copy and
// -dictionaryValue but does not affect -isEqual: or -hash.
// It may disappear at any time.
MTLPropertyStorageTransitory,

// The property is included in serialization (like `NSCoding`) and equality,
// since it can be expected to stick around.
MTLPropertyStoragePermanent,
} MTLPropertyStorage;

// An abstract base class for model objects, using reflection to provide
// sensible default behaviors.
//
Expand Down Expand Up @@ -64,24 +82,30 @@
// `model` must be an instance of the receiver's class or a subclass thereof.
- (void)mergeValuesForKeysFromModel:(MTLModel *)model;

// The storage behavior of a given key.
//
// The default implementation returns MTLPropertyStorageTransitory for weak,
// unsafe_unretained or assign properties of type id, MTLPropertyStorageNone for
// properties that are readonly and not backed by an instance variable, and
// MTLPropertyStoragePermanent otherwise.
//
// Returns the storage behavior for a given key on the receiver.
+ (MTLPropertyStorage)storageBehaviorForPropertyWithKey:(NSString *)propertyKey attributes:(mtl_propertyAttributes *)attributes;

// Compares the receiver with another object for equality.
//
// The default implementation is equivalent to comparing both models'
// -dictionaryValue.
// The default implementation is equivalent to comparing all properties of both
// models for which +storageBehaviorForPropertyWithKey:attributes: returns
// MTLPropertyStoragePermanent.
//
// Note that this may lead to infinite loops if the receiver holds a circular
// reference to another MTLModel and both use the default behavior.
// It is recommended to override -isEqual: in this scenario.
// Returns YES if the two models are considered equal, NO otherwise.
- (BOOL)isEqual:(id)object;

// A string that describes the contents of the receiver.
//
// The default implementation is based on the receiver's class and its
// -dictionaryValue.
//
// Note that this may lead to infinite loops if the receiver holds a circular
// reference to another MTLModel and both use the default behavior.
// It is recommended to override -description in this scenario.
// The default implementation is based on the receiver's class and all its
// properties for which +storageBehaviorForPropertyWithKey:attributes: returns
// MTLPropertyStoragePermanent.
- (NSString *)description;

@end
Expand Down
127 changes: 104 additions & 23 deletions Mantle/MTLModel.m
Expand Up @@ -8,14 +8,21 @@

#import "NSError+MTLModelException.h"
#import "MTLModel.h"
#import "EXTRuntimeExtensions.h"
#import "EXTScope.h"
#import "MTLReflection.h"
#import <objc/runtime.h>

// Used to cache the reflection performed in +propertyKeys.
// Used to cache the reflection performed in +generateAndCachePropertyKeys.
static void *MTLModelCachedPropertyKeysKey = &MTLModelCachedPropertyKeysKey;

// Associated in +generateAndCachePropertyKeys with a set of all transitory
// property keys.
static void *MTLModelCachedTransitoryPropertyKeysKey = &MTLModelCachedTransitoryPropertyKeysKey;

// Associated in +generateAndCachePropertyKeys with a set of all permanent
// property keys.
static void *MTLModelCachedPermanentPropertyKeysKey = &MTLModelCachedPermanentPropertyKeysKey;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you document these?


// Validates a value for an object and sets it if necessary.
//
// obj - The object for which the value is being validated. This value
Expand Down Expand Up @@ -62,6 +69,18 @@ static BOOL MTLValidateAndSetValue(id obj, NSString *key, id value, BOOL forceUp

@interface MTLModel ()

// Inspects all properties of this class using
// +storageBehaviorForPropertyWithKey and caches the results.
+ (void)generateAndCachePropertyKeys;

// Returns a set of all property keys for which
// +storageBehaviorForPropertyWithKey returned MTLPropertyStorageTransitory.
+ (NSSet *)transitoryPropertyKeys;

// Returns a set of all property keys for which
// +storageBehaviorForPropertyWithKey returned MTLPropertyStoragePermanent.
+ (NSSet *)permanentPropertyKeys;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you document these methods?


// Enumerates all properties of the receiver's class hierarchy, starting at the
// receiver, and continuing up until (but not including) MTLModel.
//
Expand All @@ -75,6 +94,41 @@ @implementation MTLModel

#pragma mark Lifecycle

+ (void)generateAndCachePropertyKeys {
NSMutableSet *propertyKeys = [NSMutableSet set];
NSMutableSet *transitoryKeys = [NSMutableSet set];
NSMutableSet *permanentKeys = [NSMutableSet set];

[self enumeratePropertiesUsingBlock:^(objc_property_t property, BOOL *stop) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we allow propertyKeys to be overriden by subclasses we can't actually generate the three sets in one go.

mtl_propertyAttributes *attributes = mtl_copyPropertyAttributes(property);
@onExit {
free(attributes);
};

NSString *key = @(property_getName(property));
switch ([self storageBehaviorForPropertyWithKey:key attributes:attributes]) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't the property attributes struct be created within this method? It's not used anywhere else here.

My only concern was that we don't parse the attributes multiple times per property.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops

case MTLPropertyStorageNone:
break;

case MTLPropertyStorageTransitory:
[transitoryKeys addObject:key];
[propertyKeys addObject:key];
break;

case MTLPropertyStoragePermanent:
[permanentKeys addObject:key];
[propertyKeys addObject:key];
break;
}
}];

// It doesn't really matter if we replace another thread's work, since we do
// it atomically and the result should be the same.
objc_setAssociatedObject(self, MTLModelCachedPropertyKeysKey, propertyKeys, OBJC_ASSOCIATION_COPY);
objc_setAssociatedObject(self, MTLModelCachedTransitoryPropertyKeysKey, transitoryKeys, OBJC_ASSOCIATION_COPY);
objc_setAssociatedObject(self, MTLModelCachedPermanentPropertyKeysKey, permanentKeys, OBJC_ASSOCIATION_COPY);
}

+ (instancetype)modelWithDictionary:(NSDictionary *)dictionary error:(NSError **)error {
return [[self alloc] initWithDictionary:dictionary error:error];
}
Expand All @@ -93,7 +147,7 @@ - (instancetype)initWithDictionary:(NSDictionary *)dictionary error:(NSError **)
// a new object to be stored in this variable (and we don't want ARC to
// double-free or leak the old or new values).
__autoreleasing id value = [dictionary objectForKey:key];

if ([value isEqual:NSNull.null]) value = nil;

BOOL success = MTLValidateAndSetValue(self, key, value, YES, error);
Expand Down Expand Up @@ -128,32 +182,57 @@ + (void)enumeratePropertiesUsingBlock:(void (^)(objc_property_t property, BOOL *
}

+ (NSSet *)propertyKeys {
NSSet *cachedKeys = objc_getAssociatedObject(self, MTLModelCachedPropertyKeysKey);
if (cachedKeys != nil) return cachedKeys;
NSSet *propertyKeys = objc_getAssociatedObject(self, MTLModelCachedPropertyKeysKey);

NSMutableSet *keys = [NSMutableSet set];
if (propertyKeys == nil) {
[self generateAndCachePropertyKeys];
propertyKeys = objc_getAssociatedObject(self, MTLModelCachedPropertyKeysKey);
}

[self enumeratePropertiesUsingBlock:^(objc_property_t property, BOOL *stop) {
mtl_propertyAttributes *attributes = mtl_copyPropertyAttributes(property);
@onExit {
free(attributes);
};
return propertyKeys;
}

if (attributes->readonly && attributes->ivar == NULL) return;
+ (NSSet *)transitoryPropertyKeys {
NSSet *transitoryPropertyKeys = objc_getAssociatedObject(self, MTLModelCachedTransitoryPropertyKeysKey);

NSString *key = @(property_getName(property));
[keys addObject:key];
}];
if (transitoryPropertyKeys == nil) {
[self generateAndCachePropertyKeys];
transitoryPropertyKeys = objc_getAssociatedObject(self, MTLModelCachedTransitoryPropertyKeysKey);
}

// It doesn't really matter if we replace another thread's work, since we do
// it atomically and the result should be the same.
objc_setAssociatedObject(self, MTLModelCachedPropertyKeysKey, keys, OBJC_ASSOCIATION_COPY);
return transitoryPropertyKeys;
}

+ (NSSet *)permanentPropertyKeys {
NSSet *permanentPropertyKeys = objc_getAssociatedObject(self, MTLModelCachedPermanentPropertyKeysKey);

if (permanentPropertyKeys == nil) {
[self generateAndCachePropertyKeys];
permanentPropertyKeys = objc_getAssociatedObject(self, MTLModelCachedPermanentPropertyKeysKey);
}

return keys;
return permanentPropertyKeys;
}

- (NSDictionary *)dictionaryValue {
return [self dictionaryWithValuesForKeys:self.class.propertyKeys.allObjects];
NSSet *keys = [self.class.transitoryPropertyKeys setByAddingObjectsFromSet:self.class.permanentPropertyKeys];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for the future: it might be faster to enumerate these two sets and dump the results into an array directly.

But that's premature optimization right now, and could even lead to -valueForKey: being invoked more times than needed (if the same key appears twice).


return [self dictionaryWithValuesForKeys:keys.allObjects];
}

+ (MTLPropertyStorage)storageBehaviorForPropertyWithKey:(NSString *)propertyKey attributes:(mtl_propertyAttributes *)attributes {
if (attributes->readonly && attributes->ivar == NULL) {
return MTLPropertyStorageNone;
}

BOOL objectType = strcmp(attributes->type, @encode(id)) == 0;
BOOL assignPolicy = attributes->memoryManagementPolicy == mtl_propertyMemoryManagementPolicyAssign;

if (attributes->weak || (assignPolicy && objectType)) {
return MTLPropertyStorageTransitory;
}

return MTLPropertyStoragePermanent;
}

#pragma mark Merging
Expand Down Expand Up @@ -206,13 +285,15 @@ - (instancetype)copyWithZone:(NSZone *)zone {
#pragma mark NSObject

- (NSString *)description {
return [NSString stringWithFormat:@"<%@: %p> %@", self.class, self, self.dictionaryValue];
NSDictionary *permanentProperties = [self dictionaryWithValuesForKeys:self.class.permanentPropertyKeys.allObjects];

return [NSString stringWithFormat:@"<%@: %p> %@", self.class, self, permanentProperties];
}

- (NSUInteger)hash {
NSUInteger value = 0;

for (NSString *key in self.class.propertyKeys) {
for (NSString *key in self.class.permanentPropertyKeys) {
value ^= [[self valueForKey:key] hash];
}

Expand All @@ -223,7 +304,7 @@ - (BOOL)isEqual:(MTLModel *)model {
if (self == model) return YES;
if (![model isMemberOfClass:self.class]) return NO;

for (NSString *key in self.class.propertyKeys) {
for (NSString *key in self.class.permanentPropertyKeys) {
id selfValue = [self valueForKey:key];
id modelValue = [model valueForKey:key];

Expand Down
63 changes: 63 additions & 0 deletions MantleTests/MTLModelSpec.m
Expand Up @@ -6,6 +6,11 @@
// Copyright (c) 2012 GitHub. All rights reserved.
//

#import <objc/runtime.h>

#import "EXTRuntimeExtensions.h"
#import "EXTScope.h"

#import "MTLTestModel.h"

SpecBegin(MTLModel)
Expand Down Expand Up @@ -99,6 +104,13 @@
expect(copiedModel).to.equal(model);
expect(copiedModel).notTo.beIdenticalTo(model);
});

it(@"should not consider weak properties for equality", ^{
MTLTestModel *copiedModel = [model copy];
copiedModel.weakModel = nil;

expect(model).to.equal(copiedModel);
});
});

it(@"should fail to initialize if dictionary validation fails", ^{
Expand All @@ -124,4 +136,55 @@
expect(target.count).to.equal(8);
});

mtl_propertyAttributes *(^attributesForProperty)(Class, NSString *) = ^(Class class, NSString *propertyKey) {
objc_property_t property = class_getProperty(class, propertyKey.UTF8String);

return mtl_copyPropertyAttributes(property);
};

it(@"should consider primitive properties permanent", ^{
mtl_propertyAttributes *attributes = attributesForProperty(MTLStorageBehaviorModel.class, @"primitive");
@onExit {
free(attributes);
};

expect([MTLTestModel storageBehaviorForPropertyWithKey:@"primitive" attributes:attributes]).to.equal(MTLPropertyStoragePermanent);
});

it(@"should consider object-type assign properties transitory", ^{
mtl_propertyAttributes *attributes = attributesForProperty(MTLStorageBehaviorModel.class, @"assignProperty");
@onExit {
free(attributes);
};

expect([MTLTestModel storageBehaviorForPropertyWithKey:@"assignProperty" attributes:attributes]).to.equal(MTLPropertyStorageTransitory);
});

it(@"should consider object-type weak properties transitory", ^{
mtl_propertyAttributes *attributes = attributesForProperty(MTLStorageBehaviorModel.class, @"weakProperty");
@onExit {
free(attributes);
};

expect([MTLTestModel storageBehaviorForPropertyWithKey:@"weakProperty" attributes:attributes]).to.equal(MTLPropertyStorageTransitory);
});

pending(@"should consider object-type strong properties permanent", ^{
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why, but unless I make this property readwrite, the memory policy I get from the attributes is assign.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. It's probably because the strong is irrelevant for a property that only has a getter. What if you have a readwrite override in a class extension?

In any case, maybe we shouldn't consider assign to be transitory after all. :\

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can still call -setValue:forKey: so I the memory policy needs to be taken account somewhere, but yeah, readwrite in a class extension fixes it. back to only having weak properties transitory then, which I think is good enough since we're mostly dealing with cyclic references.

mtl_propertyAttributes *attributes = attributesForProperty(MTLStorageBehaviorModel.class, @"strongProperty");
@onExit {
free(attributes);
};

expect([MTLTestModel storageBehaviorForPropertyWithKey:@"strongProperty" attributes:attributes]).to.equal(MTLPropertyStoragePermanent);
});

it(@"should ignore readonly properties without backing ivar", ^{
mtl_propertyAttributes *attributes = attributesForProperty(MTLStorageBehaviorModel.class, @"notIvarBacked");
@onExit {
free(attributes);
};

expect([MTLTestModel storageBehaviorForPropertyWithKey:@"notIvarBacked" attributes:attributes]).to.equal(MTLPropertyStorageNone);
});

SpecEnd
12 changes: 12 additions & 0 deletions MantleTests/MTLTestModel.h
Expand Up @@ -64,3 +64,15 @@ extern const NSInteger MTLTestModelNameMissing;
@property (nonatomic, strong) NSURL *URL;

@end

@interface MTLStorageBehaviorModel : MTLModel

@property (readonly, nonatomic, assign) BOOL primitive;

@property (readonly, nonatomic, assign) id assignProperty;
@property (readonly, nonatomic, weak) id weakProperty;
@property (readonly, nonatomic, strong) id strongProperty;

@property (readonly, nonatomic, strong) id notIvarBacked;

@end
8 changes: 8 additions & 0 deletions MantleTests/MTLTestModel.m
Expand Up @@ -181,3 +181,11 @@ + (NSValueTransformer *)URLJSONTransformer {
}

@end

@implementation MTLStorageBehaviorModel

- (id)notIvarBacked {
return self;
}

@end