-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Port Firestore Document to C++ #777
Changes from 16 commits
d74a94a
3596c88
a4a9a67
8c048df
2a18020
4488fe0
39b9218
a993f3e
d4e870e
b3e5f51
03079bd
b5983ed
21614a3
1c562d0
4d41234
da332cf
6842d83
c4547bc
cb46ddb
cda7050
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
/* | ||
* Copyright 2018 Google | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#include "Firestore/core/src/firebase/firestore/model/document.h" | ||
|
||
#include <utility> | ||
|
||
#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" | ||
|
||
namespace firebase { | ||
namespace firestore { | ||
namespace model { | ||
|
||
Document::Document(const FieldValue& data, | ||
const DocumentKey& key, | ||
const SnapshotVersion& version, | ||
bool has_local_mutations) | ||
: MaybeDocument(key, version), | ||
data_(data), | ||
has_local_mutations_(has_local_mutations) { | ||
set_type(Type::Document); | ||
FIREBASE_ASSERT(FieldValue::Type::Object == data.type()); | ||
} | ||
|
||
Document::Document(FieldValue&& data, | ||
const DocumentKey& key, | ||
const SnapshotVersion& version, | ||
bool has_local_mutations) | ||
: MaybeDocument(key, version), | ||
data_(std::move(data)), | ||
has_local_mutations_(has_local_mutations) { | ||
set_type(Type::Document); | ||
FIREBASE_ASSERT(FieldValue::Type::Object == data.type()); | ||
} | ||
|
||
} // namespace model | ||
} // namespace firestore | ||
} // namespace firebase |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
/* | ||
* Copyright 2018 Google | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_DOCUMENT_H_ | ||
#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_DOCUMENT_H_ | ||
|
||
#include "Firestore/core/src/firebase/firestore/model/field_value.h" | ||
#include "Firestore/core/src/firebase/firestore/model/maybe_document.h" | ||
|
||
namespace firebase { | ||
namespace firestore { | ||
namespace model { | ||
|
||
/** | ||
* Represents a document in Firestore with a key, version, data and whether the | ||
* data has local mutations applied to it. | ||
*/ | ||
class Document : public MaybeDocument { | ||
public: | ||
Document(const FieldValue& data, | ||
const DocumentKey& key, | ||
const SnapshotVersion& version, | ||
bool has_local_mutations); | ||
|
||
Document(FieldValue&& data, | ||
const DocumentKey& key, | ||
const SnapshotVersion& version, | ||
bool has_local_mutations); | ||
|
||
const FieldValue& data() const { | ||
return data_; | ||
} | ||
|
||
bool has_local_mutations() const { | ||
return has_local_mutations_; | ||
} | ||
|
||
private: | ||
FieldValue data_; // This is of type Object. | ||
bool has_local_mutations_; | ||
}; | ||
|
||
/** Compares against another Document. */ | ||
inline bool operator==(const Document& lhs, const Document& rhs) { | ||
return lhs.version() == rhs.version() && lhs.key() == rhs.key() && | ||
lhs.has_local_mutations() == rhs.has_local_mutations() && | ||
lhs.data() == rhs.data(); | ||
} | ||
|
||
inline bool operator!=(const Document& lhs, const Document& rhs) { | ||
return !(lhs == rhs); | ||
} | ||
|
||
} // namespace model | ||
} // namespace firestore | ||
} // namespace firebase | ||
|
||
#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_DOCUMENT_H_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
/* | ||
* Copyright 2018 Google | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#include "Firestore/core/src/firebase/firestore/model/maybe_document.h" | ||
|
||
namespace firebase { | ||
namespace firestore { | ||
namespace model { | ||
|
||
MaybeDocument::MaybeDocument(const DocumentKey& key, | ||
const SnapshotVersion& version) | ||
: type_(Type::Unknown), key_(key), version_(version) { | ||
} | ||
|
||
} // namespace model | ||
} // namespace firestore | ||
} // namespace firebase |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
/* | ||
* Copyright 2018 Google | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_MAYBE_DOCUMENT_H_ | ||
#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_MAYBE_DOCUMENT_H_ | ||
|
||
#include "Firestore/core/src/firebase/firestore/model/document_key.h" | ||
#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" | ||
|
||
namespace firebase { | ||
namespace firestore { | ||
namespace model { | ||
|
||
/** | ||
* The result of a lookup for a given path may be an existing document or a | ||
* tombstone that marks the path deleted. | ||
*/ | ||
class MaybeDocument { | ||
public: | ||
enum class Type { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This deserves a comment. At the very least a porting note on why it exists here but not in the other clients. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
Unknown, | ||
Document, | ||
NoDocument, | ||
}; | ||
|
||
MaybeDocument(const DocumentKey& key, const SnapshotVersion& version); | ||
|
||
Type type() const { | ||
return type_; | ||
} | ||
|
||
const DocumentKey& key() const { | ||
return key_; | ||
} | ||
|
||
const SnapshotVersion& version() const { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's worth taking a look through the other clients when doing these ports and seeing if there any things we missed in the Objective-C port. In this case there are some useful comments in android. /**
* Returns the version of this document if it exists or a version at which this document was
* guaranteed to not exist.
*/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
return version_; | ||
} | ||
|
||
protected: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could probably make these private. (Or at least key and version; though style guide suggests they should all be private.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
// Only allow subclass to set their types. | ||
void set_type(Type type) { | ||
type_ = type; | ||
} | ||
|
||
private: | ||
Type type_; | ||
DocumentKey key_; | ||
SnapshotVersion version_; | ||
}; | ||
|
||
/** Compares against another MaybeDocument. */ | ||
inline bool operator<(const MaybeDocument& lhs, const MaybeDocument& rhs) { | ||
return lhs.key() < rhs.key(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're not comparing type or version. Is that on purpose? Since version/type are user visible, they should (usually) be included in op<, opt==, etc. (For DocSnap/QuerySnap, we're looking to not include metadata. Perhaps that applies here too? If so, we should add some docs to make this clear as it's otherwise unexpected. OTOH, perhaps just comparing the key is an optimization? i.e. if the keys are the same, then the other fields must be the same? If so, call that out in a comment.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me sync with SFO folks to see what is the design of the comparator. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed in chat, we should only define equality for these documents but they should not be comparable by default. There exists an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
|
||
inline bool operator>(const MaybeDocument& lhs, const MaybeDocument& rhs) { | ||
return lhs.key() > rhs.key(); | ||
} | ||
|
||
inline bool operator>=(const MaybeDocument& lhs, const MaybeDocument& rhs) { | ||
return lhs.key() >= rhs.key(); | ||
} | ||
|
||
inline bool operator<=(const MaybeDocument& lhs, const MaybeDocument& rhs) { | ||
return lhs.key() <= rhs.key(); | ||
} | ||
|
||
inline bool operator!=(const MaybeDocument& lhs, const MaybeDocument& rhs) { | ||
return lhs.key() != rhs.key() || lhs.type() != rhs.type(); | ||
} | ||
|
||
inline bool operator==(const MaybeDocument& lhs, const MaybeDocument& rhs) { | ||
return lhs.key() == rhs.key() && lhs.type() == rhs.type(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're not comparing 'version'. Is that on purpose? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the objc comparison is purely key-based; but let me sync with folks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be return lhs.type() == rhs.type() && lhs.equal(rhs); Where equal is defined here as private:
virtual bool equal(const MaybeDocument& lhs, const MaybeDocument& rhs) = 0; Specific implementations can assume that lhs and rhs are properly the correct type already (and can just static cast). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
|
||
} // namespace model | ||
} // namespace firestore | ||
} // namespace firebase | ||
|
||
#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_MAYBE_DOCUMENT_H_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
/* | ||
* Copyright 2018 Google | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#include "Firestore/core/src/firebase/firestore/model/no_document.h" | ||
|
||
namespace firebase { | ||
namespace firestore { | ||
namespace model { | ||
|
||
NoDocument::NoDocument(const DocumentKey& key, const SnapshotVersion& version) | ||
: MaybeDocument(key, version) { | ||
set_type(Type::NoDocument); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should type just be a constructor parameter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
|
||
} // namespace model | ||
} // namespace firestore | ||
} // namespace firebase |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/* | ||
* Copyright 2018 Google | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_NO_DOCUMENT_H_ | ||
#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_NO_DOCUMENT_H_ | ||
|
||
#include "Firestore/core/src/firebase/firestore/model/maybe_document.h" | ||
|
||
namespace firebase { | ||
namespace firestore { | ||
namespace model { | ||
|
||
/** Represents that no documents exists for the key at the given version. */ | ||
class NoDocument : public MaybeDocument { | ||
public: | ||
NoDocument(const DocumentKey& key, const SnapshotVersion& version); | ||
}; | ||
|
||
} // namespace model | ||
} // namespace firestore | ||
} // namespace firebase | ||
|
||
#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_NO_DOCUMENT_H_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than duplicating some of the comparisons from the parent class, you could simply call the parent class' op==() and then additionally check stuff from the child class. This would make you future proof should additional fields be added to the parent class.
This assumes that for two Document objects to be equal, their MaybeDocument parent instances are also equal. (Which curiously, isn't quite true here; but see below.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, that's something I want to discuss last week's API review but didn't have time.
MaybeDocument
(and thus all subclasses) has comparator, mainly for inequality (<) check, which is purely based onDocumentKey
. On the other hand,Document
has equality check (==) that actually means to check each properties.