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.

Sign up for GitHub

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

Update enumeration methods to allow a stop flag to be flipped by caller #204

Merged
merged 5 commits into from
Oct 4, 2017
Merged

Update enumeration methods to allow a stop flag to be flipped by caller #204

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
11ee57d
Update enumeration methods to allow a stop flag to be flipped by caller
mjlazar Oct 3, 2017
1c87c6a
Fix compiler warning
mjlazar Oct 3, 2017
a9c6320
Add CHANGELOG entry
mjlazar Oct 3, 2017
9c96f0c
Changes to address garrettmoon's comments
mjlazar Oct 4, 2017
578d98d
Merge branch 'master' into enumerate-objects-improvements
garrettmoon Oct 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

* Add your own contributions to the next release on the line below this with your name.
- [fix] Add some sane limits to the disk cache: [#201]https://github.com/pinterest/PINCache/pull/201
- [new] Update enumeration methods to allow a stop flag to be flipped by caller: [#204](https://github.com/pinterest/PINCache/pull/204)

## 3.0.1 -- Beta 5
- [fix] Respect small byteLimit settings by checking object size in setObject: [#198](https://github.com/pinterest/PINCache/pull/198)
Expand Down
6 changes: 6 additions & 0 deletions Source/PINCaching.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ typedef void (^PINCacheBlock)(id<PINCaching> cache);
*/
typedef void (^PINCacheObjectBlock)(id<PINCaching> cache, NSString *key, id _Nullable object);

/**
A callback block used for enumeration which provides the cache, key and object as arguments plus a stop flag that
may be flipped by the caller.
*/
typedef void (^PINCacheObjectEnumerationBlock)(id<PINCaching> cache, NSString *key, id _Nullable object, BOOL *stop);

/**
A callback block which provides a BOOL value as argument
*/
Expand Down
10 changes: 8 additions & 2 deletions Source/PINDiskCache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ typedef void (^PINDiskCacheObjectBlock)(PINDiskCache *cache, NSString *key, id <
*/
typedef void (^PINDiskCacheFileURLBlock)(NSString *key, NSURL * _Nullable fileURL);

/**
A callback block used for enumeration which provides the key and fileURL of the object plus a stop flag that
may be flipped by the caller.
*/
typedef void (^PINDiskCacheFileURLEnumerationBlock)(NSString *key, NSURL * _Nullable fileURL, BOOL *stop);

/**
A callback block which provides a BOOL value as argument
*/
Expand Down Expand Up @@ -399,7 +405,7 @@ PIN_SUBCLASSING_RESTRICTED
lock is held.

*/
- (void)enumerateObjectsWithBlockAsync:(PINDiskCacheFileURLBlock)block completionBlock:(nullable PINCacheBlock)completionBlock;
- (void)enumerateObjectsWithBlockAsync:(PINDiskCacheFileURLEnumerationBlock)block completionBlock:(nullable PINCacheBlock)completionBlock;

#pragma mark - Synchronous Methods
/// @name Synchronous Methods
Expand Down Expand Up @@ -480,7 +486,7 @@ PIN_SUBCLASSING_RESTRICTED
lock is held.

*/
- (void)enumerateObjectsWithBlock:(PIN_NOESCAPE PINDiskCacheFileURLBlock)block;
- (void)enumerateObjectsWithBlock:(PIN_NOESCAPE PINDiskCacheFileURLEnumerationBlock)block;

@end

Expand Down
13 changes: 9 additions & 4 deletions Source/PINDiskCache.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ - (void)removeAllObjectsAsync:(PINCacheBlock)block
} withPriority:PINOperationQueuePriorityLow];
}

- (void)enumerateObjectsWithBlockAsync:(PINDiskCacheFileURLBlock)block completionBlock:(PINCacheBlock)completionBlock
- (void)enumerateObjectsWithBlockAsync:(PINDiskCacheFileURLEnumerationBlock)block completionBlock:(PINCacheBlock)completionBlock
{
__weak PINDiskCache *weakSelf = self;

Expand Down Expand Up @@ -1093,7 +1093,7 @@ - (void)removeAllObjects
[self unlock];
}

- (void)enumerateObjectsWithBlock:(PINDiskCacheFileURLBlock)block
- (void)enumerateObjectsWithBlock:(PINDiskCacheFileURLEnumerationBlock)block
{
if (!block)
return;
Expand All @@ -1106,7 +1106,10 @@ - (void)enumerateObjectsWithBlock:(PINDiskCacheFileURLBlock)block
NSURL *fileURL = [self encodedFileURLForKey:key];
// If the cache should behave like a TTL cache, then only fetch the object if there's a valid ageLimit and the object is still alive
if (!self->_ttlCache || self->_ageLimit <= 0 || fabs([[_dates objectForKey:key] timeIntervalSinceDate:now]) < self->_ageLimit) {
block(key, fileURL);
BOOL stop;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you initialize this to NO (same comment as below).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

block(key, fileURL, &stop);
if (stop)
break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

super nit, but can you do 4 spaces here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would recommend creating a .clang-format file containing your specific style rules. It works great with objc, and you could probably integrate it as a presubmit check on your CI.

See https://clang.llvm.org/docs/ClangFormat.html.

}
}
[self unlock];
Expand Down Expand Up @@ -1444,7 +1447,9 @@ - (void)removeAllObjects:(nullable PINDiskCacheBlock)block

- (void)enumerateObjectsWithBlock:(PINDiskCacheFileURLBlock)block completionBlock:(nullable PINDiskCacheBlock)completionBlock
{
[self enumerateObjectsWithBlockAsync:block completionBlock:completionBlock];
[self enumerateObjectsWithBlockAsync:^(NSString * _Nonnull key, NSURL * _Nullable fileURL, BOOL * _Nonnull stop) {
block(key, fileURL);
} completionBlock:completionBlock];
}

@end
4 changes: 2 additions & 2 deletions Source/PINMemoryCache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ PIN_SUBCLASSING_RESTRICTED
@param block A block to be executed for every object in the cache.
@param completionBlock An optional block to be executed concurrently when the enumeration is complete.
*/
- (void)enumerateObjectsWithBlockAsync:(PINCacheObjectBlock)block completionBlock:(nullable PINCacheBlock)completionBlock;
- (void)enumerateObjectsWithBlockAsync:(PINCacheObjectEnumerationBlock)block completionBlock:(nullable PINCacheBlock)completionBlock;

#pragma mark - Synchronous Methods
/// @name Synchronous Methods
Expand Down Expand Up @@ -212,7 +212,7 @@ PIN_SUBCLASSING_RESTRICTED
Instead use the asynchronous version, <enumerateObjectsWithBlock:completionBlock:>.

*/
- (void)enumerateObjectsWithBlock:(PIN_NOESCAPE PINCacheObjectBlock)block;
- (void)enumerateObjectsWithBlock:(PIN_NOESCAPE PINCacheObjectEnumerationBlock)block;

@end

Expand Down
16 changes: 12 additions & 4 deletions Source/PINMemoryCache.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ - (void)removeAllObjectsAsync:(PINCacheBlock)block
} withPriority:PINOperationQueuePriorityHigh];
}

