Skip to content

Create a log wrapper and better test GDLLogWriter #2190

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

Merged
merged 6 commits into from
Dec 15, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
2 changes: 2 additions & 0 deletions GoogleDataLogger.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ Shared library for iOS SDK data logging needs.
s.source_files = 'GoogleDataLogger/GoogleDataLogger/**/*'
s.public_header_files = 'GoogleDataLogger/GoogleDataLogger/Classes/Public/*.h'

s.dependency 'GoogleUtilities/Logger'

s.pod_target_xcconfig = {
'GCC_C_LANGUAGE_STANDARD' => 'c99'
}
Expand Down
7 changes: 6 additions & 1 deletion GoogleDataLogger/GoogleDataLogger/Classes/GDLLogEvent.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ typedef NS_ENUM(NSInteger, GDLLogQoS) {
/** The log map identifier, to allow backends to map the extension property to a proto. */
@property(readonly, nonatomic) NSString *logMapID;

/** The identifier for the backend this log will eventually be sent to. */
@property(readonly, nonatomic) NSInteger logTarget;

/** The log object itself, encapsulated in the transport of your choice, as long as it implements
* the GDLLogProto protocol. */
@property(nonatomic) id<GDLLogProto> extension;
Expand All @@ -60,9 +63,11 @@ typedef NS_ENUM(NSInteger, GDLLogQoS) {
/** Initializes an instance using the given logMapID.
*
* @param logMapID The log map identifier.
* @param logTarget The log's target identifier.
* @return An instance of this class.
*/
- (instancetype)initWithLogMapID:(NSString *)logMapID NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithLogMapID:(NSString *)logMapID
logTarget:(NSInteger)logTarget NS_DESIGNATED_INITIALIZER;

@end

Expand Down
4 changes: 3 additions & 1 deletion GoogleDataLogger/GoogleDataLogger/Classes/GDLLogEvent.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@

@implementation GDLLogEvent

- (instancetype)initWithLogMapID:(NSString *)logMapID {
- (instancetype)initWithLogMapID:(NSString *)logMapID logTarget:(NSInteger)logTarget {
NSAssert(logMapID.length > 0, @"Please give a valid log map ID");
NSAssert(logTarget > 0, @"A log target cannot be negative or 0");
self = [super init];
if (self) {
_logMapID = logMapID;
_logTarget = logTarget;
}
return self;
}
Expand Down
11 changes: 9 additions & 2 deletions GoogleDataLogger/GoogleDataLogger/Classes/GDLLogWriter.m
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#import <GoogleDataLogger/GDLLogTransformer.h>

#import "GDLConsoleLogger.h"
#import "GDLLogStorage.h"

@implementation GDLLogWriter
Expand Down Expand Up @@ -48,8 +49,14 @@ - (void)writeLog:(GDLLogEvent *)log
dispatch_async(_logWritingQueue, ^{
GDLLogEvent *transformedLog = log;
for (id<GDLLogTransformer> transformer in logTransformers) {
transformedLog = [transformer transform:transformedLog];
if (!transformedLog) {
if ([transformer respondsToSelector:@selector(transform:)]) {
transformedLog = [transformer transform:transformedLog];
if (!transformedLog) {
return;
}
} else {
GDLLogWarning(GDLMCWTransformerDoesntImplementTransform,
@"Transformer doesn't implement transform: %@", transformer);
return;
}
}
Expand Down
9 changes: 4 additions & 5 deletions GoogleDataLogger/GoogleDataLogger/Classes/GDLLogger.m
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#import "GDLLogger.h"

#import "GDLLogEvent.h"
#import "GDLLogWriter.h"

@interface GDLLogger ()

Expand Down Expand Up @@ -49,18 +50,16 @@ - (instancetype)initWithLogMapID:(NSString *)logMapID

- (void)logTelemetryEvent:(GDLLogEvent *)logEvent {
NSAssert(logEvent, @"You can't log a nil event");

// TODO(mikehaney24): Implement.
[[GDLLogWriter sharedInstance] writeLog:logEvent afterApplyingTransformers:_logTransformers];
}

- (void)logDataEvent:(GDLLogEvent *)logEvent {
NSAssert(logEvent, @"You can't log a nil event");

// TODO(mikehaney24): Implement.
[[GDLLogWriter sharedInstance] writeLog:logEvent afterApplyingTransformers:_logTransformers];
}

- (GDLLogEvent *)newEvent {
return [[GDLLogEvent alloc] initWithLogMapID:_logMapID];
return [[GDLLogEvent alloc] initWithLogMapID:_logMapID logTarget:_logTarget];
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
@class GDLLogEvent;

/** Defines the API that log transformers must adopt. */
@protocol GDLLogTransformer
@protocol GDLLogTransformer <NSObject>

@required

/** Transforms a log by applying some logic to it. Logs returned can be nil, for example, in
* instances where the log should be sampled.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright 2018 Google
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import "GULLogger.h"

/** The console logger prefix. */
static GULLoggerService kGDLConsoleLogger = @"[GoogleDataLogger]";

/** A list of message codes to print in the logger that help to correspond printed messages with
* code locations. */
typedef NS_ENUM(NSInteger, GDLMessageCode) {

/** For warning messages concerning transform: not being implemented by a log transformer. */
GDLMCWTransformerDoesntImplementTransform = 1
Copy link
Member

Choose a reason for hiding this comment

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

What does MCW stand for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MessageCodeWarning. I'm trying to keep these strings short. Any alternative suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

Probably fine with a comment.

};

/** */
FOUNDATION_EXTERN NSString *GDLMessageCodeEnumToString(GDLMessageCode code);

/** Logs the warningMessage string to the console at the warning level.
*
* @param warningMessageFormat The format string to log to the console.
*/
FOUNDATION_EXTERN void GDLLogWarning(GDLMessageCode messageCode,
NSString *warningMessageFormat,
...) NS_FORMAT_FUNCTION(2, 3);

// A define to wrap GULLogWarning with slightly more convenient usage.
#define GDLLogWarning(MESSAGE_CODE, MESSAGE_FORMAT, ...) \
GULLogWarning(kGDLConsoleLogger, YES, GDLMessageCodeEnumToString(MESSAGE_CODE), MESSAGE_FORMAT, \
__VA_ARGS__);
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright 2018 Google
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import "GDLConsoleLogger.h"

NSString* GDLMessageCodeEnumToString(GDLMessageCode code) {
return [[NSString alloc] initWithFormat:@"I-GDL%06ld", (long)code];
}
4 changes: 2 additions & 2 deletions GoogleDataLogger/Tests/GDLLogEventTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ @interface GDLLogEventTest : XCTestCase
@implementation GDLLogEventTest

- (void)testInit {
XCTAssertNotNil([[GDLLogEvent alloc] initWithLogMapID:@"1"]);
XCTAssertThrows([[GDLLogEvent alloc] initWithLogMapID:@""]);
XCTAssertNotNil([[GDLLogEvent alloc] initWithLogMapID:@"1" logTarget:1]);
XCTAssertThrows([[GDLLogEvent alloc] initWithLogMapID:@"" logTarget:1]);
}

@end
45 changes: 43 additions & 2 deletions GoogleDataLogger/Tests/GDLLogWriterTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,18 @@ - (GDLLogEvent *)transform:(GDLLogEvent *)logEvent {

@end

@interface GDLLogWriterTestNewLogTransformer : NSObject <GDLLogTransformer>

@end

@implementation GDLLogWriterTestNewLogTransformer

- (GDLLogEvent *)transform:(GDLLogEvent *)logEvent {
return [[GDLLogEvent alloc] initWithLogMapID:@"new" logTarget:1];
}

@end

@interface GDLLogWriterTest : XCTestCase

@end
Expand All @@ -50,18 +62,20 @@ - (void)testSharedInstance {
XCTAssertEqual([GDLLogWriter sharedInstance], [GDLLogWriter sharedInstance]);
}

/** Tests writing a log without a transformer. */
- (void)testWriteLogWithoutTransformers {
GDLLogWriter *writer = [GDLLogWriter sharedInstance];
GDLLogEvent *log = [[GDLLogEvent alloc] initWithLogMapID:@"1"];
GDLLogEvent *log = [[GDLLogEvent alloc] initWithLogMapID:@"1" logTarget:1];
XCTAssertNoThrow([writer writeLog:log afterApplyingTransformers:nil]);
dispatch_sync(writer.logWritingQueue, ^{
// TODO(mikehaney24): Assert that storage contains the log.
});
}

/** Tests writing a log with a transformer that nils out the log. */
- (void)testWriteLogWithTransformersThatNilTheLog {
GDLLogWriter *writer = [GDLLogWriter sharedInstance];
GDLLogEvent *log = [[GDLLogEvent alloc] initWithLogMapID:@"2"];
GDLLogEvent *log = [[GDLLogEvent alloc] initWithLogMapID:@"2" logTarget:1];
NSArray<id<GDLLogTransformer>> *transformers =
@[ [[GDLLogWriterTestNilingTransformer alloc] init] ];
XCTAssertNoThrow([writer writeLog:log afterApplyingTransformers:transformers]);
Expand All @@ -70,4 +84,31 @@ - (void)testWriteLogWithTransformersThatNilTheLog {
});
}

/** Tests writing a log with a transformer that creates a new log. */
- (void)testWriteLogWithTransformersThatCreateANewLog {
GDLLogWriter *writer = [GDLLogWriter sharedInstance];
GDLLogEvent *log = [[GDLLogEvent alloc] initWithLogMapID:@"2" logTarget:1];
NSArray<id<GDLLogTransformer>> *transformers =
@[ [[GDLLogWriterTestNewLogTransformer alloc] init] ];
XCTAssertNoThrow([writer writeLog:log afterApplyingTransformers:transformers]);
dispatch_sync(writer.logWritingQueue, ^{
// TODO(mikehaney24): Assert that storage contains the new log.
});
}

/** Tests that using a transformer without transform: implemented throws. */
- (void)testWriteLogWithBadTransformer {
GDLLogWriter *writer = [GDLLogWriter sharedInstance];
GDLLogEvent *log = [[GDLLogEvent alloc] initWithLogMapID:@"2" logTarget:1];
NSArray *transformers = @[ [[NSObject alloc] init] ];
@try {
dispatch_sync(writer.logWritingQueue, ^{
// TODO(mikehaney24): Assert that storage contains the new log.
[writer writeLog:log afterApplyingTransformers:transformers];
});
} @catch (NSException *exception) {
NSLog(@"");
}
}

@end