Skip to content

Commit

Permalink
bugfix: namespaceDepth filter loses entry content (#6648)
Browse files Browse the repository at this point in the history
implicit namespaces are somewhat deprecated and thus also the
namespaceDepth filter

however as long as the filter exists, it should not lose already
retrieved information about the content of real entries
  • Loading branch information
XN137 committed Apr 21, 2023
1 parent 1233683 commit 0540aa6
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -776,11 +776,15 @@ protected Set<Check> checksForEntry(KeyEntry entry) {
continue;
}

Content c = key.getContent();
EntriesResponse.Entry entry =
EntriesResponse.Entry.entry(key.getKey(), key.getType(), key.getContentId());
c != null
? EntriesResponse.Entry.entry(key.getKey(), key.getType(), c)
: EntriesResponse.Entry.entry(key.getKey(), key.getType(), key.getContentId());

entry = namespaceDepthMapping(entry, depth);
entry = maybeTruncateToDepth(entry, depth);

// add implicit namespace entries only once (single parent of multiple real entries)
if (seen.add(entry.getName())) {
if (!pagedResponseHandler.addEntry(entry)) {
pagedResponseHandler.hasMore(authz.tokenForCurrent());
Expand Down Expand Up @@ -815,12 +819,16 @@ protected Set<Check> checksForEntry(KeyEntry entry) {
}
}

private static EntriesResponse.Entry namespaceDepthMapping(
private static EntriesResponse.Entry maybeTruncateToDepth(
EntriesResponse.Entry entry, int depth) {
Content.Type type =
entry.getName().getElementCount() > depth ? Content.Type.NAMESPACE : entry.getType();
ContentKey key = ContentKey.of(entry.getName().getElements().subList(0, depth));
return EntriesResponse.Entry.entry(key, type);
List<String> nameElements = entry.getName().getElements();
boolean truncateToNamespace = nameElements.size() > depth;
if (truncateToNamespace) {
// implicit namespace entry at target depth (virtual parent of real entry)
ContentKey namespaceKey = ContentKey.of(nameElements.subList(0, depth));
return EntriesResponse.Entry.entry(namespaceKey, Content.Type.NAMESPACE);
}
return entry;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static com.google.common.collect.Maps.immutableEntry;
import static java.util.stream.Collectors.toSet;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.tuple;
import static org.junit.jupiter.api.Assumptions.abort;
import static org.projectnessie.model.CommitMeta.fromMessage;
Expand Down Expand Up @@ -341,14 +342,37 @@ public void filterEntriesByNamespaceAndPrefixDepth(ReferenceMode refMode)
immutableEntry(ContentKey.of("a", "b", "fourthTable"), Content.Type.ICEBERG_TABLE),
immutableEntry(ContentKey.of("a", "b", "c"), Content.Type.NAMESPACE));

entries = entries(reference, 4, "entry.namespace.matches('a\\\\.b\\\\.c(\\\\.|$)')");
String filterThatFindsTwoTables = "entry.namespace.matches('a\\\\.b\\\\.c(\\\\.|$)')";
entries = entries(reference, 4, filterThatFindsTwoTables);
soft.assertThat(entries)
.hasSize(2)
.map(e -> immutableEntry(e.getName(), e.getType()))
.containsExactlyInAnyOrder(
immutableEntry(ContentKey.of("a", "b", "c", "secondTable"), Content.Type.ICEBERG_TABLE),
immutableEntry(ContentKey.of("a", "b", "c", "firstTable"), Content.Type.ICEBERG_TABLE));

// namespaceDepth filter always returns contentId of real entries
entries = entries(reference.getName(), reference.getHash(), 4, filterThatFindsTwoTables, false);
soft.assertThat(entries)
.hasSize(2)
.allSatisfy(
entry -> {
assertThat(entry.getType()).isEqualTo(Content.Type.ICEBERG_TABLE);
assertThat(entry.getContentId()).isNotNull();
assertThat(entry.getContent()).isNull();
});

// namespaceDepth filter returns content of real entries if requested
entries = entries(reference.getName(), reference.getHash(), 4, filterThatFindsTwoTables, true);
soft.assertThat(entries)
.hasSize(2)
.allSatisfy(
entry -> {
assertThat(entry.getType()).isEqualTo(Content.Type.ICEBERG_TABLE);
assertThat(entry.getContentId()).isNotNull();
assertThat(entry.getContent()).isNotNull();
});

entries = entries(reference, 5, "entry.namespace.matches('(\\\\.|$)')");
soft.assertThat(entries).isEmpty();

Expand Down

0 comments on commit 0540aa6

Please sign in to comment.