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.

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 converters and port paths to FSTQuery #869

Merged
merged 4 commits into from
Mar 5, 2018
Merged

Conversation

zxu123
Copy link
Contributor

@zxu123 zxu123 commented Mar 1, 2018

This CL is part of the dissection of the grand RP#865.

  • Added temporary converters between FSTBarPath and BarPath, for Bar = {Field, Resource}. The converters are to be removed once all FSTBarPath is ported to BarPath.

  • Replace all parameter/return types of FSTBarPath to BarPath in FSTQuery, and change the rest of code accordingly by adding conversions.

  • The replacement is restricted to FSTQuery only in order to make the PR small. Eventually all FSTBarPath will be replaced. But I don't want the replacement leaked into other place (e.g. replacing parameter type to avoid conversion in methods not of FSTQuery) to avoid increase the complexity of this PR for review.

Discussion

Part of the C++ migration.

Testing

unit and integration tests pass.

API Changes

n/a

@zxu123 zxu123 requested a review from wilhuff March 1, 2018 16:52
@zxu123 zxu123 self-assigned this Mar 1, 2018
@zxu123 zxu123 requested a review from rsgowman March 1, 2018 16:52
@@ -64,7 +65,7 @@ typedef NS_ENUM(NSInteger, FSTRelationFilterOperator) {
* @param value A constant value to compare @a field to. The RHS of the expression.
* @return A new instance of FSTRelationFilter.
*/
+ (instancetype)filterWithField:(FSTFieldPath *)field
+ (instancetype)filterWithField:(firebase::firestore::model::FieldPath)field
Copy link
Member

@rsgowman rsgowman Mar 1, 2018

Choose a reason for hiding this comment

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

You're copying by value. Is that on purpose? (Naively, I would expect this to be a const&)

EDIT: Oops; didn't mean to send this out. Plus, I suspect this is on purpose and is likely fine. (More in my actual review.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, intended to combine (const&) and (&&) into a single API by value.

Copy link
Member

@rsgowman rsgowman left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -64,7 +65,7 @@ typedef NS_ENUM(NSInteger, FSTRelationFilterOperator) {
* @param value A constant value to compare @a field to. The RHS of the expression.
* @return A new instance of FSTRelationFilter.
*/
+ (instancetype)filterWithField:(FSTFieldPath *)field
+ (instancetype)filterWithField:(firebase::firestore::model::FieldPath)field
Copy link
Member

Choose a reason for hiding this comment

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

Note: You're copying by value here. I suspect that's on purpose, and you're relying on copy elision in the implementation (which is good.)

(If that's not on purpose, then please fix, but otherwise no action required.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

filterOperator:(FSTRelationFilterOperator)filterOperator
value:(FSTFieldValue *)value {
return [[FSTRelationFilter alloc] initWithField:field filterOperator:filterOperator value:value];
return [[FSTRelationFilter alloc] initWithField:std::move(field)
Copy link
Member

Choose a reason for hiding this comment

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

nit: The cpp primer recommends against std::move in this case since copy elision will take care of this for you. But I'm not sure how that interacts with obj-c? I don't think this is a big deal either way, so no action required by me.

Copy link
Contributor

Choose a reason for hiding this comment

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

No change required here, but yes, for future reference, copy elision would have handled this in the caller.

*
* @param fieldPath A C++ FieldPath.
*/
+ (instancetype)fromCPPFieldPath:(const firebase::firestore::model::FieldPath &)fieldPath;
Copy link
Member

Choose a reason for hiding this comment

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

nit: for future reference, fromCppFieldPath would probably better. But since these are temporary functions that will be removed shortly, I don't care. No action required.

"When abbreviations appear in such names, prefer to capitalize the abbreviations as single words (i.e. StartRpc(), not StartRPC())."

http://go/cpp-style#General_Naming_Rules

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack. Here actually use objc naming since it is objc method.

Copy link
Member

Choose a reason for hiding this comment

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

oh, good point. Carry on then. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

The initialism is spelled correctly for Objective-C, but the method name doesn't conform otherwise since it's supposed to start with the kind of thing it returns. In this case this should be

+ (instancetype)fieldPathWithCPPFieldPath(const firebase::firestore::model::FieldPath &)fieldPath;

This extreme verbosity is required in Objective-C because there's no name mangling (it's all C linkage). For future interop, I'd suggest making an inline ToObjc() method guarded by #if __OBJC__ on the C++ object instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack. BTW: I slightly prefer to keep the temporary conversion methods inside objc file. So it will be removed together with the FST* class once the port is done and objc files removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

SG. Not a big deal here, but I fear that in other cases these conversions may be relatively long-lived. When that's the case I think the extreme verbosity of the objc version harms readability enough to warrant opting for a more compact form, if possible.

For example: @gsoltis is rewriting FSTWriteGroup but we're not going to be able to break the dependency on GPBMessage until we're ready to cut over to the alternative nanopb implementation. The bridge will exist for a nontrivial amount of time.

Meanwhile, we're already going to have to audit all of the things this finds anyway:

find Firestore/core -type f | xargs grep __OBJC__

Shows a number of utilities and bridges we've already written for ourselves. I wouldn't worry too much that we're going to miss these things :-).

Again, because this one will be so short lived it really doesn't matter in this instance.

@zxu123 zxu123 assigned wilhuff and unassigned rsgowman Mar 1, 2018
Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

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

LGTM

Only one actionable thing in here (the test helpers) and I'm happy to take that in a follow-on PR.

@@ -55,30 +60,31 @@ @implementation FSTQueryTests
- (void)testConstructor {
FSTResourcePath *path =
[FSTResourcePath pathWithSegments:@[ @"rooms", @"Firestore", @"messages", @"0001" ]];
FSTQuery *query = [FSTQuery queryWithPath:path];
FSTQuery *query = [FSTQuery queryWithPath:[path toCPPResourcePath]];
Copy link
Contributor

Choose a reason for hiding this comment

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

For future reference (not in this PR since these are already obsolete) this can be handled more readably by adding an inline C++ constructor guarded by #if __OBJC__.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

ascending:NO]];
query = [query
queryByAddingSortOrder:[FSTSortOrder
sortOrderWithFieldPath:[FSTTestFieldPath(@"length") toCPPFieldPath]
Copy link
Contributor

Choose a reason for hiding this comment

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

In the end state of this C++ conversion all of these test helpers (FSTTestFieldPath here) should be ported too. All of these are too long in Objective-C, so it's worth looking at how this works in the Android or JS ports.

On Android a lot of these are grouped in TestUtil. This one is TestUtil.field(String). On JS this is just a free function named Field.

On Android we have static imports, and in JS we have ES6 imports both of which allow us to import field to look like a free function, keeping the testing code compact. In C++ there's no easy way to alias a function so we won't quite be able to make the equivalent.

I've already started created some unit testing utilities in a C++ testutil.

I suggest:

  • Create Firestore/core/test/firebase/firestore/testutil/testutil.h
  • Namespace firebase::firestore::testutil
  • Free functions just within there, probably with names equivalent to the js-sdk, not objc

Then create a namespace alias locally in the tests that use helpers. We could even make that helper shorter than testutil given all the usage in here. Maybe even namespace t = firebase::firestore::testutil? Dunno if that last part is a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sgtm done

filterOperator:(FSTRelationFilterOperator)filterOperator
value:(FSTFieldValue *)value {
return [[FSTRelationFilter alloc] initWithField:field filterOperator:filterOperator value:value];
return [[FSTRelationFilter alloc] initWithField:std::move(field)
Copy link
Contributor

Choose a reason for hiding this comment

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

No change required here, but yes, for future reference, copy elision would have handled this in the caller.

}

- (NSUInteger)hash {
return [self.field hash];
return _field.Hash();
Copy link
Contributor

Choose a reason for hiding this comment

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

For future reference: NSUInteger and size_t aren't guaranteed to be the same size (though I think on arm32 and arm64 they happen to be).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

return fieldValue != nil && [fieldValue isEqual:[FSTDoubleValue nanValue]];
}

- (NSString *)canonicalID {
return [NSString stringWithFormat:@"%@ IS NaN", [self.field canonicalString]];
return
[NSString stringWithFormat:@"%@ IS NaN", util::WrapNSStringNoCopy(_field.CanonicalString())];
Copy link
Contributor

Choose a reason for hiding this comment

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

This could also be

return [NSString stringWithFormat:@"%s IS NaN", _field.CanonicalString().c_str()];

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

*
* @param fieldPath A C++ FieldPath.
*/
+ (instancetype)fromCPPFieldPath:(const firebase::firestore::model::FieldPath &)fieldPath;
Copy link
Contributor

Choose a reason for hiding this comment

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

The initialism is spelled correctly for Objective-C, but the method name doesn't conform otherwise since it's supposed to start with the kind of thing it returns. In this case this should be

+ (instancetype)fieldPathWithCPPFieldPath(const firebase::firestore::model::FieldPath &)fieldPath;

This extreme verbosity is required in Objective-C because there's no name mangling (it's all C linkage). For future interop, I'd suggest making an inline ToObjc() method guarded by #if __OBJC__ on the C++ object instead.

Copy link
Contributor Author

@zxu123 zxu123 left a comment

Choose a reason for hiding this comment

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

updated

@@ -55,30 +60,31 @@ @implementation FSTQueryTests
- (void)testConstructor {
FSTResourcePath *path =
[FSTResourcePath pathWithSegments:@[ @"rooms", @"Firestore", @"messages", @"0001" ]];
FSTQuery *query = [FSTQuery queryWithPath:path];
FSTQuery *query = [FSTQuery queryWithPath:[path toCPPResourcePath]];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

ascending:NO]];
query = [query
queryByAddingSortOrder:[FSTSortOrder
sortOrderWithFieldPath:[FSTTestFieldPath(@"length") toCPPFieldPath]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sgtm done

*
* @param fieldPath A C++ FieldPath.
*/
+ (instancetype)fromCPPFieldPath:(const firebase::firestore::model::FieldPath &)fieldPath;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack. BTW: I slightly prefer to keep the temporary conversion methods inside objc file. So it will be removed together with the FST* class once the port is done and objc files removed.

Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -30,6 +30,10 @@ inline model::FieldPath Field(absl::string_view field) {
return model::FieldPath::FromServerFormat(field);
}

// Add a non-inline function to make this library buildable.
// TODO(zxu123): remove once there is non-inline function.
Copy link
Member

Choose a reason for hiding this comment

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

A quick note: cmake does support header only libraries, so we likely don't have to add this dummy function/file. However, if we expect this to not be a header only library in the long term, then adding a dummy function/file like you've done sounds like the correct approach.

@zxu123 zxu123 merged commit 1c40e7a into master Mar 5, 2018
Copy link
Contributor Author

@zxu123 zxu123 left a comment

Choose a reason for hiding this comment

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

to @rsgowman: testutil library will eventually contains non-inline functions. For now, adding dummy is easier for development; say no need to change header-only library to a normal library.

}

- (NSUInteger)hash {
return [self.field hash];
return _field.Hash();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

return fieldValue != nil && [fieldValue isEqual:[FSTDoubleValue nanValue]];
}

- (NSString *)canonicalID {
return [NSString stringWithFormat:@"%@ IS NaN", [self.field canonicalString]];
return
[NSString stringWithFormat:@"%@ IS NaN", util::WrapNSStringNoCopy(_field.CanonicalString())];
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

@zxu123 zxu123 deleted the cpp/port_respath branch March 5, 2018 18:24
zxu123 pushed a commit to zxu123/firebase-ios-sdk that referenced this pull request May 23, 2018
Import of firebase-ios-sdk from Github.

  - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com>
  - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman@google.com>
  - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman@google.com>
  - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com>
  - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman@google.com>
  - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman@google.com>
  - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu@google.com>
  - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman@google.com>
  - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg@google.com>
  - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien@google.com>
  - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien@google.com>
  - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu@google.com>
  - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com>
  - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu@google.com>
  - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman@google.com>
  - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang@google.com>
  - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman@google.com>
  - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu@google.com>
  - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman@google.com>

ORIGINAL_AUTHOR=Gil <wilhuff@github.com>
PiperOrigin-RevId: 188809445
zxu123 pushed a commit to zxu123/firebase-ios-sdk that referenced this pull request May 23, 2018
Import of firebase-ios-sdk from Github.

  - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com>
  - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman@google.com>
  - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman@google.com>
  - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com>
  - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman@google.com>
  - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman@google.com>
  - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu@google.com>
  - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman@google.com>
  - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg@google.com>
  - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien@google.com>
  - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien@google.com>
  - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu@google.com>
  - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com>
  - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu@google.com>
  - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman@google.com>
  - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang@google.com>
  - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman@google.com>
  - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu@google.com>
  - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman@google.com>

PiperOrigin-RevId: 188809445
zxu123 pushed a commit to zxu123/firebase-ios-sdk that referenced this pull request May 23, 2018
Import of firebase-ios-sdk from Github.

  - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com>
  - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman@google.com>
  - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman@google.com>
  - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com>
  - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman@google.com>
  - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman@google.com>
  - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu@google.com>
  - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman@google.com>
  - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg@google.com>
  - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien@google.com>
  - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien@google.com>
  - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu@google.com>
  - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com>
  - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu@google.com>
  - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman@google.com>
  - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang@google.com>
  - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman@google.com>
  - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu@google.com>
  - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman@google.com>

ORIGINAL_AUTHOR=Gil <wilhuff@github.com>
PiperOrigin-RevId: 188809445
zxu123 pushed a commit to zxu123/firebase-ios-sdk that referenced this pull request May 23, 2018
Import of firebase-ios-sdk from Github.

  - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com>
  - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman>
  - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman>
  - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com>
  - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman>
  - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman>
  - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu>
  - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman>
  - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg>
  - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien>
  - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien>
  - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu>
  - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com>
  - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu>
  - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman>
  - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang>
  - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman>
  - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu>
  - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman>

ORIGINAL_AUTHOR=Gil <wilhuff@github.com>
PiperOrigin-RevId: 188809445
zxu123 pushed a commit to zxu123/firebase-ios-sdk that referenced this pull request May 23, 2018
Import of firebase-ios-sdk from Github.

  - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com>
  - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman>
  - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman>
  - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com>
  - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman>
  - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman>
  - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu>
  - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman>
  - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg>
  - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien>
  - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien>
  - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu>
  - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com>
  - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu>
  - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman>
  - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang>
  - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman>
  - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu>
  - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman>

ORIGINAL_AUTHOR=Gil <wilhuff@github.com>
PiperOrigin-RevId: 188809445
zxu123 pushed a commit to zxu123/firebase-ios-sdk that referenced this pull request May 23, 2018
--
197713382 by zxu <zxu>:

Implement more on listener class and implement ListenerRegistration.

TESTED:
blaze test //sensored/testapp:testapp_build_test
blaze test //sensored/testapp:testapp_build_test --config=android_x86
blaze test //sensored/testapp:android_testapp_test --config=android_x86 --define firebase_build=stable
blaze test //sensored/cpp:firestore_kokoro
blaze test //sensored/cpp:listener_registration_test
--
196551381 by zxu <zxu>:

Implement more on listener class and implement the DocumentReference::AddSnapshotListener.

Also added a few test case and fixed a bug in DocumentReferenceInternal.

ListenerRegistration will be implemented in subsequent CL.

TESTED:
blaze test //sensored/testapp:testapp_build_test
blaze test //sensored/testapp:testapp_build_test --config=android_x86
blaze test //sensored/testapp:android_testapp_test --config=android_x86 --define firebase_build=stable
blaze test //sensored/cpp:firestore_kokoro
blaze test //sensored/cpp:firestore_test
--
196276752 by zxu <zxu>:

Implement the SnapshotMetadata with inline methods and (non-)Wrapper for Firestore C++.

TESTED:
blaze test //sensored/testapp:testapp_build_test
blaze test //sensored/testapp:testapp_build_test --config=android_x86
blaze test //sensored/testapp:android_testapp_test --config=android_x86 --experimental_one_version_enforcement=warning
blaze test //sensored/cpp:firestore_kokoro
blaze test //sensored/cpp:firestore_test
--
195841793 by zxu <zxu>:

Implement the wrapper class for callback (EventListener).

People unlike huge CL. So sending this short one for review. In subsequent CLs, I'll do:
*   cache/uncache or the CppEventListener class and the wiring of native method;
*   actually using this to implement DocumentReference::AddSnapshotListener;
*   update the testapp to run android_test with callback.
--
194112388 by zxu <zxu>:

Add Android-Wrapper for DocumentReference's non-callback methods.

Tested:
blaze test //sensored/testapp:testapp_build_test
blaze test //sensored/testapp:testapp_build_test --config=android_x86
blaze test //sensored/cpp:firestore_kokoro
blaze test //sensored/cpp:firestore_test
--
192445183 by zxu <zxu>:

Add Android-Wrapper for Firestore's remaining methods.
Tested:
blaze test //sensored/testapp:testapp_build_test
blaze test //sensored/testapp:testapp_build_test --config=android_x86
blaze test //sensored/cpp:firestore_kokoro
blaze test //sensored/cpp:firestore_test
--
190986604 by mcg <mcg>:

Manually import the public portion of

  - 22c226a C++ migration: make Timestamp class a part of public API ... by Konstantin Varlamov <var-const@users.noreply.github.com>

Note that this only imports the header, which otherewise won't come over with a
regular copybara run because we're currently excluding the public headers from
the Firestore import with cl/188810622.

The .cc implementation of this will come over with a regular copybara import,
coming shortly.
--
189013767 by zxu <zxu>:

Add Android-Wrapper for Firestore's method that does not involve other Firestore types.
Tested:
blaze test //sensored/testapp:testapp_build_test
blaze test //sensored/testapp:testapp_build_test --config=android_x86
blaze test //sensored/cpp:firestore_kokoro
--
188809445 by mcg <mcg>:

Import of firebase-ios-sdk from Github.

  - 07f4695 Updates Changelog for 4.5.0 release (firebase#908) by Zsika Phillip <protocol86@users.noreply.github.com>
  - 2b82a2e Allow serializing arbitrary length FieldValues (firebase#889) by rsgowman <rgowman>
  - 3e41f25 Fix test failures that occur during prod build (firebase#910) by rsgowman <rgowman>
  - f122cf7 Fix MutationQueue issue resulting in re-sending acknowled... by Michael Lehenbauer <mikelehen@gmail.com>
  - 7522314 Partial wrapping of pb_ostream_t (pt2) (firebase#903) by rsgowman <rgowman>
  - 0d3adb1 Partial wrapping of pb_ostream_t (pt1) (firebase#899) by rsgowman <rgowman>
  - e41f4b1 Merge Release 4.10.1 into Master (firebase#896) by zxu <zxu>
  - 2ae36f1 [De]Serialize FieldValue map_values ("Objects") (firebase#878) by rsgowman <rgowman>
  - 5930ad2 Factor out a universal build script (firebase#884) by Gil <mcg>
  - 8ef0f14 Adds Email link sign-in (firebase#882) by Paul Beusterien <paulbeusterien>
  - 0b8f216 Merge pull request firebase#880 from firebase/release-4.10.0 by Paul Beusterien <paulbeusterien>
  - 8311c64 port paths to FSTDocumentKey (firebase#877) by zxu <zxu>
  - 34ebf10 Add 10 second timeout waiting for connection before clien... by Michael Lehenbauer <mikelehen@gmail.com>
  - 1c40e7a add converters and port paths to FSTQuery (firebase#869) by zxu <zxu>
  - 9b5b4d8 Serialize (and deserialize) string values (firebase#864) by rsgowman <rgowman>
  - c6f73f7 Fix the issue completion handler is not triggered in subs... by Chen Liang <chliang>
  - 1ecf690 Add FieldValue.boolean_value() (firebase#862) by rsgowman <rgowman>
  - 3e7c062 replacing Auth by C++ auth implementation (firebase#802) by zxu <zxu>
  - 13aeb61 Eliminate TypedValue and serialize direct from FieldValue... by rsgowman <rgowman>
--
187049498 by mcg <mcg>:

Import of firebase-ios-sdk from Github.

  - f7d9f29 Fix lint warnings (firebase#840) by Gil <mcg>
  - b5c60ad Refactor [En|De]codeVarint to be symetric wrt tags (firebase#837) by rsgowman <rgowman>
  - ddc12c0 Merge pull request firebase#694 from morganchen12/auth-docs by Morgan Chen <morganchen12@gmail.com>
  - 442d46f Avoid warnings about failing to override a designated ini... by Gil <mcg>
  - 4dc63f8 Fix Firestore tests for M22 (firebase#834) by Gil <mcg>
  - 935f3ca  Avoid wrapping and rewrapping NSStrings when constructin... by Gil <mcg>
  - 6ce954a Serialize (and deserialize) int64 values (firebase#818) (firebase#829) by rsgowman <rgowman>
  - 41dcd4b Fix trivial mem leak in the test suite (firebase#828) by rsgowman <rgowman>
  - 50f9df9 Accept FIRTimestamp where NSDate is currently accepted as... by Konstantin Varlamov <var-const@users.noreply.github.com>
  - 67b068e Fix implicit retain self warnings (firebase#808) by rsgowman <rgowman>
  - 14ea068 Make FSTTimestamp into a public Firestore class (firebase#698) by Konstantin Varlamov <var-const@users.noreply.github.com>
  - de00de2 [En|De]codeUnsignedVarint -> [En|De]codeVarint (firebase#817) by rsgowman <rgowman>
  - 9bf73ab Fix two stream close issues (b/73167987, b/73382103). (firebase#8... by Michael Lehenbauer <mikelehen@gmail.com>
  - 7a4a2ea replacing FSTGetTokenResult by C++ Token implementation (... by zxu <zxu>
  - a9f3f35 Delete stale Firestore instances after FIRApp is deleted.... by Ryan Wilson <wilsonryan>
  - aa6f1ae Disable -Wrange-loop-analysis for abseil (firebase#807) by rsgowman <rgowman>
  - ef55eef Require official 1.4.0 for min CocoaPods version (firebase#806) by Paul Beusterien <paulbeusterien>
  - 7cddb97 Serialize (and deserialize) bool values (firebase#791) by rsgowman <rgowman>
  - 7a97f6c Enable -Wcomma for our build; disable it for abseil. (firebase#799) by rsgowman <rgowman>
  - 81ee594 DispatchQueue delayed callback improvements + testing (firebase#7... by Michael Lehenbauer <mikelehen@gmail.com>
  - fd9fd27 replacing Auth/FSTUser by C++ auth implementation (firebase#804) by zxu <zxu>
  - 6889850 Merge pull request firebase#801 from firebase/release-4.9.0 by Paul Beusterien <paulbeusterien>
  - e4be254 Fixed capitalization of init in test function. (firebase#797) by Ryan Wilson <wilsonryan>
  - 933be73 Improve documentation on auto-init property in FCM. (firebase#792) by Chen Liang <chliang>
  - 0f3c24b Actually ignore events on inactive streams, rather than j... by Greg Soltis <gsoltis>
  - fe19fca Serialize and deserialize null (firebase#783) by rsgowman <rgowman>
  - 95d0411 fix flaky test (firebase#788) by zxu <zxu>
  - b6613bd Cleaning up implicit retain for the RTDB and Storage by Sebastian Schmidt <mrschmidt>
  - 5f5f808 Keep track of number of queries in the query cache (firebase#776) by Greg Soltis <gsoltis>
  - c7c51a7 Fix double parsing on 32 bit devices. (firebase#787) by Ryan Wilson <wilsonryan>
  - 9504582 Port Firestore Document to C++ (firebase#777) by zxu <zxu>
  - adf9fb3 Update FieldValue of type Reference (firebase#775) by zxu <zxu>
  - 4afcfb1 Move to explicit nil check. (firebase#782) by Ryan Wilson <wilsonryan>
  - fd83e07 Strawman C++ API (firebase#763) by rsgowman <rgowman>
  - 8003348 C++ port: add C++ equivalent of FSTDocumentKey. (firebase#762) by Konstantin Varlamov <var-const@users.noreply.github.com>
  - 612d99c Fix Core CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF warnings (#... by Paul Beusterien <paulbeusterien>
  - bf45a15 port Firestore SnapshotVersion in C++ (firebase#767) by zxu <zxu>
  - d70c23e port Firestore Auth module in C++ (firebase#733) by zxu <zxu>
  - 633eb7b Let Travis run for `CMake` test and `lint.sh` (firebase#769) by zxu <zxu>
  - 274fe52 cmake build fixes (firebase#770) by rsgowman <rgowman>
--
184568931 by mcg <mcg>:

Import of firebase-ios-sdk from Github.

  - 32266c5 Make sure Firestore/core/include is in the podspec (firebase#748) by Gil <mcg>
  - 070c0ea Merge pull request firebase#746 from morganchen12/unguarded-block by Morgan Chen <morganchen12@gmail.com>
  - 13f572e Increase expectation timeout to 25 seconds. (firebase#744) by Michael Lehenbauer <mikelehen@gmail.com>
  - 47d81fd fix (firebase#739) by Chen Liang <chliang>
  - 515625c Remove predecessorKey,Object,Document, etc (firebase#735) by Gil <mcg>
  - ac30522 Start on ArraySortedMap in C++ (firebase#721) by Gil <mcg>
  - 729b8d1 Move all Firestore Objective-C to Objective-C++ (firebase#734) by Gil <mcg>
  - 693d064 Merge pull request firebase#732 from OleksiyA/FCM-subscribe-compl... by Oleksiy Ivanov <oleksiy@informationrd.com>
  - 3cbdbf2 Schema migrations for LevelDB (firebase#728) by Greg Soltis <gsoltis>
  - 6474a82 Firestore DatabaseId in C++ (firebase#727) by zxu <zxu>
  - 05ef5ae Add absl_strings to firebase_firestore_util_test dependen... by rsgowman <rgowman>
  - 03a0069 Add changelog entry for my last PR (oops) and also add a ... by Michael Lehenbauer <mikelehen@gmail.com>
  - 5a280ca  Import iterator_adaptors from google3 (firebase#718) by Gil <mcg>
  - 63a380b Use fixed-sized types (firebase#719) by Gil <mcg>
  - 6dc1373 Version updates to 4.8.2 (firebase#722) by Paul Beusterien <paulbeusterien>
  - a74d39f Fix a number of c++ build errors (firebase#715) by rsgowman <rgowman>
  - cceeec3 travis: check for copyright in sources (firebase#717) by Paul Beusterien <paulbeusterien>
  - 4d7c973 Fix b/72502745: OnlineState changes cause limbo document ... by Michael Lehenbauer <mikelehen@gmail.com>
  - af6976a normalize and port the rest of Firebase/Port code (firebase#713) by zxu <zxu>
  - 7226b4a port TargetIdGenerator to iOS (firebase#709) by zxu <zxu>
  - 5306474 adding unit test for auto init enable function (firebase#710) by Chen Liang <chliang>
  - 821fb90 implement `TargetIdGenerator` in C++ for Firestore (firebase#701) by zxu <zxu>
  - 5fdda3f normalize string_util (firebase#708) by zxu <zxu>
  - 15a2926 Update CHANGELOG for M21.1 release (firebase#679) by Ryan Wilson <wilsonryan>
  - f027214 Fix incorrect deprecation message (firebase#688) by Iulian Onofrei <6d0847b9@opayq.com>
  - bfa0e40 Implement the rest of FieldValue types for C++ (firebase#687) by zxu <zxu>
  - 1f77212 Properly publish Abseil sources as a part of the podspec ... by Gil <mcg>

ORIGINAL_AUTHOR=Firebase <firebase-noreply>
PiperOrigin-RevId: 197713382
minafarid pushed a commit to minafarid/firebase-ios-sdk that referenced this pull request Jun 6, 2018
* add converters and fix FSTQuery.{h,m} only

* address changes

* a change forget to address

* add a dummy function to make inline-only-library buildable
@firebase firebase locked and limited conversation to collaborators Nov 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants