Skip to content
This repository has been archived by the owner on Nov 22, 2024. It is now read-only.

Commit

Permalink
Use String as Id instead of integer
Browse files Browse the repository at this point in the history
Summary:
Using integers as Id was not a good idea. This is because the size of int on iOS is 64bits is 64bits unsigned long, where as the max safe int in a js is 9007199254740991

What happens whne you send  a large int than this is that the number is truncated.

Coupled to this NSString hash function is not randomly distributed and share a common prefix, this causes  us to get collisions in ids in bloks where share a common client id prefix.

To fix we send ids as strings, this allows us to represent numbers of any length at a small performance penalty

Reviewed By: lblasa

Differential Revision: D55692665

fbshipit-source-id: 2fc0725fbd14adb681a328ca9f5f119a7d088199
  • Loading branch information
Luke De Feo authored and facebook-github-bot committed Apr 4, 2024
1 parent 3782566 commit c5f250e
Show file tree
Hide file tree
Showing 20 changed files with 40 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ typedef void (^ReportAttributeEditorResult)(NSError* _Nullable);
+ (instancetype)attributeEditorWithDescriptorRegister:
(UIDDescriptorRegister*)descriptorRegister;

- (void)editNodeWithId:(NSNumber*)nodeId
- (void)editNodeWithId:(NSString*)nodeId
value:(id)value
metadataIdentifiers:(NSArray<UIDMetadataId>*)metadataIdentifiers
compoundTypeHint:(UIDCompoundTypeHint)compoundTypeHint
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ + (instancetype)attributeEditorWithDescriptorRegister:
initWithDescriptorRegister:descriptorRegister];
}

- (void)editNodeWithId:(NSNumber*)nodeId
- (void)editNodeWithId:(NSString*)nodeId
value:(id)value
metadataIdentifiers:(NSArray<UIDMetadataId>*)metadataIdentifiers
compoundTypeHint:(UIDCompoundTypeHint)compoundTypeHint
Expand All @@ -106,8 +106,7 @@ - (void)editNodeWithId:(NSNumber*)nodeId
return;
}

