Skip to content

Commit ab6bf9e

Browse files
MichaelCuevasmeta-codesync[bot]
authored andcommitted
FilteredBackingStore: show compareObjectsById failure when filter versions differ
Summary: # This diff Adds a test that shows that compareObjectsById fails when two FilteredObjectIDs are semantically equal, but not bytewise equal. This bug is addressed in the next diff # Context See D87029065 Reviewed By: vilatto Differential Revision: D87029072 fbshipit-source-id: 17dd840d89f94c19d624b63aa9dcea0d2f1c0c75
1 parent cc71e0a commit ab6bf9e

1 file changed

Lines changed: 34 additions & 5 deletions

File tree

eden/fs/store/test/FilteredBackingStoreTest.cpp

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ using folly::io::Cursor;
3939
const char kTestFilter1[] = "V1:foo";
4040
const char kTestFilter2[] = "V1:football2";
4141
const char kTestFilter3[] = "V2:football3";
42-
const char kTestFilter4[] = "Legacy:shouldFilterZeroObjects";
42+
const char kTestFilter4Legacy[] = "Legacy:shouldFilterZeroObjects";
43+
const char kTestFilter4V1[] = "V1:shouldFilterZeroObjects";
4344
const char kTestFilter5[] = "V1:bazbar";
4445
const char kTestFilter6[] =
4546
"\
@@ -733,7 +734,7 @@ TEST_F(FakeSubstringFilteredBackingStoreTest, testCompareTreeObjectsById) {
733734
auto rootFuture1 = filteredStore_
734735
->getRootTree(
735736
RootId{FilteredBackingStore::createFilteredRootId(
736-
"1", kTestFilter4)},
737+
"1", kTestFilter4Legacy)},
737738
ObjectFetchContext::getNullContext())
738739
.semi()
739740
.via(&executor);
@@ -744,6 +745,14 @@ TEST_F(FakeSubstringFilteredBackingStoreTest, testCompareTreeObjectsById) {
744745
ObjectFetchContext::getNullContext())
745746
.semi()
746747
.via(&executor);
748+
auto rootFuture1V1 =
749+
filteredStore_
750+
->getRootTree(
751+
RootId{FilteredBackingStore::createFilteredRootId(
752+
"1", kTestFilter4V1)},
753+
ObjectFetchContext::getNullContext())
754+
.semi()
755+
.via(&executor);
747756

748757
// Trigger commit1, then rootDirTree to make rootFuture1 ready.
749758
commit1->trigger();
@@ -752,6 +761,7 @@ TEST_F(FakeSubstringFilteredBackingStoreTest, testCompareTreeObjectsById) {
752761
rootDirTree->trigger();
753762
executor.drain();
754763
auto rootDirRes1 = std::move(rootFuture1).get(0ms);
764+
auto rootDirRes1V1 = std::move(rootFuture1V1).get(0ms);
755765

756766
// Get the object IDs of all the trees from commit 1.
757767
auto [childName, childEntry] = *rootDirRes1.tree->find("child"_pc);
@@ -763,14 +773,25 @@ TEST_F(FakeSubstringFilteredBackingStoreTest, testCompareTreeObjectsById) {
763773
auto [grandchildName, grandchildEntry] = *childDirRes1->find("grandchild"_pc);
764774
auto grandchildOID = grandchildEntry.getObjectId();
765775

776+
// Get the object IDs of all the trees from commit 1 (V1 filters).
777+
auto [childName1V1, childEntry1V1] = *rootDirRes1V1.tree->find("child"_pc);
778+
auto childOID1V1 = childEntry1V1.getObjectId();
779+
auto childFuture1V1 = filteredStore_->getTree(
780+
childOID1V1, ObjectFetchContext::getNullContext());
781+
childTree->trigger();
782+
auto childDirRes1V1 = std::move(childFuture1V1).get(0ms).tree;
783+
auto [grandchildName1V1, grandchildEntry1V1] =
784+
*childDirRes1V1->find("grandchild"_pc);
785+
auto grandchildOID1V1 = grandchildEntry1V1.getObjectId();
786+
766787
// Trigger commit2, then rootDirTreeExtended to make rootFuture2 ready.
767788
commit2->trigger();
768789
executor.drain();
769790
modifiedRootDirTree->trigger();
770791
executor.drain();
771792
auto rootDirCommit2Res = std::move(rootFuture2).get(0ms);
772793

773-
// Get the object IDs of all the blobs from commit 1.
794+
// Get the object IDs of all the blobs from commit 2.
774795
auto [childName2, childEntry2] = *rootDirCommit2Res.tree->find("child"_pc);
775796
auto childOID2 = childEntry2.getObjectId();
776797
auto childFuture2 =
@@ -804,6 +825,14 @@ TEST_F(FakeSubstringFilteredBackingStoreTest, testCompareTreeObjectsById) {
804825
EXPECT_TRUE(
805826
filteredStore_->compareObjectsById(grandchildOID, grandchildOID2) ==
806827
ObjectComparison::Unknown);
828+
829+
// FIXME: These two trees have the same underlying tree (grandchildTreeId) and
830+
// semantically identical filters. They should be Identical, but this will
831+
// fail because compareObjectsById does a bytewise comparison instead of
832+
// calling areFiltersIdentical.
833+
EXPECT_EQ(
834+
filteredStore_->compareObjectsById(grandchildOID, grandchildOID1V1),
835+
ObjectComparison::Unknown);
807836
}
808837

809838
TEST_F(FakeSubstringFilteredBackingStoreTest, getGlobFiles) {
@@ -812,8 +841,8 @@ TEST_F(FakeSubstringFilteredBackingStoreTest, getGlobFiles) {
812841
RootId{FilteredBackingStore::createFilteredRootId("1", kTestFilter1)};
813842
RootId rootId2 =
814843
RootId{FilteredBackingStore::createFilteredRootId("2", kTestFilter2)};
815-
RootId rootId3 =
816-
RootId{FilteredBackingStore::createFilteredRootId("3", kTestFilter4)};
844+
RootId rootId3 = RootId{
845+
FilteredBackingStore::createFilteredRootId("3", kTestFilter4Legacy)};
817846
RootId rootId4 =
818847
RootId{FilteredBackingStore::createFilteredRootId("4", kTestFilter7)};
819848
RootId rootId5 =

0 commit comments

Comments
 (0)