* Prevent race conditions in DatastoreTest
Previously it was possible for tasks on the async queue to outlive the
instance of the DatastoreTest which could crash or cause TSAN
violations.
This fixes a flake that appeared in the AuthOutlivesDatastore test case.
* Improve readability of the error classification tests
* Move FSTQueryCacheTests files to Firestore/core
* Rewrite QueryCache tests in C++
* Fix test flake in local store tests
This doesn't affect production because we always set the additional
references pointer there, but it's technically optional and some tests
don't do this.
* Fix field initialization order
* Reduce dependencies in objc_compatibility_apple_test
This makes it possible to build and test util without having to build
everything
* Add a lint check for CMake files
* Find XCTest, if available
* Fix CMakeLists.txt to include all sources
* Fix test flake in local store tests
This doesn't affect production because we always set the additional
references pointer there, but it's technically optional and some tests
don't do this.
There are more sources that can be `.cc` now (e.g., in API); these are left for a follow-up PR.
The dependency on Objective-C proto library cannot be removed just yet due to current lack of pretty-printing support for Nanopb.
After this PR is merged, the Objective-C serializers can be completely removed from the project. Also, this PR changes the `Message` class quite a bit; it's probably in its final form now.
I did some performance testing locally and came to the conclusion that replacing Objective-C serializers with their C++ versions brings no performance downgrade; actually, there seems to be a very slight improvement. I tested writing large batches and reading large collections:
* both are network-dominated; wall time is about twice longer than CPU time;
* serialization represents a pretty small percentage of the total CPU time;
* for writing, C++ serialization is a little more efficient, taking ~6% of total CPU time, compared to ~8% in the Objective-C version. I've seen similar results for reading.
All integration tests pass, under Address Sanitizer as well.
* Move FSTPersistenceHelpers to core/test/.../local
* Rewrite persistence_testing in C++
* Migrate from FSTPersistenceTestHelpers to C++ persistence_testing
* Rename .mm to .cc where possible in IndexManager tests
* Remove Objective-C gunk from index_manager_test.h
This came up while I was investigating the effect of new C++ serializers on performance. When writing large batches, one of the hottest functions turned out to be `firebase::firestore::util::operator==(const DocumentKey& lhs, const DocumentKey& rhs)`, accounting for 15-20% of total CPU time. There are two reasons why comparing keys is currently not very efficient:
* `BasePath::CompareTo`, to which this comparison eventually resolves, mistakenly uses the general-purpose `util::Compare` function rather than the more efficient `util::CompareContainer`;
* the default implementation of `Comparable::operator==` results in two comparisons per element ("less-than" followed by "greater-than"), even though one would be sufficient.
Fixing these issues results in 8-10% CPU time improvement when writing large batches.
To work around the latter, this PR defines a new mixin `InequalityComparable` that is exactly the same as `Comparable`, except it doesn't define `operator==`, forcing the inheriting class to implement it manually.
Some measurements (all done on the emulator in release mode with no logging) follow; the test case was to write 10 batches of 500 very small documents. Note that I found wall clock to fluctuate too much to be reliable and instead use the measurements from XCode's Time Profiler, which (presumably) shows CPU time. Each measurement shows total CPU time spent on `firebase::firestore::util::operator==` (all instantiations, not just for `DocumentKey`), followed by the percentage of time spent in this function compared to total runtime:
* no optimization:
* 275ms/18.8%;
* 245ms/16.9%;
* 252ms/18.5%.
* `CompareContainer` optimization:
* 124ms/9.7%;
* 110ms/8.5%;
* 128ms/10.0%.
* `CompareContainer` + `ComparableWithEqual` optimizations:
* 95ms/7.3%;
* 100ms/8.0%;
* 112ms/9.2%.
Replace all uses of `FSTSerializerBeta` in `remote_objc_bridge.{h,mm}` with the C++ `Serializer`. Also, make most of `remote/` sources pure C++ and remove no-longer-used methods in `FSTSerializerBeta`. All integration tests pass, under Address Sanitizer as well.
Unfortunately, pretty-printing is still delegated to Objective-C, so that `remote_objc_bridge.mm` still has to be an Objective-C++ source (and which prevents inlining `remote_objc_bridge` into the streams).
#no-changelog Will add a changelog entry when local serializer is ported, too.
* Rationalize input validation exceptions and assertions
... under a single ThrowHandler concept. This makes it possible to set
an alternative ThrowHandler in our C# bindings that properly translates
input validation exceptions to the correct .NET exception types.
* Rename api/input_validation.h to util/exception.h
* Reimplement util/hard_assert.h in terms of util/exception.h
* Rewrite SelfDocumentID as a property wrapper
Make wrapped types conform to DocumentReferenceConvertible and use that
protocol to implement the conversion.
* Change DocumentReferenceConvertible to DocumentIDWrappable
* Make ServerTimestampWrappable use static functions
Use the same form as DocumentIDWrappable.
* Update extension String: ServerTimestampWrappable
* Use #if compiler, not #if swift
The FirebaseFirestoreSwift module is declared as conforming to Swift
4.0 so #if swift is still false, even when compiling with Swift 5.1.
Using #if compiler will see the actual compiler version. Property
wrappers don't depend on runtime features, so this works.
Also, allow Wrappable protocols to throw during conversions.
These were discovered by running integration tests using the new `Serializer`.
Also, port all the missing tests from `FSTSerializerBetaTests.mm` (that I could find).
* Update versions for Release 6.10.0
* Cherrypick #3983 and include Database in release. (#3986)
* Remove log argument that causes SocketRocket crash (#3983)
* Add FirebaseDatabase to release.
* Update CHANGELOG with version.
* Cherrypick for #3984 along with CHANGELOG update. (#3988)
* Fix race condition in FIRComponentContainer instance creation (#3984)
* Fix race condition in FIRComponentContainer instance creation
Reported in #3967. Users sometimes seeing a crash when multiple threads
concurrently try to create Firestore instances. By inspection I found
that there's a race here where two threads can check for presence in the
cache, see nothing, proceed to creation, both store a value in the cache,
and both return their copies. This would lead to two Firestores being
created for a single App.
This replaces `dispatch_sync` on a serial queue with good old
`@synchronized` blocks. `dispatch_sync` will self-deadlock if called
from a thread already on the queue while `@synchronized` allows
recursion.
No new tests are added because several tests like
`testDependencyDoesntBlock` already cover this case.
* format
* Update Core CHANGELOG
* Update Core CHANGELOG again
* Fix merge from CHANGELOG
* Move ServerTimestamp into its own file
* Rewrite ServerTimestamp as a property wrapper
Add Swift4ServerTimestamp that behaves as before.
* Make ServerTimestampWrappable dictate timestamp conversion.
This makes the @ServerTimestamp property wrapper completely open to
extension by end users too.
* Drop dependency on OCMock
* Move Firestore onto Xcode 11 in Travis
* Special case simulators based on Xcode version, not product
This allows Firestore's build with Xcode 8.3.3 to continue to work.
* Make the Firestore Podfile install only one platform at a time
Use the PLATFORM environment variable to limit which targets are
modified. This makes the selected platform work but leaves the other
platforms in a broken state in the Xcode UI.
* Fix code signing errors on macOS builds
The as-of-yet-unreleased Clang 10 adds some new warnings which make the CMake build fail (due to `-Werror`):
* BoringSSL build now fails because some of its C code contains fallthroughs in switch statements that aren't properly annotated. This wasn't an issue before because previous versions of Clang ignored `-Wimplicit-fallthrough` in C mode;
* serializer test contains an expression that produces the following warning:
```
/src/firebase-ios-sdk/Firestore/core/test/firebase/firestore/remote/serializer_test.cc:446:29:
error: implicit conversion from 'std::__1::numeric_limits<long>::type' (aka 'long') to 'double'
changes value from 9223372036854775807 to 9223372036854775808 [-Werror,
-Wimplicit-int-float-conversion]
```
Fixes:
* upgrade the version of BoringSSL used to the latest master. There's no particular reason to choose this commit, other than it happens to be the latest. The actual fixes we need are [this](https://github.com/google/boringssl/commit/05cd93068b0a553afc48f69acbceae10c6a17593) and [this](https://github.com/google/boringssl/commit/fbebe833b180a1a0685803c59bba38916826dd2b);
* apply `static_cast` to the serializer test. Also, because the cast produces a different value, add a couple of additional values to test.
The breakage was discovered thanks to the fuzz build (which already uses Clang 10 from trunk).
* Rewrite FirestoreEncoderTests using assertThat
* Throw invalidValue for use of invalid top-level values
* Provide an message for not supported errors
Gil