id<NSObject> node = [traversal findWithId:[nodeId unsignedIntegerValue]
inHierarchyWithRoot:root];
id<NSObject> node = [traversal findWithId:nodeId inHierarchyWithRoot:root];
if (!node) {
reportResult(
[NSError UID_errorWithType:AttributeEditorErrorTypeNodeNotFound]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ NS_ASSUME_NONNULL_BEGIN
@interface UIDFrameUpdate : NSObject<UIDUpdate>

@property(nonatomic) NSString* observerType;
@property(nonatomic) NSUInteger rootId;
@property(nonatomic) NSString* rootId;
@property(nonatomic) NSArray* nodes;
@property(nonatomic) NSTimeInterval timestamp;
@property(nonatomic) long traversalMS;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ NS_ASSUME_NONNULL_BEGIN
/**
A globally unique ID used to identify a node in the hierarchy.
*/
- (NSUInteger)UID_identifier;
- (NSString*)UID_identifier;

/**
The name used to identify this node in the inspector. Does not need to be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ NS_ASSUME_NONNULL_BEGIN
/**
A globally unique ID used to identify a node in the hierarchy.
*/
- (NSUInteger)identifierForNode:(T)node;
- (NSString*)identifierForNode:(T)node;

/**
The name used to identify this node in the inspector. Does not need to be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

@implementation UIDNodeDescriptor

- (NSUInteger)identifierForNode:(id)node {
return [node hash];
- (NSString*)identifierForNode:(id)node {
return [NSString stringWithFormat:@"%ld", [node hash]];
}

- (NSString*)nameForNode:(id)node {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@
FB_LINKABLE(UIView_UIDDescriptor)
@implementation UIView (UIDDescriptor)

- (NSUInteger)UID_identifier {
return [self hash];
- (NSString*)UID_identifier {
return [NSString stringWithFormat:@"%lu", [self hash]];
}

- (nonnull NSString*)UID_name {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ NS_ASSUME_NONNULL_BEGIN

@interface UIDSnapshotInfo : NSObject

@property(nonatomic) NSUInteger nodeId;
@property(nonatomic) NSString* nodeId;
@property(nonatomic, strong) UIImage* image;

@end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ NS_ASSUME_NONNULL_BEGIN

@interface UIDInitEvent : NSObject

@property(nonatomic) NSUInteger rootId;
@property(nonatomic) NSString* rootId;
@property(nonatomic) UIDTraversalMode currentTraversalMode;
@property(nonatomic, strong)
NSArray<UIDFrameworkEventMetadata*>* frameworkEventMetadata;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ typedef NSArray<NSString*>* UIDStacktrace;

@interface UIDFrameworkEvent : NSObject

@property(nonatomic) NSUInteger nodeIdentifier;
@property(nonatomic) NSString* nodeIdentifier;
@property(nonatomic, strong) NSString* type;
@property(nonatomic, strong) NSDate* timestamp;
@property(nonatomic, strong) UIDEventPayload payload;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@ NS_ASSUME_NONNULL_BEGIN

@interface UIDNode : NSObject

@property(nonatomic) NSUInteger identifier;
@property(nonatomic) NSString* identifier;
@property(nonatomic, strong) NSString* qualifiedName;
@property(nonatomic, strong) NSString* name;
@property(nonatomic, strong) UIDBounds* bounds;
@property(nonatomic, strong) NSSet<NSString*>* tags;
@property(nonatomic, strong) UIDAttributes* attributes;
@property(nonatomic, strong) UIDInlineAttributes* inlineAttributes;
@property(nonatomic, strong) UIDGenericAttributes* hiddenAttributes;
@property(nonatomic, nullable) NSNumber* parent;
@property(nonatomic, nullable) NSString* parent;
@property(nonatomic, strong) NSArray<NSNumber*>* children;
@property(nonatomic) NSNumber* activeChild;
@property(nonatomic) NSString* activeChild;

- (instancetype)initWithIdentifier:(NSUInteger)identifier
- (instancetype)initWithIdentifier:(NSString*)identifier
qualifiedName:(NSString*)qualifiedName
name:(NSString*)name
bounds:(UIDBounds*)bounds
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

@implementation UIDNode

- (instancetype)initWithIdentifier:(NSUInteger)identifier
- (instancetype)initWithIdentifier:(NSString*)identifier
qualifiedName:(NSString*)qualifiedName
name:(NSString*)name
bounds:(UIDBounds*)bounds
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ - (void)startWithContext:(UIDContext*)context {
[_context.connection
receive:@"editAttribute"
withBlock:^(NSDictionary* data, id<FlipperResponder> responder) {
NSNumber* nodeId = [data[@"nodeId"] isKindOfClass:NSNumber.class]
NSString* nodeId = [data[@"nodeId"] isKindOfClass:NSString.class]
? data[@"nodeId"]
: nil;
if (nodeId == nil) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ @implementation UIDSnapshotInfo (Foundation)

- (id)toFoundation {
return @{
@"nodeId" : [NSNumber numberWithInteger:self.nodeId],
@"nodeId" : self.nodeId,
@"data" : self.image ? [self.image toFoundation] : @"",
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ @implementation UIDFrameworkEvent (Foundation)

- (id)toFoundation {
NSMutableDictionary* data = [NSMutableDictionary dictionaryWithDictionary:@{
@"nodeId" : [NSNumber numberWithUnsignedInteger:self.nodeIdentifier],
@"nodeId" : self.nodeIdentifier,
@"type" : self.type,
@"timestamp" :
[NSNumber numberWithDouble:self.timestamp.timeIntervalSince1970],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ @implementation UIDInitEvent (Foundation)

- (id)toFoundation {
return @{
@"rootId" : [NSNumber numberWithUnsignedInteger:self.rootId],
@"rootId" : self.rootId,
@"frameworkEventMetadata" : self.frameworkEventMetadata
? [self.frameworkEventMetadata toFoundation]
: @[],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ @implementation UIDNode (Foundation)

- (id)toFoundation {
NSMutableDictionary* data = [NSMutableDictionary dictionaryWithDictionary:@{
@"id" : [NSNumber numberWithUnsignedInteger:self.identifier],
@"id" : self.identifier,
@"qualifiedName" : self.qualifiedName ?: @"",
@"name" : self.name,
@"bounds" : [self.bounds toFoundation],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,15 @@ - (instancetype)initWithDescriptorRegister:
_accessibilityLeafDescendantsWithOptions:options] mutableCopy];

UIDNode* rootNode = [self _uidNodeForNode:root];
NSInteger rootIdentifier = rootNode.identifier;
NSString* rootIdentifier = rootNode.identifier;

NSMutableArray<UIDNode*>* nodes = [NSMutableArray new];
NSMutableArray* childrenIds = [NSMutableArray new];
for (NSObject* node in allyNodes) {
UIDNode* uidNode = [self _uidNodeForNode:node];
uidNode.parent = @(rootIdentifier);
uidNode.parent = rootIdentifier;
[nodes addObject:uidNode];
[childrenIds
addObject:[NSNumber numberWithUnsignedInteger:uidNode.identifier]];
[childrenIds addObject:uidNode.identifier];
}
rootNode.children = childrenIds;
[nodes insertObject:rootNode atIndex:0];
Expand All @@ -90,7 +89,7 @@ - (instancetype)initWithDescriptorRegister:
- (UIDNode*)_uidNodeForNode:(NSObject*)node {
UIDNodeDescriptor* descriptor =
[_descriptorRegister descriptorForClass:[node class]];
NSUInteger nodeIdentifier = [descriptor identifierForNode:node];
NSString* nodeIdentifier = [descriptor identifierForNode:node];
UIDNode* uidNode = [[UIDNode alloc]
initWithIdentifier:nodeIdentifier
qualifiedName:[descriptor nameForNode:node]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ NS_ASSUME_NONNULL_BEGIN

- (NSArray<UIDNode*>*)traverse:(id)root;

- (id<NSObject>)findWithId:(NSUInteger)nodeId inHierarchyWithRoot:(id)root;
- (id<NSObject>)findWithId:(NSString*)nodeId inHierarchyWithRoot:(id)root;

@end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
@interface UIDTransientNode : NSObject

@property(nonatomic) id node;
@property(nonatomic, nullable) NSNumber* parent;
@property(nonatomic, nullable) NSString* parent;
@property(nonatomic) BOOL shallow;

- (instancetype)initWithNode:(id)node;
- (instancetype)initWithNode:(id)node parent:(NSNumber*)parent;
- (instancetype)initWithNode:(id)node parent:(NSString*)parent;

@end

Expand All @@ -27,7 +27,7 @@ - (instancetype)initWithNode:(id)node {
return [self initWithNode:node parent:nil];
}

- (instancetype)initWithNode:(id)node parent:(NSNumber*)parent {
- (instancetype)initWithNode:(id)node parent:(NSString*)parent {
if (self = [super init]) {
_node = node;
_parent = parent;
Expand Down Expand Up @@ -74,7 +74,7 @@ + (instancetype)createWithDescriptorRegister:
UIDNodeDescriptor* descriptor =
[self.descriptorRegister descriptorForClass:[node class]];

NSUInteger nodeIdentifier = [descriptor identifierForNode:node];
NSString* nodeIdentifier = [descriptor identifierForNode:node];

UIDNode* uidNode =
[[UIDNode alloc] initWithIdentifier:nodeIdentifier
Expand All @@ -96,27 +96,22 @@ + (instancetype)createWithDescriptorRegister:

NSArray* children = [descriptor childrenOfNode:node];
id<NSObject> activeChild = [descriptor activeChildForNode:node];
NSNumber* activeChildId = nil;
NSString* activeChildId = nil;
if (activeChild != nil) {
UIDNodeDescriptor* activeChildDescriptor =
[self.descriptorRegister descriptorForClass:[activeChild class]];
NSUInteger childId =
[activeChildDescriptor identifierForNode:activeChild];
activeChildId = [NSNumber numberWithUnsignedInteger:childId];
activeChildId = [activeChildDescriptor identifierForNode:activeChild];
}

NSMutableArray* childrenIds = [NSMutableArray new];
for (id child in children) {
UIDNodeDescriptor* childDescriptor =
[self.descriptorRegister descriptorForClass:[child class]];
assert(childDescriptor != nil);
[childrenIds
addObject:[NSNumber numberWithUnsignedInteger:
[childDescriptor identifierForNode:child]]];
[childrenIds addObject:[childDescriptor identifierForNode:child]];

UIDTransientNode* transientChildNode = [[UIDTransientNode alloc]
initWithNode:child
parent:[NSNumber numberWithUnsignedInteger:nodeIdentifier]];
UIDTransientNode* transientChildNode =
[[UIDTransientNode alloc] initWithNode:child parent:nodeIdentifier];

// This is a child which is not active, so mark it as to not
// traverse its children.
Expand All @@ -136,7 +131,7 @@ + (instancetype)createWithDescriptorRegister:
return nodes;
}

- (id<NSObject>)findWithId:(NSUInteger)nodeId inHierarchyWithRoot:(id)root {
- (id<NSObject>)findWithId:(NSString*)nodeId inHierarchyWithRoot:(id)root {
if (root == nil) {
return nil;
}
Expand All @@ -151,9 +146,9 @@ + (instancetype)createWithDescriptorRegister:
UIDNodeDescriptor* descriptor =
[self.descriptorRegister descriptorForClass:[node class]];

NSUInteger nodeIdentifier = [descriptor identifierForNode:node];
NSString* nodeIdentifier = [descriptor identifierForNode:node];

if (nodeIdentifier == nodeId) {
if ([nodeIdentifier isEqualToString:nodeId]) {
return node;
}

Expand Down

0 comments on commit c5f250e

Please sign in to comment.