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

Add the callback of send operation. #458

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Add the callback of send operation. #458

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f1e1cee
Add the callback of send operation.
NathanLi Aug 14, 2016
c7b8f8d
Merge remote-tracking branch 'facebook/master'
NathanLi Nov 3, 2016
296c151
Rename SRSendCompleteBlock to SRSendCompletionBlock
NathanLi Nov 3, 2016
f599d0c
Fixed naming problems
NathanLi Nov 3, 2016
ac1c632
Change the call back key for uniqune.
NathanLi Nov 3, 2016
399e25b
Remove useless white space.
NathanLi Nov 3, 2016
00545e5
Remove useless white space.
NathanLi Nov 3, 2016
b313fe8
Improving the method chains and signatures for error:completion: styl…
NathanLi Nov 8, 2016
706623c
Use `[NSValue valueWithRange:dataRange]` as a key of _sendCallbacks.
NathanLi Nov 8, 2016
f630b95
Change the error message.
NathanLi Dec 23, 2016
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
24 changes: 24 additions & 0 deletions SocketRocket/SRWebSocket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ extern NSString *const SRHTTPResponseErrorKey;

@protocol SRWebSocketDelegate;

typedef void (^SRSendCompletionBlock)(NSError * _Nullable error);

///--------------------------------------
#pragma mark - SRWebSocket
///--------------------------------------
Expand Down Expand Up @@ -278,6 +280,17 @@ extern NSString *const SRHTTPResponseErrorKey;
*/
- (BOOL)sendString:(NSString *)string error:(NSError **)error NS_SWIFT_NAME(send(string:));

/**
Send a UTF-8 String to the server.

@param string String to send.
@param completion The call back of send result.
If an error occurs, this block will invoked with an `NSError` object containing information about the error, otherwise this block will be invoked with `nil`.

@return `YES` if the string was scheduled to send, otherwise - `NO`.
*/
- (BOOL)sendString:(NSString *)string completion:(nullable SRSendCompletionBlock)completion NS_SWIFT_NAME(send(string:completion:));

/**
Send binary data to the server.

Expand All @@ -302,6 +315,17 @@ extern NSString *const SRHTTPResponseErrorKey;
*/
- (BOOL)sendDataNoCopy:(nullable NSData *)data error:(NSError **)error NS_SWIFT_NAME(send(dataNoCopy:));

/**
Send binary data to the server, without making a defensive copy of it first.

@param data Data to send.
@param completion The call back of send result.
If an error occurs, this block will invoked with an `NSError` object containing information about the error, otherwise this block will be invoked with `nil`.

@return `YES` if the string was scheduled to send, otherwise - `NO`.
*/
- (BOOL)sendDataNoCopy:(nullable NSData *)data completion:(nullable SRSendCompletionBlock)completion NS_SWIFT_NAME(send(dataNoCopy:completion:));