- (void)enumerateObjectsWithBlockAsync:(PINCacheObjectBlock)block completionBlock:(PINCacheBlock)completionBlock
- (void)enumerateObjectsWithBlockAsync:(PINCacheObjectEnumerationBlock)block completionBlock:(PINCacheBlock)completionBlock
{
__weak PINMemoryCache *weakSelf = self;

Expand Down Expand Up @@ -550,7 +550,7 @@ - (void)removeAllObjects

}

- (void)enumerateObjectsWithBlock:(PINCacheObjectBlock)block
- (void)enumerateObjectsWithBlock:(PINCacheObjectEnumerationBlock)block
{
if (!block)
return;
Expand All @@ -562,7 +562,10 @@ - (void)enumerateObjectsWithBlock:(PINCacheObjectBlock)block
for (NSString *key in keysSortedByDate) {
// If the cache should behave like a TTL cache, then only fetch the object if there's a valid ageLimit and the object is still alive
if (!self->_ttlCache || self->_ageLimit <= 0 || fabs([[_dates objectForKey:key] timeIntervalSinceDate:now]) < self->_ageLimit) {
block(self, key, _dictionary[key]);
BOOL stop;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is unsafe? If the block doesn't set the stop value this will be garbage, not NO? I think only ivars are automatically initialized to zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Value types are always initialized to zero when they're allocated on the stack. Happy to change this to make it more readable though.

block(self, key, _dictionary[key], &stop);
if (stop)
break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

4 species please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
}
[self unlock];
Expand Down Expand Up @@ -827,7 +830,12 @@ - (void)removeAllObjects:(nullable PINMemoryCacheBlock)block

- (void)enumerateObjectsWithBlock:(PINMemoryCacheObjectBlock)block completionBlock:(nullable PINMemoryCacheBlock)completionBlock
{
[self enumerateObjectsWithBlockAsync:block completionBlock:completionBlock];
[self enumerateObjectsWithBlockAsync:^(id<PINCaching> _Nonnull cache, NSString * _Nonnull key, id _Nullable object, BOOL * _Nonnull stop) {
if ([cache isKindOfClass:[PINMemoryCache class]]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

4 space indentation (sorry) 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

PINMemoryCache *memoryCache = (PINMemoryCache *)cache;
block(memoryCache, key, object);
}
} completionBlock:completionBlock];
}

@end
12 changes: 6 additions & 6 deletions Tests/PINCacheTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ - (void)testMemoryCacheEnumerationWithWarning
__block NSUInteger enumCount = 0;

self.cache.memoryCache.didReceiveMemoryWarningBlock = ^(PINMemoryCache *cache) {
[cache enumerateObjectsWithBlockAsync:^(PINMemoryCache *cache, NSString *key, id object) {
[cache enumerateObjectsWithBlockAsync:^(PINMemoryCache *cache, NSString *key, id object, BOOL *stop) {
@synchronized (self) {
enumCount++;
}
Expand Down Expand Up @@ -543,7 +543,7 @@ - (void)testDiskCacheEnumeration

__block NSUInteger enumCount = 0;

[self.cache.diskCache enumerateObjectsWithBlockAsync:^(NSString *key, NSURL *fileURL) {
[self.cache.diskCache enumerateObjectsWithBlockAsync:^(NSString *key, NSURL *fileURL, BOOL *stop) {
@synchronized (self) {
enumCount++;
}
Expand Down Expand Up @@ -759,14 +759,14 @@ - (void)_testTTLCacheObjectEnumeration {
// With the TTL cache enabled, we expect enumerating over the caches to yield 0 objects
NSUInteger expectedObjCount = 0;
__block NSUInteger objCount = 0;
[self.cache.diskCache enumerateObjectsWithBlock:^(NSString * _Nonnull key, NSURL * _Nullable fileURL) {
[self.cache.diskCache enumerateObjectsWithBlock:^(NSString * _Nonnull key, NSURL * _Nullable fileURL, BOOL *stop) {
objCount++;
}];

XCTAssertEqual(objCount, expectedObjCount, @"Expected %lu objects in the cache", (unsigned long)expectedObjCount);

objCount = 0;
[self.cache.memoryCache enumerateObjectsWithBlock:^(PINMemoryCache *cache, NSString *key, id _Nullable object) {
[self.cache.memoryCache enumerateObjectsWithBlock:^(PINMemoryCache *cache, NSString *key, id _Nullable object, BOOL *stop) {
objCount++;
}];

Expand All @@ -785,14 +785,14 @@ - (void)_testTTLCacheObjectEnumeration {
// With the TTL cache disabled, we expect enumerating over the caches to yield 1 object each, since the 2nd cache clearing hasn't happened yet
expectedObjCount = 1;
objCount = 0;
[self.cache.diskCache enumerateObjectsWithBlock:^(NSString * _Nonnull key, NSURL * _Nullable fileURL) {
[self.cache.diskCache enumerateObjectsWithBlock:^(NSString * _Nonnull key, NSURL * _Nullable fileURL, BOOL *stop) {
objCount++;
}];

XCTAssertEqual(objCount, expectedObjCount, @"Expected %lu objects in the cache", (unsigned long)expectedObjCount);

objCount = 0;
[self.cache.memoryCache enumerateObjectsWithBlock:^(PINMemoryCache * _Nonnull cache, NSString * _Nonnull key, id _Nullable object) {
[self.cache.memoryCache enumerateObjectsWithBlock:^(PINMemoryCache * _Nonnull cache, NSString * _Nonnull key, id _Nullable object, BOOL *stop) {
objCount++;
}];

Expand Down