Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
import java.io.BufferedReader;
import java.io.IOException;
import java.io.Reader;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Deque;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
Expand All @@ -20,9 +18,9 @@

public class CodeownersImpl implements Codeowners {

private final Collection<Section> sections;
private final List<Section> sections;

private CodeownersImpl(Collection<Section> sections) {
private CodeownersImpl(List<Section> sections) {
this.sections = sections;
}

Expand All @@ -32,17 +30,25 @@ private CodeownersImpl(Collection<Section> sections) {
*/
@Override
public @Nullable Collection<String> getOwners(@Nonnull String path) {
char[] pathCharacters = path.toCharArray();
if (sections.size() == 1) {
Section section = sections.get(0);
if (section.isExcluded(path)) {
return new ArrayList<>();
}
Entry entry = section.findMatchingEntry(path);
return entry != null ? new ArrayList<>(entry.getOwners()) : null;
}

Set<String> owners = null;
for (Section section : sections) {
if (section.isExcluded(pathCharacters)) {
if (section.isExcluded(path)) {
if (owners == null) {
owners = new LinkedHashSet<>();
}
continue;
}

Entry entry = section.findMatchingEntry(pathCharacters);
Entry entry = section.findMatchingEntry(path);
if (entry != null) {
if (owners == null) {
owners = new LinkedHashSet<>();
Expand Down Expand Up @@ -90,36 +96,4 @@ public static Codeowners parse(Reader r) throws IOException {
sections.addAll(namedSections.values());
return new CodeownersImpl(sections);
}

private static final class Section {

private final Deque<Entry> entries = new ArrayDeque<>();
private final Collection<Entry> exclusions = new ArrayList<>();

private void add(Entry entry) {
if (entry.isExclusion()) {
exclusions.add(entry);
} else {
entries.offerFirst(entry);
}
}

private boolean isExcluded(char[] path) {
for (Entry exclusion : exclusions) {
if (exclusion.getMatcher().consume(path, 0) >= 0) {
return true;
}
}
return false;
}

private @Nullable Entry findMatchingEntry(char[] path) {
for (Entry entry : entries) {
if (entry.getMatcher().consume(path, 0) >= 0) {
return entry;
}
}
return null;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
package datadog.trace.civisibility.codeowners;

import datadog.trace.civisibility.codeowners.matcher.Matcher;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashSet;
import javax.annotation.Nullable;

public class Entry {

private final Matcher matcher;
private final Collection<String> owners;
private final boolean exclusion;
private final @Nullable String indexKey;

public Entry(Matcher matcher, Collection<String> owners, boolean exclusion) {
public Entry(
Matcher matcher, Collection<String> owners, boolean exclusion, @Nullable String indexKey) {
this.matcher = matcher;
this.owners = owners;
this.owners = owners.size() > 1 ? new ArrayList<>(new LinkedHashSet<>(owners)) : owners;
this.exclusion = exclusion;
this.indexKey = indexKey;
}

public Matcher getMatcher() {
Expand All @@ -26,4 +32,8 @@ public Collection<String> getOwners() {
public boolean isExclusion() {
return exclusion;
}

public @Nullable String getIndexKey() {
return indexKey;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,13 @@ public EntryBuilder(CharacterMatcher.Factory characterMatcherFactory, String s)
offset++;
}

String indexKey = parseIndexKey();
Matcher matcher = parseMatcher();
Collection<String> owners = exclusion ? Collections.emptyList() : parseOwners();
if (!exclusion && owners.isEmpty()) {
owners = sectionDefaultOwners;
}
return new Entry(matcher, owners, exclusion);
return new Entry(matcher, owners, exclusion, indexKey);

} catch (Exception e) {
log.warn("Skipping malformed CODEOWNERS entry: {}", new String(c), e);
Expand Down Expand Up @@ -224,6 +225,38 @@ private Matcher parseMatcher() {
return new CompositeMatcher(characterMatchers.toArray(new Matcher[0]));
}

private @Nullable String parseIndexKey() {
// Index by the first two fixed path segments; patterns without a safe prefix stay linear.
int position = offset;
boolean patternContainsSlashes = c[position] == '/';
if (patternContainsSlashes) {
position++;
}
int prefixStart = position;
int firstSeparator = -1;
for (; position < c.length && !isPatternTerminator(c[position]); position++) {
char character = c[position];
if (character == '*' || character == '?' || character == '[') {
return null;
}
if (character == '\\') {
return null;
}
if (character == '/') {
patternContainsSlashes = true;
if (firstSeparator >= 0) {
return new String(c, prefixStart, position - prefixStart);
Comment on lines +247 to +248

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep trailing-slash rules out of the fixed-prefix index

In large sections where the index activates, a two-segment trailing-slash rule such as /services/api/ is bucketed under services/api, but the existing matcher still treats that rule as a plain prefix because no end-of-segment matcher is appended for trailing slashes. This makes behavior depend on file size: with 511 prior indexable entries services/api-v2/Test.java is matched by the linear scan, while with 512 entries the indexed lookup only checks the services/api-v2 bucket and returns no owner; exclusions have the same issue. Either keep these trailing-slash patterns in the fallback until the matcher semantics are tightened, or make the matcher and index agree.

Useful? React with 👍 / 👎.

}
firstSeparator = position;
}
}

if (!patternContainsSlashes || firstSeparator < 0 || firstSeparator == position - 1) {
return null;
}
return new String(c, prefixStart, position - prefixStart);
}

private boolean consumeDoubleAsterisk() {
int position = offset;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package datadog.trace.civisibility.codeowners;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;

/**
* Resolves entries by rule priority, switching large rule sets from linear scans to fixed-prefix
* buckets while retaining a fallback for patterns that cannot be indexed.
*/
final class EntryIndex {

private static final int MIN_INDEX_SIZE = 512;

private List<IndexedEntry> entries = new ArrayList<>();
private Map<String, List<IndexedEntry>> entriesByKey;
private List<IndexedEntry> unindexedEntries;
private int indexableEntryCount;

void add(Entry entry, int order) {
IndexedEntry indexedEntry = new IndexedEntry(entry, order);
if (entriesByKey != null) {
index(indexedEntry);
} else {
entries.add(indexedEntry);
if (entry.getIndexKey() != null) {
indexableEntryCount++;
}
if (entries.size() >= MIN_INDEX_SIZE && indexableEntryCount * 2 >= entries.size()) {
entriesByKey = new HashMap<>();
unindexedEntries = new ArrayList<>();
for (IndexedEntry existingEntry : entries) {
index(existingEntry);
}
entries = Collections.emptyList();
}
}
}

@Nullable
Entry find(String path) {
IndexedEntry entry = findIndexedEntry(path);
return entry != null ? entry.entry : null;
}

private @Nullable IndexedEntry findIndexedEntry(String path) {
if (entriesByKey == null) {
return findFirstMatch(entries, path);
}
IndexedEntry unindexedMatch = findFirstMatch(unindexedEntries, path);

int firstSeparator = path.indexOf('/');
if (firstSeparator < 0) {
return unindexedMatch;
}
int secondSeparator = path.indexOf('/', firstSeparator + 1);
int keyEnd = secondSeparator >= 0 ? secondSeparator : path.length();
List<IndexedEntry> indexedEntries = entriesByKey.get(path.substring(0, keyEnd));
IndexedEntry indexedMatch = findFirstMatch(indexedEntries, path);

if (unindexedMatch == null) {
return indexedMatch;
}
if (indexedMatch == null) {
return unindexedMatch;
}
return indexedMatch.order > unindexedMatch.order ? indexedMatch : unindexedMatch;
}

private void index(IndexedEntry indexedEntry) {
String indexKey = indexedEntry.entry.getIndexKey();
if (indexKey != null) {
entriesByKey.computeIfAbsent(indexKey, key -> new ArrayList<>()).add(indexedEntry);
} else {
unindexedEntries.add(indexedEntry);
}
}

private static @Nullable IndexedEntry findFirstMatch(
@Nullable List<IndexedEntry> entries, String path) {
if (entries != null) {
for (int i = entries.size() - 1; i >= 0; i--) {
IndexedEntry entry = entries.get(i);
if (entry.entry.getMatcher().consume(path, 0) >= 0) {
return entry;
}
}
}
return null;
}

private static final class IndexedEntry {

private final Entry entry;
private final int order;

private IndexedEntry(Entry entry, int order) {
this.entry = entry;
this.order = order;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package datadog.trace.civisibility.codeowners;

import javax.annotation.Nullable;

/** Groups ownership and exclusion rules for a CODEOWNERS section while preserving rule order. */
final class Section {

private final EntryIndex entries = new EntryIndex();
private final EntryIndex exclusions = new EntryIndex();
private int entryOrder;

void add(Entry entry) {
if (entry.isExclusion()) {
exclusions.add(entry, entryOrder++);
} else {
entries.add(entry, entryOrder++);
}
}

boolean isExcluded(String path) {
return exclusions.find(path) != null;
}

@Nullable
Entry findMatchingEntry(String path) {
return entries.find(path);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ public class AsteriskMatcher implements Matcher {
private AsteriskMatcher() {}

@Override
public int consume(char[] line, int offset) {
return offset < line.length && line[offset] != '/' ? 1 : -1;
public int consume(String line, int offset) {
return offset < line.length() && line.charAt(offset) != '/' ? 1 : -1;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ private CharacterMatcher(char character) {
}

@Override
public int consume(char[] line, int offset) {
return offset < line.length && line[offset] == character ? 1 : -1;
public int consume(String line, int offset) {
return offset < line.length() && line.charAt(offset) == character ? 1 : -1;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ public CompositeMatcher(Matcher[] delegates) {
}

@Override
public int consume(char[] line, int offset) {
public int consume(String line, int offset) {
return consume(line, offset, 0);
}

private int consume(char[] line, int offset, int matcherOffset) {
private int consume(String line, int offset, int matcherOffset) {
int position = offset;
while (matcherOffset < delegates.length) {
Matcher delegate = delegates[matcherOffset];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ public class DoubleAsteriskMatcher implements Matcher {
private DoubleAsteriskMatcher() {}

@Override
public int consume(char[] line, int offset) {
if (offset == line.length) {
public int consume(String line, int offset) {
if (offset == line.length()) {
return -1;
}

int position = offset;
while (position < line.length && line[position++] != '/') {}
while (position < line.length() && line.charAt(position++) != '/') {}
return position - offset;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ public class EndOfLineMatcher implements Matcher {
public static final Matcher INSTANCE = new EndOfLineMatcher();

@Override
public int consume(char[] line, int offset) {
return offset == line.length ? 0 : -1;
public int consume(String line, int offset) {
return offset == line.length() ? 0 : -1;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ public class EndOfSegmentMatcher implements Matcher {
public static final Matcher INSTANCE = new EndOfSegmentMatcher();

@Override
public int consume(char[] line, int offset) {
return offset == line.length || line[offset] == '/' ? 0 : -1;
public int consume(String line, int offset) {
return offset == line.length() || line.charAt(offset) == '/' ? 0 : -1;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ public interface Matcher {
* @return the number of characters matched from the line starting with the offset. Negative value
* means matching failed
*/
int consume(char[] line, int offset);
int consume(String line, int offset);

/**
* @return {@code true} if this matcher can be used [0..*] times. {@code false} if the matcher
Expand Down
Loading
Loading