/**
Send Ping message to the server with optional data.

Expand Down
132 changes: 121 additions & 11 deletions SocketRocket/SRWebSocket.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,31 @@
NSString *const SRWebSocketErrorDomain = @"SRWebSocketErrorDomain";
NSString *const SRHTTPResponseErrorKey = @"HTTPResponseStatusCode";

@interface SRDataCallback : NSObject
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you are constructing this one time only, let's ensure we can't mutate completion later.
You would need to replace the code block with this:

@interface SRDataCallback : NSObject

@property (nonatomic, assign) NSRange range;
@property (nonatomic, copy, readonly) SRSendCompletionBlock completion;

+ (instancetype)new NS_UNAVAILABLE;
- (instancetype)init NS_UNAVAILABLE;
- (instancetype)initWithRange:(NSRange)range 
                   completion:(SRSendCompletionBlock)completion NS_DESIGNATED_INITIALIZER;

@end

@implementation SRDataCallback

- (instancetype)initWithRange:(NSRange)range completion:(SRSendCompletionBlock)completion
{
  self = [super init];

  _range = range;
  _completion = [completion copy];

  return self;
}

@end

Copy link
Contributor

Choose a reason for hiding this comment

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

The above code block also contains the fixes for minor nits, like whitespace, naming as well as proper method attributes (NS_UNAVAILABLE to make sure no one can initialize this class with alloc] init] or new as well as attribute for designated initializer).

@property (nonatomic, assign) NSRange range;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Remove extra space aka make it look like NSRange range;

@property (nonatomic, copy, readonly) SRSendCompletionBlock completion;

+ (instancetype)new NS_UNAVAILABLE;
- (instancetype)init NS_UNAVAILABLE;
- (instancetype)initWithRange:(NSRange)range
completion:(SRSendCompletionBlock)completion NS_DESIGNATED_INITIALIZER;

@end

@implementation SRDataCallback

- (instancetype)initWithRange:(NSRange)range completion:(SRSendCompletionBlock)completion
{
self = [super init];

_range = range;
_completion = [completion copy];

return self;
}

@end

@interface SRWebSocket () <NSStreamDelegate>

@property (atomic, assign, readwrite) SRReadyState readyState;
Expand Down Expand Up @@ -138,6 +163,8 @@ @implementation SRWebSocket {

// proxy support
SRProxyConnect *_proxyConnect;

NSMutableDictionary<NSNumber *, SRDataCallback *> *_sendCallbacks;
}

@synthesize readyState = _readyState;
Expand Down Expand Up @@ -179,6 +206,8 @@ - (instancetype)initWithURLRequest:(NSURLRequest *)request protocols:(NSArray<NS
_consumerPool = [[SRIOConsumerPool alloc] init];

_scheduledRunloops = [[NSMutableSet alloc] init];

_sendCallbacks = [NSMutableDictionary dictionary];

return self;
}
Expand Down Expand Up @@ -548,6 +577,13 @@ - (void)_closeWithProtocolError:(NSString *)message;
- (void)_failWithError:(NSError *)error;
{
dispatch_async(_workQueue, ^{
[_sendCallbacks enumerateKeysAndObjectsUsingBlock:^(NSNumber * _Nonnull key, SRDataCallback * _Nonnull obj, BOOL * _Nonnull stop) {
if (obj.completion) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since completion is never nil - you don't need this check.

obj.completion(error);
}
}];
[_sendCallbacks removeAllObjects];

if (self.readyState != SR_CLOSED) {
_failed = YES;
[self.delegateController performDelegateBlock:^(id<SRWebSocketDelegate> _Nullable delegate, SRDelegateAvailableMethods availableMethods) {
Expand All @@ -566,14 +602,31 @@ - (void)_failWithError:(NSError *)error;
});
}

- (void)_writeData:(NSData *)data;
- (void)_writeData:(NSData *)data
{
[self assertOnWorkQueue];
[self _writeData:data completion:nil];
}

- (void)_writeData:(NSData *)data completion:(nullable SRSendCompletionBlock)completion
{
[self assertOnWorkQueue];

if (_closeWhenFinishedWriting) {
if (completion) {
completion(SRErrorWithCodeDescription(2134, @"socket is closed"));
}
return;
}


if (completion != nil) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Replace with if (completion) {

NSUInteger location = dispatch_data_get_size(_outputBuffer);
NSRange dataRange = NSMakeRange(location, data.length);
SRDataCallback *record = [[SRDataCallback alloc] initWithRange:dataRange completion:completion];

static NSUInteger keyCount = 0;
_sendCallbacks[@(keyCount++)] = record;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this is the best way to use a key here.
Can we use a [NSValue valueWithRange:dataRange] as a key?

}

__block NSData *strongData = data;
dispatch_data_t newData = dispatch_data_create(data.bytes, data.length, nil, ^{
strongData = nil;
Expand All @@ -597,18 +650,32 @@ - (void)send:(nullable id)message

- (BOOL)sendString:(NSString *)string error:(NSError **)error
{
return [self sendString:string error:error completion:nil];
}

- (BOOL)sendString:(NSString *)string completion:(nullable SRSendCompletionBlock)completion {
return [self sendString:string error:NULL completion:completion];
}

- (BOOL)sendString:(NSString *)string error:(NSError **)error completion:(nullable SRSendCompletionBlock)completion {
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think if we would remove error argument from this method and move it only to the method that needs it? Thus we can eliminate the need for weird method with synchronous and asynchronous signatures?

if (self.readyState != SR_OPEN) {
NSString *message = @"Invalid State: Cannot call `sendString:error:` until connection is open.";
NSError *locialError = SRErrorWithCodeDescription(2134, message);
if (error) {
*error = SRErrorWithCodeDescription(2134, message);
*error = locialError;
}

if (completion) {
completion(locialError);
}

SRDebugLog(message);
return NO;
}

string = [string copy];
dispatch_async(_workQueue, ^{
[self _sendFrameWithOpcode:SROpCodeTextFrame data:[string dataUsingEncoding:NSUTF8StringEncoding]];
[self _sendFrameWithOpcode:SROpCodeTextFrame data:[string dataUsingEncoding:NSUTF8StringEncoding] completion:completion];
});
return YES;
}
Expand All @@ -620,21 +687,36 @@ - (BOOL)sendData:(nullable NSData *)data error:(NSError **)error
}

- (BOOL)sendDataNoCopy:(nullable NSData *)data error:(NSError **)error
{
return [self sendDataNoCopy:data error:error completion:nil];
}

- (BOOL)sendDataNoCopy:(nullable NSData *)data completion:(nullable SRSendCompletionBlock)completion
{
return [self sendDataNoCopy:data error:NULL completion:completion];
}

- (BOOL)sendDataNoCopy:(nullable NSData *)data error:(NSError **)error completion:(SRSendCompletionBlock)completion
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as for sendString:error:completion:

{
if (self.readyState != SR_OPEN) {
NSString *message = @"Invalid State: Cannot call `sendDataNoCopy:error:` until connection is open.";
NSError *localError = SRErrorWithCodeDescription(2134, message);
if (error) {
*error = SRErrorWithCodeDescription(2134, message);
*error = localError;
}

if (completion) {
completion(localError);
}
SRDebugLog(message);
return NO;
}

dispatch_async(_workQueue, ^{
if (data) {
[self _sendFrameWithOpcode:SROpCodeBinaryFrame data:data];
[self _sendFrameWithOpcode:SROpCodeBinaryFrame data:data completion:completion];
} else {
[self _sendFrameWithOpcode:SROpCodeTextFrame data:nil];
[self _sendFrameWithOpcode:SROpCodeTextFrame data:nil completion:completion];
}
});
return YES;
Expand Down Expand Up @@ -1060,7 +1142,23 @@ - (void)_pumpWriting;

_outputBufferOffset += bytesWritten;

NSMutableArray<NSNumber *> *removeKeys = [NSMutableArray array];
[_sendCallbacks enumerateKeysAndObjectsUsingBlock:^(NSNumber * _Nonnull key, SRDataCallback * _Nonnull obj, BOOL * _Nonnull stop) {
if (NSMaxRange(obj.range) <= _outputBufferOffset) {
[removeKeys addObject:key];
obj.completion(nil);
}
}];

[_sendCallbacks removeObjectsForKeys:removeKeys];

if (_outputBufferOffset > SRDefaultBufferSize() && _outputBufferOffset > dataLength / 2) {
[_sendCallbacks enumerateKeysAndObjectsUsingBlock:^(NSNumber * _Nonnull key, SRDataCallback * _Nonnull obj, BOOL * _Nonnull stop) {
NSRange range = obj.range;
range.location -= _outputBufferOffset;
obj.range = range;
}];

_outputBuffer = dispatch_data_create_subrange(_outputBuffer, _outputBufferOffset, dataLength - _outputBufferOffset);
_outputBufferOffset = 0;
}
Expand Down Expand Up @@ -1320,19 +1418,26 @@ -(void)_pumpScanner;

static const size_t SRFrameHeaderOverhead = 32;

- (void)_sendFrameWithOpcode:(SROpCode)opCode data:(NSData *)data
- (void)_sendFrameWithOpcode:(SROpCode)opCode data:(NSData *)data completion:(SRSendCompletionBlock)completion
Copy link
Contributor

Choose a reason for hiding this comment

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

completion argument needs proper nullability annotation aka completion:(nullable SRSendCompletionBlock)completion

{
[self assertOnWorkQueue];

if (!data) {
if (completion) {
completion(nil);
}
return;
}

size_t payloadLength = data.length;

NSMutableData *frameData = [[NSMutableData alloc] initWithLength:payloadLength + SRFrameHeaderOverhead];
if (!frameData) {
[self closeWithCode:SRStatusCodeMessageTooBig reason:@"Message too big"];
NSString *reason = @"Message too big";
[self closeWithCode:SRStatusCodeMessageTooBig reason:reason];
if (completion) {
completion(SRErrorWithCodeDescription(SRStatusCodeMessageTooBig, reason));
}
return;
}
uint8_t *frameBuffer = (uint8_t *)frameData.mutableBytes;
Expand Down Expand Up @@ -1387,7 +1492,12 @@ - (void)_sendFrameWithOpcode:(SROpCode)opCode data:(NSData *)data
assert(frameBufferSize <= frameData.length);
frameData.length = frameBufferSize;

[self _writeData:frameData];
[self _writeData:frameData completion:completion];
}

- (void)_sendFrameWithOpcode:(SROpCode)opCode data:(NSData *)data
{
[self _sendFrameWithOpcode:opCode data:data completion:nil];
}

- (void)stream:(NSStream *)aStream handleEvent:(NSStreamEvent)eventCode
Expand Down