Skip to content

C++: replace FSTMaybeDocumentDictionary with a C++ equivalent #2139

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 17 commits into from
Dec 10, 2018

Conversation

var-const
Copy link
Contributor

Also eliminate most usages of FSTDocumentKey, remove most methods from the Objective-C class and make it just a wrapper over DocumentKey. The only usage that cannot be directly replaced by C++ DocumentKey is in FSTFieldValue.

@@ -158,6 +161,22 @@ - (FSTDocument *)setTestDocumentAtPath:(const absl::string_view)path {
return doc;
}

- (void)expectMap:(const MaybeDocumentMap &)map
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This helper function will be used more in a follow-up PR.

Copy link
Member

Choose a reason for hiding this comment

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

Nice; though imo, the usage here is already sufficient to justify it's existence.

* @param expectedArray A sorted array of keys that actualSet must be equal to (after converting
* to an array and sorting).
*/
#define FSTAssertEqualSets(actualSet, expectedArray) \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was unused.

- (const DocumentKey &)key {
return _delegate;
}

- (BOOL)isEqual:(id)object {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method is still useful because it's used a lot (indirectly) in tests.

@@ -51,18 +51,14 @@ class DocumentKey {
explicit DocumentKey(ResourcePath&& path);

#if defined(__OBJC__)
DocumentKey(FSTDocumentKey* key) // NOLINT(runtime/explicit)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer necessary.

@@ -163,7 +166,7 @@ NS_ASSUME_NONNULL_BEGIN
- (void)releaseQuery:(FSTQuery *)query;

/** Runs @a query against all the documents in the local store and returns the results. */
- (FSTDocumentDictionary *)executeQuery:(FSTQuery *)query;
- (firebase::firestore::model::MaybeDocumentMap)executeQuery:(FSTQuery *)query;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is one notable change: returning a map of FSTMaybeDocuments instead of FSTDocuments. It's because unlike Objective-C, SortedMap<K, Derived*> cannot be converted to SortedMap<K, Base*>, and I can't think of a good workaround. While returning FSTDocuments could potentially be more efficient by saving a cast, in practice I don't think this is ever used -- all the maps of documents are eventually passed to FSTView computeChangesWithDocuments, which takes a map of maybe documents.

Copy link
Member

Choose a reason for hiding this comment

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

You could add a conversion function. It'd be O(n), but presuming you're going to do something with (all) the documents, you're (at least) O(n) anyways.

As it currently stands, this PR assumes elsewhere that these MaybeDocs are actual Docs. The assumption's currently correct, but feels a bit dangerous. If I'm the next one to touch this executeQuery function, I'd happily add the ability to return other subclasses of MaybeDoc because the API says that's fine. You could fix that with documentation, but then I'd have to actually read it. :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to impose the performance penalty from building a tree of shared pointers (in the implementation of SortedMap) for potentially thousands of documents, especially considering that this PR is partially motivated by optimization.

On one hand, I'm not sure it makes sense for executeQuery to return FSTDeletedDocuments or FSTUnknownDocuments. On the other hand, I totally agree that this would be best reflected in the return type.

Most of the trouble comes from functions which accept a map of FSTMaybeDocuments. Passing iterators doesn't work, because the functions may want to use more map functionality than just iterating over it. One workaround would be to make those functions template functions. The other is to have two overloads, one taking MaybeDocumentMap and the other DocumentMap (in Objective-C I presume they would have to have different names), and try to come up with a mostly-shared implementation. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

impose the performance penalty from building a tree of shared pointers

I had a feeling you'd say that. :) (Also, I forgot this was using a tree.)

A template function feels more natural to me? (Not sure how that interacts with obj-c though. I suppose since we're technically using objc++, it should be fine?) The overload would work too, though seems less good to me. I'd probably still prefer it over passing the "wrong" type and casting everywhere though.

Another (insane; don't do this) option: Use void*. At least then, we'd know that the type is "lying" and would read the docs every time. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, so I took a stab at it. AFAICT, Objective-C supports neither overloading nor templates, so what I ended up doing is:

  • have two versions of both computeView functions, one taking a DocumentMap and the other taking a MaybeDocumentMap;
  • in the implementation file, have both of them delegate to a template function;
  • the only difference in the template function is when accessing values from the map. I added an overloaded function which either casts the given FSTMaybeDocument to an FSTDocument, or just returns the given FSTDocument. In theory, this is more efficient because when dealing with a DocumentMap, it avoids casting and type checking altogether.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I like it. If you do too, then lets proceed with this version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I certainly think it's an improvement, thanks for raising this up.

@zxu123 zxu123 removed their assignment Dec 3, 2018
@@ -158,6 +161,22 @@ - (FSTDocument *)setTestDocumentAtPath:(const absl::string_view)path {
return doc;
}

- (void)expectMap:(const MaybeDocumentMap &)map
Copy link
Member

Choose a reason for hiding this comment

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

Nice; though imo, the usage here is already sufficient to justify it's existence.

@@ -158,6 +161,22 @@ - (FSTDocument *)setTestDocumentAtPath:(const absl::string_view)path {
return doc;
}

- (void)expectMap:(const MaybeDocumentMap &)map
Copy link
Member

Choose a reason for hiding this comment

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

Consider avoiding the word "map" to ensure no one confuses it with std::map.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to change the name MaybeDocumentMap because it's used on Android and in Web client. In this function, I feel using a different name for it would be inconsistent. The argument type should prevent any confusion with std::map.

Copy link
Member

Choose a reason for hiding this comment

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

sgtm.

FSTDocument *actual = nil;
auto found = map.find(doc.key);
if (found != map.end()) {
actual = static_cast<FSTDocument *>(found->second);
Copy link
Member

Choose a reason for hiding this comment

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

Is this safe? From this method's API, we don't seem to have any guarantee that this is an actual FSTDocument*. If this needs to be downcast, then in other locations, we've added a type() attribute and checked (or asserted) that first.

Also, would XCTAssertEqualObjects() work if the pointer types were slightly different, though still in the same inheritance hierarchy? i.e. could we do this:

FSTMaybeDocument* actual = found->second
XCTAssertEqualObjects(actual, doc);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, would XCTAssertEqualObjects() work if the pointer types were slightly different, though still in the same inheritance hierarchy?

Yes, this works, done.

/**
* Convenience type for a map of keys to Documents, since they are so common.
*/
using DocumentMap = immutable::SortedMap<DocumentKey, FSTDocument*>;
Copy link
Member

Choose a reason for hiding this comment

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

I like ImmutableDocumentMap better. Otherwise, I assume 'DocumentMap' means std::map<DocumentKey, Document> or similar. (Same for MaybeDocumentMap above.) But it is a little wordy.

Optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be inconsistent with other platforms (in particular, on Android the standard library class is also called map). Also, the immutable map is used much more often than the standard library one.

Copy link
Member

Choose a reason for hiding this comment

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

I didn't understand (the first half of) your comment? To reclarify my original in case that's where the misunderstanding started:

To me; 'map' implies 'mutable map' (cause that's the default in c++/java/etc, i.e. std::map, java.util.Map). Therefore XMap implies a mutable map of X's.

Also, the immutable map is used much more often than the standard library one.

But this is a fair point too; you can easily make the argument that we generally expect all types to be immutable, and that should be the default assumption. Which is fine by me.

Still optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't understand (the first half of) your comment?

Sorry the comment wasn't clear. I wanted to say that the same problem exists on Android: the standard library Map class is mutable, but DocumentMap is immutable, so there exists a potential for confusion.

I guess I phrased it better in the other PR: I think these classes can be seen as Firestore vocabulary types, and are used widely enough to make memorizing their interface reasonable.

MaybeDocumentMap unfiltered = results;
for (const auto &kv : unfiltered) {
const DocumentKey &key = kv.first;
FSTDocument *doc = static_cast<FSTDocument *>(kv.second);
Copy link
Member

Choose a reason for hiding this comment

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

(see previous comment)

@@ -163,7 +166,7 @@ NS_ASSUME_NONNULL_BEGIN
- (void)releaseQuery:(FSTQuery *)query;

/** Runs @a query against all the documents in the local store and returns the results. */
- (FSTDocumentDictionary *)executeQuery:(FSTQuery *)query;
- (firebase::firestore::model::MaybeDocumentMap)executeQuery:(FSTQuery *)query;
Copy link
Member

Choose a reason for hiding this comment

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

You could add a conversion function. It'd be O(n), but presuming you're going to do something with (all) the documents, you're (at least) O(n) anyways.

As it currently stands, this PR assumes elsewhere that these MaybeDocs are actual Docs. The assumption's currently correct, but feels a bit dangerous. If I'm the next one to touch this executeQuery function, I'd happily add the ability to return other subclasses of MaybeDoc because the API says that's fine. You could fix that with documentation, but then I'd have to actually read it. :/

NSEnumerator<FSTDocumentKey *> *enumerator = [self.docs keyEnumeratorFrom:prefix];
for (FSTDocumentKey *key in enumerator) {
if (!query.path.IsPrefixOf(key.path)) {
DocumentKey prefix{query.path.Append("")};
Copy link
Member

Choose a reason for hiding this comment

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

query.path.Append("")

No worse than before, but seems a bit odd. We're not really appending anything, we're just using this to copy. Could we just invoke the copy ctor 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.

This adds a trailing slash; my understanding is that when, for example, the search is for "foo", it allows searching only under "foo/", not under "foobar/", "foo123/", etc.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right. Still not super obvious, but never mind.

if (![maybeDoc isKindOfClass:[FSTDocument class]]) {
continue;
}
FSTDocument *doc = (FSTDocument *)maybeDoc;
FSTDocument *doc = static_cast<FSTDocument *>(maybeDoc);
Copy link
Member

Choose a reason for hiding this comment

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

(again)

Copy link
Member

Choose a reason for hiding this comment

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

Oh; and to be clear, this usage here is perfectly fine due to the isKindOfClass check immediately preceding. Plus it being exactly analogous to the previous version. Don't know why I commented on this usage in the first place (probably wan't paying enough attention.)

@rsgowman rsgowman assigned var-const and unassigned rsgowman Dec 4, 2018
@var-const var-const assigned rsgowman and unassigned var-const Dec 5, 2018
@rsgowman rsgowman assigned var-const and unassigned rsgowman Dec 6, 2018
@var-const var-const assigned rsgowman and unassigned var-const Dec 6, 2018
@@ -26,6 +26,8 @@
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
enableAddressSanitizer = "YES"
enableASanStackUseAfterReturn = "YES"
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a good idea; though unrelated to this PR. No need to split or anything; I'm just calling it out in case it was unintentional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is accidental, thanks for spotting. I commit this with some regularity, unfortunately.


DocumentKeySet newMutatedKeys = previousChanges ? previousChanges.mutatedKeys : _mutatedKeys;
DocumentKeySet oldMutatedKeys = _mutatedKeys;
DocumentKeySet newMutatedKeys = previousChanges ? previousChanges.mutatedKeys : mutatedKeys;
Copy link
Member

Choose a reason for hiding this comment

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

Is mutatedKeys just self->_mutatedKeys? If so, you might be able to eliminate a parameter to this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a free function, and I don't think I can make it a member function of the Objective-C class because it's a template.

if (![maybeDoc isKindOfClass:[FSTDocument class]]) {
continue;
}
FSTDocument *doc = (FSTDocument *)maybeDoc;
FSTDocument *doc = static_cast<FSTDocument *>(maybeDoc);
Copy link
Member

Choose a reason for hiding this comment

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

Oh; and to be clear, this usage here is perfectly fine due to the isKindOfClass check immediately preceding. Plus it being exactly analogous to the previous version. Don't know why I commented on this usage in the first place (probably wan't paying enough attention.)

@@ -163,7 +166,7 @@ NS_ASSUME_NONNULL_BEGIN
- (void)releaseQuery:(FSTQuery *)query;

/** Runs @a query against all the documents in the local store and returns the results. */
- (FSTDocumentDictionary *)executeQuery:(FSTQuery *)query;
- (firebase::firestore::model::MaybeDocumentMap)executeQuery:(FSTQuery *)query;
Copy link
Member

Choose a reason for hiding this comment

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

I like it. If you do too, then lets proceed with this version.

(const firebase::firestore::model::DocumentMap &)docChanges;

/** Like `computeChangesWithDocuments:`, but accepts a map of `MaybeDocument`s. */
// PORTING NOTE: in C++, a container of `Derived` does not convert to a container of `Base`,
Copy link
Contributor Author

@var-const var-const Dec 6, 2018

Choose a reason for hiding this comment

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

@wilhuff Most of this PR is straightforward. The only thing I'd really like to know your stance on is the issue that in C++, SortedMap<Key, Derived*> does not convert to SortedMap<Key, Base*> -- specifically, a DocumentMap cannot be passed in place of MaybeDocumentMap. I'd prefer to avoid creating a new map on the fly, due to performance concerns. The approach here is to have one method taking a DocumentMap and another taking a MaybeDocumentMap, and in the implementation file they both delegate to a template function. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with the source of the problem--that in C++ different template arguments create unrelated types--but I wish we could find a way to avoid this altogether rather than accommodate the two through localized template magic.

Two thoughts:

  • Can we avoid needing both DocumentMap and MaybeDocumentMap?
  • Can we make these types non-aliases that do have an inheritance relationship so that these types are substitutable?

As I remember things, DocumentMap exists to make some cases more type-safe so that callers don't have to handle the non-Document results. However, that choice was made when all the languages we used allowed for covariant template arguments. I think we could reasonably revisit this rather than doubling the API surface area of anything that currently takes a DocumentMap.

Copy link
Contributor

Choose a reason for hiding this comment

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

As I think about this more I'm convinced that we don't need DocumentMap to be a substitutable type for MaybeDocumentMap.

While it's true that ExecuteQuery should not produce anything other than Documents, we only use the result of ExecuteQuery to compute changes to views and that accepts MaybeDocuments. In a world where a DocumentMap can be cheaply substituted for MaybeDocumentMap the type safety this affords is nice, but isn't worth much to preserve.

On the flip side, having to return DocumentMap forces the implementation of ExecuteQuery to explicitly handle other types to prevent adding them to the query results. This is a very useful property that I don't want to give away.

I think what this means is that DocumentMap should have a has-a relationship with an underlying MaybeDocumentMap. You should not be able to construct a DocumentMap from a MaybeDocumentMap but you can build up a DocumentMap using APIs that only accept Documents and then ask for a cheap reference to the underlying MaybeDocumentMap. SortedMap is immutable so this should be cheap.

I think it should be sufficient for DocumentMap to be a very limited subset of SortedMap (omitting iteration, etc). WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really like the idea of DocumentMap being a wrapper class, I think it addresses the concern about signature of executeQuery and makes platform differences better contained. Done, let me know what you think.

a very limited subset of SortedMap (omitting iteration, etc).

I've tried out both approaches -- see the "No iterator?" commit for a diff. I don't feel strongly either way. On one hand, there is some amount of code that uses iteration and calls to find, which would all have to go through underlying_map if DocumentMap does not support iterators. On the other hand, when added, iterator is the largest chunk of code in DocumentMap and the only source of any complexity. All in all, I'd slightly prefer no iterator, but can easily be convinced otherwise.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's proceed without the iterator. The "No iterator?" commit shows that this isn't really worth it.

@wilhuff wilhuff assigned var-const and unassigned wilhuff Dec 8, 2018
}

/** Use this function to "convert" `DocumentMap` to a `MaybeDocumentMap`. */
const MaybeDocumentMap& underlying_map() const {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feel free to suggest a different name.

Copy link
Contributor

Choose a reason for hiding this comment

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

Elsewhere we've used asFoo for the name of a method that returned a Foo-typed view of some other type's underlying data. In that vein, maybe as_maybe_documents? as_maybe_document_map?

It's a little weird in this case though because the accessor-naming pattern makes the actual name seem unrelated to the type.

I have no strong feelings on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I cannot find any names by regex as[A-Z], so I can't compare, but to me, as sort of implies creating new object (I take it to mean something like "create a version of this object represented as a different type). I think underlying implies that a reference is returned, so that it's clear it's a cheap operation. I also don't feel at all strongly about this; I'll be happy to rename if you'd like.

@var-const var-const assigned wilhuff and unassigned var-const Dec 10, 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

@var-const var-const merged commit 673b085 into master Dec 10, 2018
mikehaney24 added a commit that referenced this pull request Jan 2, 2019
* Clean up FIRAuth bits from FIRApp (#2110)

* Clean up FIRAuth bits from FIRApp

* Fix tvOS sample's Auth APIs. (#2158)

* Update CHANGELOG for Firestore v0.16.1 (#2164)

* Update the name of certificates bundle (#2171)

To accommodate for release 5.14.0.

* Fix format string in FSTRemoteStore error logging (#2172)

* C++: replace `FSTMaybeDocumentDictionary` with a C++ equivalent (#2139)

Also eliminate most usages of `FSTDocumentKey`, remove most methods from the Objective-C class and make it just a wrapper over `DocumentKey`. The only usage that cannot be directly replaced by C++ `DocumentKey` is in `FSTFieldValue`.

* Port performance optimizations to speed up reading large collections from Android (#2140)

Straightforward port of firebase/firebase-android-sdk#123.

* When searching for gRPC certificates, search the main bundle as well (#2183)

When the project is manually configured, it's possible that the certificates file gets added to the main bundle, not the Firestore framework bundle; make sure the bundle can be loaded in that case as well.

*  Fix Rome instructions (#2184)

* Use registerLibrary for pods in Firebase workspace (#2137)

* Add versioning to Functions and convert to FIRLibrary
* Convert Firestore to FIRLibrary
* Point travis to FirebaseCore pre-release for its deps
* Update user agent strings to match spec

* Port Memory remote document cache to C++ (#2176)

* Port Memory remote document cache to C++

* Minor tweaks to release note (#2182)

* Minor tweaks

* Update CHANGELOG.md

* Update CHANGELOG.md

* Port leveldb remote document cache to C++ (#2186)

* Port leveldb remote document cache

* Remove start from persistence interface (#2173)

* Remove start from persistence interface, switch FSTLevelDB to use a factory method that returns Status

* Fix small typos in public documentation. (#2192)

* fix the unit test #1451 (#2187)

* Port FSTRemoteDocumentCacheTest to use C++ interface (#2194)

* Release 5.15.0 (#2195)

* Update versions for Release 5.15.0

* Create 5.15.0.json

* Update CHANGELOG for Firestore v0.16.1 (#2164)

* Update the name of certificates bundle (#2171)

To accommodate for release 5.14.0.

* Fix format string in FSTRemoteStore error logging (#2172)

* Update CHANGELOG.md

* Update 5.15.0.json

* Port FSTRemoteDocumentCache (#2196)

* Remove FSTRemoteDocumentCache

* Fix leaks in Firestore (#2199)

* Clean up retain cycle in FSTLevelDB.

* Explicitly CFRelease our SCNetworkReachabilityRef.

* Make gRPC stream delegates weak

* Port DocumentState and UnknownDocument. (#2160)

Part of heldwriteacks.

Serialization work for this is largely deferred until after
nanopb-master is merged with master.

* Port FSTMemoryQueryCache to C++ (#2197)

* Port FSTLevelDBQueryCache to C++ (#2202)

* Port FSTLevelDBQueryCache to C++

* Fix Storage private imports. (#2206)

* Add missing Foundation imports to Interop headers (#2207)

* Migrate Firestore to the v1 protocol (#2200)

* Use python executable directly

  * python2 is not guaranteed to exist
  * scripts aren't directly executable on Windows

* Add Firestore v1 protos

* Point cmake at Firestore v1 protos

* Regenerate protobuf-related sources

* Make local protos refer to v1 protos

* fixup! Regenerate protobuf-related sources

* Remove v1beta1 protos

* s/v1beta1/v1/g in source.

* s/v1beta1/v1/ in the Xcode project

* Remove stale bug comments.

This was fixed by adding an explicit FieldPath API rather than exposing
escaping to the end user.

* Add SymbolCollisionTest comment for ARCore (#2210)

* Continue work on ReferenceSet (#2213)

* Migrate FSTDocumentReference to C++

* Change SortedSet template parameter ordering

Makes it easier to specify a comparator without specifying what the
empty member of the underlying map is.

* Migrate MemoryMutationQueue to C++ references by key

* migrate.py

* CMake

* Finish porting ReferenceSet

* Swap reference set implementation

* Port MemoryQueryCache to use ported ReferenceSet

* Port FSTReferenceSetTest

* Port usage for limbo document refs

* Port LRU and LocalStore usages

* Remove FSTReferenceSet and FSTDocumentReference

* Style

* Add newline

* Implement QueryCache interface and port QueryCache tests (#2209)

* Implement QueryCache interface and port tests

* Port production usages of QueryCache (#2211)

* Remove FSTQueryCache and implementations

* Switch size() to size_t

* Keep imports consistent (#2217)

* Fix private tests by removing unnecessary storyboard entries (#2218)

* Fix xcode 9 build of FDLBuilderTestAppObjC (#2219)

* Rework FieldMask to use a (ordered) set of FieldPaths (#2136)

Rather than a vector.

Port of firebase/firebase-android-sdk#137

* Travis to Xcode 10.1 and clang-format to 8.0.0 (tags/google/stable/2018-08-24) (#2222)

* Travis to Xcode 10.1
* Update to clang-format 8
* Update clang-format homebrew link
* Work around space in filename style.sh issue
mikehaney24 added a commit that referenced this pull request Apr 13, 2019
* Initial structural work for the google data logger. (#2162)

* Inital commit

* Remove Example project and replace with test_spec in the podspec

* Update gitignore to ignore the generated folder.

* Add a script to generate the project.

* Add some basic structure and tests.

* Remove unnecessary files and address PR feedback.

Removes .gitkeep, .gitignore, and .travis.yml files
Modifies the root .gitignore to ignore files generated by cocoapod-generate
Modifies the root .travis.yml to add this podspec to CI
Updates the README with some instructions

* Adding googledatalogger branch to travis CI

* Adding copyrights to files that were missing them

* Move GDLLogTransformer to the public header directory

An alternative is to set CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES = 'YES', but I'm not sure this will work when publishing the pod.

* Add additional base infrastructure for the logging client (#2174)

* Generalize the concept of a logSource

Rename and change the type of the 'log source' to be more appropriately generalized as a log mapping identifier string.

* Expand the API of the logger.

* Add infrastructure for log storage.

* Add infrastructure for the log writer.

* Remove an unnecessary comment.

* Style fixes applied

* Change a missed assert message to make more sense.

* Flesh out the log event and log writer classes (#2175)

* Add timekeeping infrastructure.

* Add the log proto protocol.

* Flesh out the log event a bit.

* Flesh out the log writer.

* Put in comments for the log proto protocol.

* Move queue to a private header and update the TODO.

* Add comment about the QoS tier

* Fix style

* Enable travis for GoogleDataLogger using cocoapods-generate (#2185)

* Add logTarget as a property to GDLLogEvent and connect the logger to the writer.

* Enabled building and testing GoogleDataLogger in travis using cocoapods-generate

* Update Gemfile.lock

* Revert "Add logTarget as a property to GDLLogEvent and connect the logger to the writer."

This reverts commit cce26d3.

* Fix the workspace path.

* Add xcpretty gem

* Add the test directive to the GoogleDataLogger invocation

* Refactor GoogleDataLogger into its own section

Also remove GoogleDataLogger from Xcode9.4 pod lib linting, because the failure was not reproducible.

* Create a log wrapper and better test GDLLogWriter (#2190)

* Add logTarget as a property to GDLLogEvent and connect the logger to the writer.

* Create a log wrapper for use with GULLogger.

* GDLLogTransformer should inherit <NSObject> and require transform:

* Protect against doesNotRespond exceptions and expand tests

* Style and a missing @param.

* Update a comment

* Implement NSSecureCoding protocol for GDLLogEvent (#2191)

* Implement NSSecureCodingProtocol for GDLLogEvent

* Style changes.

* Refactor to address some comments and structure

GDLLogEvent and GDLLogProto are moved to the public folder.

GDLLogEvent has had some public API moved to a private header.

GDLLogWriter now writes the extension object to data after transforming, for serialization purposes.

Various headers updated to conform to module header rules.

* Create some core infrastructure for backends (#2198)

* s/GDLLogClock/GDLClock/

This isn't a class of log clocks, it's a class of clocks.

* Create some core infrastructure to support backends and prioritization of logs.

* Docs and slight changes to the scorer API.

* Missing return statement

* Change 'score' terminology to 'prioritize'. Also style issues.

* Change the protocol being used for a prioritizer.

* Implement -hash and -copy of GDLLogEvent, copy on log, and don't assign extensionBytes in log writer (#2204)

* Implement -hash and -copy of GDLLogEvent

Also implements a custom setter for setting the extension that changes the default behavior to set extensionBytes upon assignment of extension.

Copy the log upon logging, as the comments promised.

Remove setting extensionBytes in the log writer.

Implement a missing method

* Copy the log object upon logging

* Don't assign extensionBytes in the log writer

* Make an implicit loss of precision explicit.

* Add a comment on performance

* Add some test helpers and structure for new classes (#2212)

* Test helpers for GDLBackend and GDLLogPrioritizer

* Add shared uploader structure

* Implement some stubbed methods, update umbrella header, add missing test (#2214)

* Add missing test

* Implement some stubbed methods, update the umbrella header

* Implement log storage (#2215)

* Implement log storage

Includes tests and categories on existing classes to add testing functionality

* Better error handling in tye log storage

* Style and pod lib lint fixes

* Add missing comment

* Implement NSSecureCoding for GDLLogStorage (#2216)

* Implement NSSecureCoding for GDLLogStorage

* Fix style

* Rename variable

* merge master into googledatalogger branch (#2224)

* Clean up FIRAuth bits from FIRApp (#2110)

* Clean up FIRAuth bits from FIRApp

* Fix tvOS sample's Auth APIs. (#2158)

* Update CHANGELOG for Firestore v0.16.1 (#2164)

* Update the name of certificates bundle (#2171)

To accommodate for release 5.14.0.

* Fix format string in FSTRemoteStore error logging (#2172)

* C++: replace `FSTMaybeDocumentDictionary` with a C++ equivalent (#2139)

Also eliminate most usages of `FSTDocumentKey`, remove most methods from the Objective-C class and make it just a wrapper over `DocumentKey`. The only usage that cannot be directly replaced by C++ `DocumentKey` is in `FSTFieldValue`.

* Port performance optimizations to speed up reading large collections from Android (#2140)

Straightforward port of firebase/firebase-android-sdk#123.

* When searching for gRPC certificates, search the main bundle as well (#2183)

When the project is manually configured, it's possible that the certificates file gets added to the main bundle, not the Firestore framework bundle; make sure the bundle can be loaded in that case as well.

*  Fix Rome instructions (#2184)

* Use registerLibrary for pods in Firebase workspace (#2137)

* Add versioning to Functions and convert to FIRLibrary
* Convert Firestore to FIRLibrary
* Point travis to FirebaseCore pre-release for its deps
* Update user agent strings to match spec

* Port Memory remote document cache to C++ (#2176)

* Port Memory remote document cache to C++

* Minor tweaks to release note (#2182)

* Minor tweaks

* Update CHANGELOG.md

* Update CHANGELOG.md

* Port leveldb remote document cache to C++ (#2186)

* Port leveldb remote document cache

* Remove start from persistence interface (#2173)

* Remove start from persistence interface, switch FSTLevelDB to use a factory method that returns Status

* Fix small typos in public documentation. (#2192)

* fix the unit test #1451 (#2187)

* Port FSTRemoteDocumentCacheTest to use C++ interface (#2194)

* Release 5.15.0 (#2195)

* Update versions for Release 5.15.0

* Create 5.15.0.json

* Update CHANGELOG for Firestore v0.16.1 (#2164)

* Update the name of certificates bundle (#2171)

To accommodate for release 5.14.0.

* Fix format string in FSTRemoteStore error logging (#2172)

* Update CHANGELOG.md

* Update 5.15.0.json

* Port FSTRemoteDocumentCache (#2196)

* Remove FSTRemoteDocumentCache

* Fix leaks in Firestore (#2199)

* Clean up retain cycle in FSTLevelDB.

* Explicitly CFRelease our SCNetworkReachabilityRef.

* Make gRPC stream delegates weak

* Port DocumentState and UnknownDocument. (#2160)

Part of heldwriteacks.

Serialization work for this is largely deferred until after
nanopb-master is merged with master.

* Port FSTMemoryQueryCache to C++ (#2197)

* Port FSTLevelDBQueryCache to C++ (#2202)

* Port FSTLevelDBQueryCache to C++

* Fix Storage private imports. (#2206)

* Add missing Foundation imports to Interop headers (#2207)

* Migrate Firestore to the v1 protocol (#2200)

* Use python executable directly

  * python2 is not guaranteed to exist
  * scripts aren't directly executable on Windows

* Add Firestore v1 protos

* Point cmake at Firestore v1 protos

* Regenerate protobuf-related sources

* Make local protos refer to v1 protos

* fixup! Regenerate protobuf-related sources

* Remove v1beta1 protos

* s/v1beta1/v1/g in source.

* s/v1beta1/v1/ in the Xcode project

* Remove stale bug comments.

This was fixed by adding an explicit FieldPath API rather than exposing
escaping to the end user.

* Add SymbolCollisionTest comment for ARCore (#2210)

* Continue work on ReferenceSet (#2213)

* Migrate FSTDocumentReference to C++

* Change SortedSet template parameter ordering

Makes it easier to specify a comparator without specifying what the
empty member of the underlying map is.

* Migrate MemoryMutationQueue to C++ references by key

* migrate.py

* CMake

* Finish porting ReferenceSet

* Swap reference set implementation

* Port MemoryQueryCache to use ported ReferenceSet

* Port FSTReferenceSetTest

* Port usage for limbo document refs

* Port LRU and LocalStore usages

* Remove FSTReferenceSet and FSTDocumentReference

* Style

* Add newline

* Implement QueryCache interface and port QueryCache tests (#2209)

* Implement QueryCache interface and port tests

* Port production usages of QueryCache (#2211)

* Remove FSTQueryCache and implementations

* Switch size() to size_t

* Keep imports consistent (#2217)

* Fix private tests by removing unnecessary storyboard entries (#2218)

* Fix xcode 9 build of FDLBuilderTestAppObjC (#2219)

* Rework FieldMask to use a (ordered) set of FieldPaths (#2136)

Rather than a vector.

Port of firebase/firebase-android-sdk#137

* Travis to Xcode 10.1 and clang-format to 8.0.0 (tags/google/stable/2018-08-24) (#2222)

* Travis to Xcode 10.1
* Update to clang-format 8
* Update clang-format homebrew link
* Work around space in filename style.sh issue

* Create testing infrastructure that simplifies catching exceptions in dispatch_queues (#2226)

* Apply updated clang-format

* Add a custom assert and use it instead of NSAssert

* Define a shared unit test test class and change unit tests to use it

* Add the GDLAssertHelper to be used by GDLAsserts

* Change copyright year, style, and only define the assert macro body if !defined(NS_BLOCK_ASSERTIONS)

* Remove rvm specification from travis (#2227)

* Implement additional tests and enhance GDLLogEvent (#2231)

* Move qosTier to the public API and add a custom prioritization dict

* Set the default qosTier in each logging API

* Change a missing transform: impl to an error, rearrange error enums

We can only rearrange the enums because we've not shipped anything yet.

* Implement additional tests

* Remove extra space

Damned flat macbook keyboards.

* Refactor to allow injection of fakes, and take warnings seriously (#2232)

* Create fakes that can be used during unit tests

* Create a private header for the logger

* All log storage to be injected into the log writer, and now give logs to log storage.

Also changes the tests to use the fakes

* Treat all warnings as errors, and warn pedantic

* Ok nevermind, don't warn_pedantic.

* remove trailing comma

* Remove obsolete TODOs

Not needed, because a fake is being used.

* Add fakes and injection to log storage for the uploader, implement a fast qos test

* Move all unit tests to the Tests/Unit folder (#2234)

* s/GDLUploader->GDLUploadCoordinator/g and s/GDLLogBackend/GDLLogUploader (#2256)

* Implement a clock. (#2273)

* Implement a clock.

Files to pay attention to: GDLClock.h/m

* style

* Enhance the log storage class (#2275)

* Rename fields related to the previous notion of 'backends' to 'uploaders'.

Also changes the declaration of the uploader callback block.

* Add the ability to delete a set of logs by filename, and a conversion method for hashes to files.

* Change the log storage removeLogs method to be log hashes instead of URLS

* Style, and change the completion block to include an error

* Change to sync, since async to adding more async created race condition opportunity.

* Test new storage methods

* Fix coordinator method declarations

* Add test-related functionality, make GDLRegistrar thread-safe and tested (#2285)

* Add some functionality to the test prioritizer

* Change the registrar's API and make it thread safe.

* Add an error message enum for a failed upload

* Add more functionality to the log storage fake

* Make a property readonly.

* Implement the upload coordinator (#2290)

* Implement the upload coordinator

This is a thread safe class to manage the different GDLUploader implementations.

* Remove a bad comment

* Spelling

* Code cleanup (#2297)

Add some nullability specifiers, remove a test that won't compile, change the pod gen script.

* Update podspec and factor out common test sources (#2336)

* Update podspec and factor out common test sources

* Add a wifi-only QoS specifier

* Change the prioritizer protocol to include upload conditions

* Remove an unused log target.

* Call unprioritizeLog and remove an assert that wasn't helpful

* Put the upload completionBlock on the uploader queue

* Fix the CI and podspec.

* [DO NOT MERGE TO MASTER] Raise the cocoapods version to 1.6.0.rc.2

* [DO NOT MERGE TO MASTER] Update Gemfile correctly

* [DO NOT MERGE TO MASTER]  Use the tag, not the version number.

* Correct an incorrect commit

* Remove the name for standard unit tests

* Implement an integration/E2E test of the logging pipeline (#2356)

* Move the -removeLog API to be file-private, it's unused publicly.

* Remove altering of in flight log set, that's done in the onComplete block

* Copy the set of logs given to upload so it's not altered while the pipeline is operating on it.

* Implement an integration/E2E test of the library's pipeline

Includes a dependency on GCDWebServer in the test_spec to run an HTTP server endpoint that the uploader can upload to.

* Rename -protoBytes to -transportBytes

* Change the integration test timing

* Spelling.

* Fix the scheme names in build.sh

* Change cocoapods version from 1.6.0.rc.2 to 1.6.0. (#2358)

* Change cocoapods version from 1.6.0.rc.2 to 1.6.0.

* Update Gemfile.lock

* Rename googledatalogger to GoogleDataTransport (#2379)

* Rename GoogleDataLogger to GoogleDataTransport

All files should be renamed, GDL prefixes should now be GDT.

* Remove references to logging and replace with notion of 'transporting'

* Style, and cleaning up a few more references

* Change travis config to googledatatransport instead of googledatalogger

* Add 'upload packages' to allow prioritizers to pass arbitrary data to uploaders (#2470)

* Update some comments and move the event clock snapshot to the public header

* Create the notion of an 'upload package'

This will allow prioritizers to pass arbitrary data to the uploader that might be needed at upload time.

* Make the -transportBytes protocol method required.

* Make the rest of the framework use the upload package

* Style

* Remove cct.nanopb.c

It was accidentally added.

* Implement a stored event object to simplify in-memory storage (#2497)

* Implement a stored event object to simplify in-memory storage

This will make passing data to the prioritizers and uploaders easier, and it significantly simplifies logic in the storage system while reducing memory footprint.

* Remove two files that always seem to sneak in somehow.

* Style and a needed cast

* Lay the groundwork for CCT support, and include the prioritizer imple… (#2602)

* Lay the groundwork for CCT support, and include the prioritizer implementation.

* Style

* Adjust a flaky test.

* Add GoogleDataTransportCCTSupport configs to travis

* Fix small issues with travis configs

* Fix syntax error in build.sh

* Add SpecsStaging as a source for generation of GDTCCT

* Address shortening warning

* Remove headers that were unintentionally committed.

* Make the podspec description != to the summary

* Spelling

* Expose all properties of a GDTClock, better -hash and -isEqual impls (#2632)

* Expose all properties of a GDTClock, add -hash and -isEqual implementations for GDTStoredEvent

* Style, remove unused define

* Change GDTStoredEvent -isEqual implementation

* Remove unintentionally committed files

* Implement the CCT proto using nanopb (#2652)

* Write scripts to help generate the CCT proto

* Expand the event generator to generate consistent events

* Add the CCT proto and generator options

* Add the nanopb generated sources and nanopb helpers

* Ignore the proto-related directories in GDTCCTSupport

* Fix whitespace

* Clean up generate_cct_protos.sh

Use git instead of curl, rename zip to tar.gz, use readonly variables and proper variable expansion

* Address review comments

Check return of pb_decode correctly.
Use more arbitrary numbers for test event data.
Reimplement GDTCCTEncodeString more intelligently.
Use better initial values for messages.

* Fix memory leak in tests

* Use FOUNDATION_EXTERN consistently

* Fix test name and always initialize nanopb vars.

* Change FOUNDATION_EXTERN to FOUNDATION_EXPORT, fix missing assert code

* Reinit a corrupted response to the default

* Populate an error if decoding the response fails.

* Make high priority events force an upload via upload conditions (#2699)

* Make high priority events force an upload via upload conditions

Forced uploads should be defined as an additional upload condition, as opposed to constructing an ad-hoc upload package. This allows the prioritizer and uploader to do housekeeping more easily.

Address a race condition during high-priority event sending in which a package can be asked for whilst the storage is still asking the prioritizer to deprioritize events that have already been removed from disk.

* Make _timeMillis in GDTClock be the actual unix time

* Style

* Implement the CCT prioritizer and an integration test (#2701)

* Implement the CCT prioritizer

* Add support for high priority uploads

* Add an integration test, demonstrating usage of CCT

* Update the podspec

* Change library podspecs to require 1.5.3, and change old tag string (#2702)

* Remove the GULLogger dependency (#2703)

* Remove the GULLogger dependency

Wrote a quick console logging function that will log to the console in debug mode.

* Style

* Remove the .proto and .options files from the cocoapod (#2708)

* Remove the .proto and .options files from the cocoapod

* Update check_whitespace

* Implement app-lifecycle groundwork (#2800)

* Create the lifecycle class

* Add running in background properties and implement archive paths for the stateful classes.

* Move -stopTimer into the upload coordinator

* Add test for encoding and decoding the storage singleton.

* Add a test transport target.

* Style

* Temporarily commenting out sending lifecycle events.

* More style.

* Demonstrate that I know what year I made this file

* Implement app lifecycle reactivity. (#2801)

* Implement app lifecycle reactivity.

* Re-enable the lifecycle code

* Implement reachability and upload conditions (#2826)

* Add a reachability implementation

* Add a reachability private header

* Define more useful upload conditions and start at 1 instead of 0

* Implement determining upload conditions

* Add hooks for lifecycle events to the CCT implementation.

* Style

* Lower cocoapods required version to 1.4.0 and remove googledatatransport branch from travis config

* Add travis_retry for GoogleDataTransport unit tests.

The E2E integration tests are a bit flaky.
Corrob pushed a commit that referenced this pull request Apr 25, 2019
* Initial structural work for the google data logger. (#2162)

* Inital commit

* Remove Example project and replace with test_spec in the podspec

* Update gitignore to ignore the generated folder.

* Add a script to generate the project.

* Add some basic structure and tests.

* Remove unnecessary files and address PR feedback.

Removes .gitkeep, .gitignore, and .travis.yml files
Modifies the root .gitignore to ignore files generated by cocoapod-generate
Modifies the root .travis.yml to add this podspec to CI
Updates the README with some instructions

* Adding googledatalogger branch to travis CI

* Adding copyrights to files that were missing them

* Move GDLLogTransformer to the public header directory

An alternative is to set CLANG_ALLOW_NON_MODULAR_INCLUDES_IN_FRAMEWORK_MODULES = 'YES', but I'm not sure this will work when publishing the pod.

* Add additional base infrastructure for the logging client (#2174)

* Generalize the concept of a logSource

Rename and change the type of the 'log source' to be more appropriately generalized as a log mapping identifier string.

* Expand the API of the logger.

* Add infrastructure for log storage.

* Add infrastructure for the log writer.

* Remove an unnecessary comment.

* Style fixes applied

* Change a missed assert message to make more sense.

* Flesh out the log event and log writer classes (#2175)

* Add timekeeping infrastructure.

* Add the log proto protocol.

* Flesh out the log event a bit.

* Flesh out the log writer.

* Put in comments for the log proto protocol.

* Move queue to a private header and update the TODO.

* Add comment about the QoS tier

* Fix style

* Enable travis for GoogleDataLogger using cocoapods-generate (#2185)

* Add logTarget as a property to GDLLogEvent and connect the logger to the writer.

* Enabled building and testing GoogleDataLogger in travis using cocoapods-generate

* Update Gemfile.lock

* Revert "Add logTarget as a property to GDLLogEvent and connect the logger to the writer."

This reverts commit cce26d3.

* Fix the workspace path.

* Add xcpretty gem

* Add the test directive to the GoogleDataLogger invocation

* Refactor GoogleDataLogger into its own section

Also remove GoogleDataLogger from Xcode9.4 pod lib linting, because the failure was not reproducible.

* Create a log wrapper and better test GDLLogWriter (#2190)

* Add logTarget as a property to GDLLogEvent and connect the logger to the writer.

* Create a log wrapper for use with GULLogger.

* GDLLogTransformer should inherit <NSObject> and require transform:

* Protect against doesNotRespond exceptions and expand tests

* Style and a missing @param.

* Update a comment

* Implement NSSecureCoding protocol for GDLLogEvent (#2191)

* Implement NSSecureCodingProtocol for GDLLogEvent

* Style changes.

* Refactor to address some comments and structure

GDLLogEvent and GDLLogProto are moved to the public folder.

GDLLogEvent has had some public API moved to a private header.

GDLLogWriter now writes the extension object to data after transforming, for serialization purposes.

Various headers updated to conform to module header rules.

* Create some core infrastructure for backends (#2198)

* s/GDLLogClock/GDLClock/

This isn't a class of log clocks, it's a class of clocks.

* Create some core infrastructure to support backends and prioritization of logs.

* Docs and slight changes to the scorer API.

* Missing return statement

* Change 'score' terminology to 'prioritize'. Also style issues.

* Change the protocol being used for a prioritizer.

* Implement -hash and -copy of GDLLogEvent, copy on log, and don't assign extensionBytes in log writer (#2204)

* Implement -hash and -copy of GDLLogEvent

Also implements a custom setter for setting the extension that changes the default behavior to set extensionBytes upon assignment of extension.

Copy the log upon logging, as the comments promised.

Remove setting extensionBytes in the log writer.

Implement a missing method

* Copy the log object upon logging

* Don't assign extensionBytes in the log writer

* Make an implicit loss of precision explicit.

* Add a comment on performance

* Add some test helpers and structure for new classes (#2212)

* Test helpers for GDLBackend and GDLLogPrioritizer

* Add shared uploader structure

* Implement some stubbed methods, update umbrella header, add missing test (#2214)

* Add missing test

* Implement some stubbed methods, update the umbrella header

* Implement log storage (#2215)

* Implement log storage

Includes tests and categories on existing classes to add testing functionality

* Better error handling in tye log storage

* Style and pod lib lint fixes

* Add missing comment

* Implement NSSecureCoding for GDLLogStorage (#2216)

* Implement NSSecureCoding for GDLLogStorage

* Fix style

* Rename variable

* merge master into googledatalogger branch (#2224)

* Clean up FIRAuth bits from FIRApp (#2110)

* Clean up FIRAuth bits from FIRApp

* Fix tvOS sample's Auth APIs. (#2158)

* Update CHANGELOG for Firestore v0.16.1 (#2164)

* Update the name of certificates bundle (#2171)

To accommodate for release 5.14.0.

* Fix format string in FSTRemoteStore error logging (#2172)

* C++: replace `FSTMaybeDocumentDictionary` with a C++ equivalent (#2139)

Also eliminate most usages of `FSTDocumentKey`, remove most methods from the Objective-C class and make it just a wrapper over `DocumentKey`. The only usage that cannot be directly replaced by C++ `DocumentKey` is in `FSTFieldValue`.

* Port performance optimizations to speed up reading large collections from Android (#2140)

Straightforward port of firebase/firebase-android-sdk#123.

* When searching for gRPC certificates, search the main bundle as well (#2183)

When the project is manually configured, it's possible that the certificates file gets added to the main bundle, not the Firestore framework bundle; make sure the bundle can be loaded in that case as well.

*  Fix Rome instructions (#2184)

* Use registerLibrary for pods in Firebase workspace (#2137)

* Add versioning to Functions and convert to FIRLibrary
* Convert Firestore to FIRLibrary
* Point travis to FirebaseCore pre-release for its deps
* Update user agent strings to match spec

* Port Memory remote document cache to C++ (#2176)

* Port Memory remote document cache to C++

* Minor tweaks to release note (#2182)

* Minor tweaks

* Update CHANGELOG.md

* Update CHANGELOG.md

* Port leveldb remote document cache to C++ (#2186)

* Port leveldb remote document cache

* Remove start from persistence interface (#2173)

* Remove start from persistence interface, switch FSTLevelDB to use a factory method that returns Status

* Fix small typos in public documentation. (#2192)

* fix the unit test #1451 (#2187)

* Port FSTRemoteDocumentCacheTest to use C++ interface (#2194)

* Release 5.15.0 (#2195)

* Update versions for Release 5.15.0

* Create 5.15.0.json

* Update CHANGELOG for Firestore v0.16.1 (#2164)

* Update the name of certificates bundle (#2171)

To accommodate for release 5.14.0.

* Fix format string in FSTRemoteStore error logging (#2172)

* Update CHANGELOG.md

* Update 5.15.0.json

* Port FSTRemoteDocumentCache (#2196)

* Remove FSTRemoteDocumentCache

* Fix leaks in Firestore (#2199)

* Clean up retain cycle in FSTLevelDB.

* Explicitly CFRelease our SCNetworkReachabilityRef.

* Make gRPC stream delegates weak

* Port DocumentState and UnknownDocument. (#2160)

Part of heldwriteacks.

Serialization work for this is largely deferred until after
nanopb-master is merged with master.

* Port FSTMemoryQueryCache to C++ (#2197)

* Port FSTLevelDBQueryCache to C++ (#2202)

* Port FSTLevelDBQueryCache to C++

* Fix Storage private imports. (#2206)

* Add missing Foundation imports to Interop headers (#2207)

* Migrate Firestore to the v1 protocol (#2200)

* Use python executable directly

  * python2 is not guaranteed to exist
  * scripts aren't directly executable on Windows

* Add Firestore v1 protos

* Point cmake at Firestore v1 protos

* Regenerate protobuf-related sources

* Make local protos refer to v1 protos

* fixup! Regenerate protobuf-related sources

* Remove v1beta1 protos

* s/v1beta1/v1/g in source.

* s/v1beta1/v1/ in the Xcode project

* Remove stale bug comments.

This was fixed by adding an explicit FieldPath API rather than exposing
escaping to the end user.

* Add SymbolCollisionTest comment for ARCore (#2210)

* Continue work on ReferenceSet (#2213)

* Migrate FSTDocumentReference to C++

* Change SortedSet template parameter ordering

Makes it easier to specify a comparator without specifying what the
empty member of the underlying map is.

* Migrate MemoryMutationQueue to C++ references by key

* migrate.py

* CMake

* Finish porting ReferenceSet

* Swap reference set implementation

* Port MemoryQueryCache to use ported ReferenceSet

* Port FSTReferenceSetTest

* Port usage for limbo document refs

* Port LRU and LocalStore usages

* Remove FSTReferenceSet and FSTDocumentReference

* Style

* Add newline

* Implement QueryCache interface and port QueryCache tests (#2209)

* Implement QueryCache interface and port tests

* Port production usages of QueryCache (#2211)

* Remove FSTQueryCache and implementations

* Switch size() to size_t

* Keep imports consistent (#2217)

* Fix private tests by removing unnecessary storyboard entries (#2218)

* Fix xcode 9 build of FDLBuilderTestAppObjC (#2219)

* Rework FieldMask to use a (ordered) set of FieldPaths (#2136)

Rather than a vector.

Port of firebase/firebase-android-sdk#137

* Travis to Xcode 10.1 and clang-format to 8.0.0 (tags/google/stable/2018-08-24) (#2222)

* Travis to Xcode 10.1
* Update to clang-format 8
* Update clang-format homebrew link
* Work around space in filename style.sh issue

* Create testing infrastructure that simplifies catching exceptions in dispatch_queues (#2226)

* Apply updated clang-format

* Add a custom assert and use it instead of NSAssert

* Define a shared unit test test class and change unit tests to use it

* Add the GDLAssertHelper to be used by GDLAsserts

* Change copyright year, style, and only define the assert macro body if !defined(NS_BLOCK_ASSERTIONS)

* Remove rvm specification from travis (#2227)

* Implement additional tests and enhance GDLLogEvent (#2231)

* Move qosTier to the public API and add a custom prioritization dict

* Set the default qosTier in each logging API

* Change a missing transform: impl to an error, rearrange error enums

We can only rearrange the enums because we've not shipped anything yet.

* Implement additional tests

* Remove extra space

Damned flat macbook keyboards.

* Refactor to allow injection of fakes, and take warnings seriously (#2232)

* Create fakes that can be used during unit tests

* Create a private header for the logger

* All log storage to be injected into the log writer, and now give logs to log storage.

Also changes the tests to use the fakes

* Treat all warnings as errors, and warn pedantic

* Ok nevermind, don't warn_pedantic.

* remove trailing comma

* Remove obsolete TODOs

Not needed, because a fake is being used.

* Add fakes and injection to log storage for the uploader, implement a fast qos test

* Move all unit tests to the Tests/Unit folder (#2234)

* s/GDLUploader->GDLUploadCoordinator/g and s/GDLLogBackend/GDLLogUploader (#2256)

* Implement a clock. (#2273)

* Implement a clock.

Files to pay attention to: GDLClock.h/m

* style

* Enhance the log storage class (#2275)

* Rename fields related to the previous notion of 'backends' to 'uploaders'.

Also changes the declaration of the uploader callback block.

* Add the ability to delete a set of logs by filename, and a conversion method for hashes to files.

* Change the log storage removeLogs method to be log hashes instead of URLS

* Style, and change the completion block to include an error

* Change to sync, since async to adding more async created race condition opportunity.

* Test new storage methods

* Fix coordinator method declarations

* Add test-related functionality, make GDLRegistrar thread-safe and tested (#2285)

* Add some functionality to the test prioritizer

* Change the registrar's API and make it thread safe.

* Add an error message enum for a failed upload

* Add more functionality to the log storage fake

* Make a property readonly.

* Implement the upload coordinator (#2290)

* Implement the upload coordinator

This is a thread safe class to manage the different GDLUploader implementations.

* Remove a bad comment

* Spelling

* Code cleanup (#2297)

Add some nullability specifiers, remove a test that won't compile, change the pod gen script.

* Update podspec and factor out common test sources (#2336)

* Update podspec and factor out common test sources

* Add a wifi-only QoS specifier

* Change the prioritizer protocol to include upload conditions

* Remove an unused log target.

* Call unprioritizeLog and remove an assert that wasn't helpful

* Put the upload completionBlock on the uploader queue

* Fix the CI and podspec.

* [DO NOT MERGE TO MASTER] Raise the cocoapods version to 1.6.0.rc.2

* [DO NOT MERGE TO MASTER] Update Gemfile correctly

* [DO NOT MERGE TO MASTER]  Use the tag, not the version number.

* Correct an incorrect commit

* Remove the name for standard unit tests

* Implement an integration/E2E test of the logging pipeline (#2356)

* Move the -removeLog API to be file-private, it's unused publicly.

* Remove altering of in flight log set, that's done in the onComplete block

* Copy the set of logs given to upload so it's not altered while the pipeline is operating on it.

* Implement an integration/E2E test of the library's pipeline

Includes a dependency on GCDWebServer in the test_spec to run an HTTP server endpoint that the uploader can upload to.

* Rename -protoBytes to -transportBytes

* Change the integration test timing

* Spelling.

* Fix the scheme names in build.sh

* Change cocoapods version from 1.6.0.rc.2 to 1.6.0. (#2358)

* Change cocoapods version from 1.6.0.rc.2 to 1.6.0.

* Update Gemfile.lock

* Rename googledatalogger to GoogleDataTransport (#2379)

* Rename GoogleDataLogger to GoogleDataTransport

All files should be renamed, GDL prefixes should now be GDT.

* Remove references to logging and replace with notion of 'transporting'

* Style, and cleaning up a few more references

* Change travis config to googledatatransport instead of googledatalogger

* Add 'upload packages' to allow prioritizers to pass arbitrary data to uploaders (#2470)

* Update some comments and move the event clock snapshot to the public header

* Create the notion of an 'upload package'

This will allow prioritizers to pass arbitrary data to the uploader that might be needed at upload time.

* Make the -transportBytes protocol method required.

* Make the rest of the framework use the upload package

* Style

* Remove cct.nanopb.c

It was accidentally added.

* Implement a stored event object to simplify in-memory storage (#2497)

* Implement a stored event object to simplify in-memory storage

This will make passing data to the prioritizers and uploaders easier, and it significantly simplifies logic in the storage system while reducing memory footprint.

* Remove two files that always seem to sneak in somehow.

* Style and a needed cast

* Lay the groundwork for CCT support, and include the prioritizer imple… (#2602)

* Lay the groundwork for CCT support, and include the prioritizer implementation.

* Style

* Adjust a flaky test.

* Add GoogleDataTransportCCTSupport configs to travis

* Fix small issues with travis configs

* Fix syntax error in build.sh

* Add SpecsStaging as a source for generation of GDTCCT

* Address shortening warning

* Remove headers that were unintentionally committed.

* Make the podspec description != to the summary

* Spelling

* Expose all properties of a GDTClock, better -hash and -isEqual impls (#2632)

* Expose all properties of a GDTClock, add -hash and -isEqual implementations for GDTStoredEvent

* Style, remove unused define

* Change GDTStoredEvent -isEqual implementation

* Remove unintentionally committed files

* Implement the CCT proto using nanopb (#2652)

* Write scripts to help generate the CCT proto

* Expand the event generator to generate consistent events

* Add the CCT proto and generator options

* Add the nanopb generated sources and nanopb helpers

* Ignore the proto-related directories in GDTCCTSupport

* Fix whitespace

* Clean up generate_cct_protos.sh

Use git instead of curl, rename zip to tar.gz, use readonly variables and proper variable expansion

* Address review comments

Check return of pb_decode correctly.
Use more arbitrary numbers for test event data.
Reimplement GDTCCTEncodeString more intelligently.
Use better initial values for messages.

* Fix memory leak in tests

* Use FOUNDATION_EXTERN consistently

* Fix test name and always initialize nanopb vars.

* Change FOUNDATION_EXTERN to FOUNDATION_EXPORT, fix missing assert code

* Reinit a corrupted response to the default

* Populate an error if decoding the response fails.

* Make high priority events force an upload via upload conditions (#2699)

* Make high priority events force an upload via upload conditions

Forced uploads should be defined as an additional upload condition, as opposed to constructing an ad-hoc upload package. This allows the prioritizer and uploader to do housekeeping more easily.

Address a race condition during high-priority event sending in which a package can be asked for whilst the storage is still asking the prioritizer to deprioritize events that have already been removed from disk.

* Make _timeMillis in GDTClock be the actual unix time

* Style

* Implement the CCT prioritizer and an integration test (#2701)

* Implement the CCT prioritizer

* Add support for high priority uploads

* Add an integration test, demonstrating usage of CCT

* Update the podspec

* Change library podspecs to require 1.5.3, and change old tag string (#2702)

* Remove the GULLogger dependency (#2703)

* Remove the GULLogger dependency

Wrote a quick console logging function that will log to the console in debug mode.

* Style

* Remove the .proto and .options files from the cocoapod (#2708)

* Remove the .proto and .options files from the cocoapod

* Update check_whitespace

* Implement app-lifecycle groundwork (#2800)

* Create the lifecycle class

* Add running in background properties and implement archive paths for the stateful classes.

* Move -stopTimer into the upload coordinator

* Add test for encoding and decoding the storage singleton.

* Add a test transport target.

* Style

* Temporarily commenting out sending lifecycle events.

* More style.

* Demonstrate that I know what year I made this file

* Implement app lifecycle reactivity. (#2801)

* Implement app lifecycle reactivity.

* Re-enable the lifecycle code

* Implement reachability and upload conditions (#2826)

* Add a reachability implementation

* Add a reachability private header

* Define more useful upload conditions and start at 1 instead of 0

* Implement determining upload conditions

* Add hooks for lifecycle events to the CCT implementation.

* Style

* Lower cocoapods required version to 1.4.0 and remove googledatatransport branch from travis config

* Add travis_retry for GoogleDataTransport unit tests.

The E2E integration tests are a bit flaky.
@firebase firebase locked and limited conversation to collaborators Oct 24, 2019
@paulb777 paulb777 deleted the varconst/eliminate-fst-maybe-document-dictionary branch July 8, 2020 23:57
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.

5 participants