Merge pull request #1285 from jcbertin/fix-message-equality

Fixed -isEqual: and -hash methods for DDLogMessage class.
CocoaLumberjack · Feb 11, 2022 · cff9d3e · cff9d3e
2 parents e2bf662 + 8afa55e
commit cff9d3e
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 9 deletions.
diff --git a/Sources/CocoaLumberjack/DDLog.m b/Sources/CocoaLumberjack/DDLog.m
@@ -1031,10 +1031,22 @@ - (instancetype)initWithMessage:(NSString *)message
     return self;
 }
 
+NS_INLINE BOOL _nullable_strings_equal(NSString* _Nullable lhs, NSString* _Nullable rhs)
+{
+    if (lhs == nil) {
+        if (rhs == nil)
+            return YES;
+    } else if (rhs != nil && [lhs isEqualToString:(NSString* _Nonnull)rhs])
+        return YES;
+    return NO;
+}
+
 - (BOOL)isEqual:(id)other {
+    // Subclasses of NSObject should not call [super isEqual:] here.
+    // See https://stackoverflow.com/questions/36593038/confused-about-the-default-isequal-and-hash-implements
     if (other == self) {
         return YES;
-    } else if (![super isEqual:other] || ![other isKindOfClass:[self class]]) {
+    } else if (!other || ![other isKindOfClass:[DDLogMessage class]]) {
         return NO;
     } else {
         __auto_type otherMsg = (DDLogMessage *)other;
@@ -1043,11 +1055,9 @@ - (BOOL)isEqual:(id)other {
         && otherMsg->_flag == _flag
         && otherMsg->_context == _context
         && [otherMsg->_file isEqualToString:_file]
-        && [otherMsg->_fileName isEqualToString:_fileName]
-        && [otherMsg->_function isEqualToString:_function]
+        && _nullable_strings_equal(otherMsg->_function, _function)
         && otherMsg->_line == _line
         && (([otherMsg->_representedObject respondsToSelector:@selector(isEqual:)] && [otherMsg->_representedObject isEqual:_representedObject]) || otherMsg->_representedObject == _representedObject)
-        && otherMsg->_options == _options
         && [otherMsg->_timestamp isEqualToDate:_timestamp]
         && [otherMsg->_threadID isEqualToString:_threadID] // If the thread ID is the same, the name will likely be the same as well.
         && [otherMsg->_queueLabel isEqualToString:_queueLabel]
@@ -1056,17 +1066,16 @@ - (BOOL)isEqual:(id)other {
 }
 
 - (NSUInteger)hash {
-    return [super hash]
-    ^ _message.hash
+    // Subclasses of NSObject should not call [super hash] here.
+    // See https://stackoverflow.com/questions/36593038/confused-about-the-default-isequal-and-hash-implements
+    return _message.hash
     ^ _level
     ^ _flag
     ^ _context
     ^ _file.hash
-    ^ _fileName.hash
     ^ _function.hash
     ^ _line
-    ^ ([_representedObject respondsToSelector:@selector(hash)] ? [_representedObject hash] : 0)
-    ^ _options
+    ^ ([_representedObject respondsToSelector:@selector(hash)] ? [_representedObject hash] : (NSUInteger)_representedObject)
     ^ _timestamp.hash
     ^ _threadID.hash
     ^ _queueLabel.hash

diff --git a/Tests/CocoaLumberjackTests/DDLogMessageTests.m b/Tests/CocoaLumberjackTests/DDLogMessageTests.m
@@ -68,6 +68,19 @@ + (DDLogMessage *)test_messageWithFunction:(NSString *)function
                                        timestamp:nil];
 }
 
++ (DDLogMessage *)test_messageWithWithoutFunction {
+    return [[DDLogMessage alloc] initWithMessage:kDefaultMessage
+                                           level:DDLogLevelDebug
+                                            flag:DDLogFlagError
+                                         context:1
+                                            file:@(__FILE__)
+                                        function:nil
+                                            line:__LINE__
+                                             tag:NULL
+                                         options:(DDLogMessageOptions)0
+                                       timestamp:nil];
+}
+
 + (DDLogMessage *)test_messageWithFile:(NSString *)file
                                options:(DDLogMessageOptions)options {
     return [[DDLogMessage alloc] initWithMessage:kDefaultMessage
@@ -236,6 +249,15 @@ - (void)testCopyWithZoneCreatesValidCopy {
     XCTAssertEqualObjects(self.message.threadName, copy.threadName);
     XCTAssertEqualObjects(self.message.queueLabel, copy.queueLabel);
     XCTAssertEqual(self.message.qos, copy.qos);
+    XCTAssertEqual(self.message.hash, copy.hash);
+    XCTAssertEqualObjects(self.message, copy);
+}
+
+- (void)testEqualityCopyWithoutFunction {
+    __auto_type message = [DDLogMessage test_messageWithWithoutFunction];
+    __auto_type copy = (typeof(message))[message copy];
+    XCTAssertEqual(message.hash, copy.hash);
+    XCTAssertEqualObjects(message, copy);
 }
 
 @end