Skip to content

Commit 145666f

Browse files
FelixTJDietrichFelixTJDietrich
authored
feat(application-server): prevent team filtering mismatches (#400)
1 parent 995c900 commit 145666f

3 files changed

Lines changed: 191 additions & 36 deletions

File tree

‎server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/team/TeamRepository.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package de.tum.in.www1.hephaestus.gitprovider.team;
22

3+
import java.util.List;
34
import org.springframework.data.jpa.repository.JpaRepository;
45
import org.springframework.stereotype.Repository;
56

67
@Repository
7-
public interface TeamRepository extends JpaRepository<Team, Long> {}
8+
public interface TeamRepository extends JpaRepository<Team, Long> {
9+
List<Team> findAllByName(String name);
10+
}

‎server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java‎

Lines changed: 161 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,14 @@
1414
import jakarta.transaction.Transactional;
1515
import java.time.Instant;
1616
import java.util.Comparator;
17+
import java.util.HashMap;
18+
import java.util.HashSet;
1719
import java.util.List;
1820
import java.util.Map;
21+
import java.util.Collection;
22+
import java.util.Objects;
1923
import java.util.Optional;
24+
import java.util.Set;
2025
import java.util.stream.Collectors;
2126
import java.util.stream.IntStream;
2227
import org.slf4j.Logger;
@@ -128,9 +133,7 @@ public List<LeaderboardEntryDTO> createLeaderboard(
128133
Optional<String> team,
129134
Optional<LeaderboardSortType> sort
130135
) {
131-
Optional<Team> teamEntity = team
132-
.map(t -> teamRepository.findAll().stream().filter(tm -> t.equals(tm.getName())).findFirst())
133-
.orElse(Optional.empty());
136+
Optional<Team> teamEntity = team.flatMap(this::findTeamByPath);
134137
logger.info(
135138
"Creating leaderboard dataset with timeframe: {} - {} and team: {}",
136139
after,
@@ -273,6 +276,161 @@ public List<LeaderboardEntryDTO> createLeaderboard(
273276
return leaderboard;
274277
}
275278

279+
private Optional<Team> findTeamByPath(String path) {
280+
if (path == null || path.trim().isEmpty()) {
281+
return Optional.empty();
282+
}
283+
String[] parts = path.split(" / ");
284+
285+
// Start from leaf name candidates
286+
String leaf = parts[parts.length - 1];
287+
List<Team> candidates = teamRepository.findAllByName(leaf);
288+
if (candidates.isEmpty()) {
289+
return Optional.empty();
290+
}
291+
if (parts.length == 1) {
292+
// If only the leaf name is given, accept only if unique to avoid ambiguity
293+
return candidates.size() == 1 ? Optional.of(candidates.get(0)) : Optional.empty();
294+
}
295+
296+
// Cache teams by id to minimize DB round-trips
297+
Map<Long, Team> cache = candidates.stream().collect(Collectors.toMap(Team::getId, t -> t));
298+
299+
// Working set: map candidateId -> current node (starts at candidate)
300+
Map<Long, Team> currentByCandidate = candidates.stream().collect(Collectors.toMap(Team::getId, t -> t));
301+
302+
// Walk up visible ancestors breadth-first over candidates, eliminating mismatches per segment
303+
for (int index = parts.length - 2; index >= 0 && currentByCandidate.size() > 1; index--) {
304+
String expected = parts[index];
305+
306+
// Resolve each candidate's next VISIBLE parent in batched fashion
307+
Map<Long, Team> nextVisibleByCandidate = new HashMap<>();
308+
309+
// We may need multiple fetch rounds if chains include hidden ancestors
310+
boolean pendingResolution = true;
311+
while (pendingResolution) {
312+
pendingResolution = false;
313+
Set<Long> missingIds = new HashSet<>();
314+
315+
for (Map.Entry<Long, Team> entry : currentByCandidate.entrySet()) {
316+
Long candidateId = entry.getKey();
317+
Team cursor = entry.getValue();
318+
if (nextVisibleByCandidate.containsKey(candidateId)) {
319+
continue; // already resolved
320+
}
321+
322+
Long parentId = cursor.getParentId();
323+
while (parentId != null) {
324+
Team parent = cache.get(parentId);
325+
if (parent == null) {
326+
missingIds.add(parentId);
327+
break; // fetch in batch first
328+
}
329+
if (!parent.isHidden()) {
330+
nextVisibleByCandidate.put(candidateId, parent);
331+
break;
332+
}
333+
parentId = parent.getParentId();
334+
}
335+
if (parentId == null && !nextVisibleByCandidate.containsKey(candidateId)) {
336+
// No more visible parent
337+
nextVisibleByCandidate.put(candidateId, null);
338+
}
339+
}
340+
341+
if (!missingIds.isEmpty()) {
342+
// Batch fetch all missing parents
343+
teamRepository.findAllById(missingIds).forEach(t -> cache.put(t.getId(), t));
344+
pendingResolution = true; // try resolving again with filled cache
345+
}
346+
}
347+
348+
// Eliminate candidates whose next visible parent does not match the expected segment
349+
Map<Long, Team> filtered = new HashMap<>();
350+
for (Map.Entry<Long, Team> entry : currentByCandidate.entrySet()) {
351+
Long candidateId = entry.getKey();
352+
Team nextVisible = nextVisibleByCandidate.get(candidateId);
353+
if (nextVisible != null && expected.equals(nextVisible.getName())) {
354+
filtered.put(candidateId, nextVisible);
355+
}
356+
}
357+
currentByCandidate = filtered;
358+
359+
if (currentByCandidate.isEmpty()) {
360+
return Optional.empty();
361+
}
362+
if (currentByCandidate.size() == 1) {
363+
// Early exit as soon as unique
364+
Long onlyId = currentByCandidate.keySet().iterator().next();
365+
return Optional.of(cache.get(onlyId));
366+
}
367+
}
368+
369+
// If we consumed all parts and still have multiple, attempt exact full visible path match; otherwise pick first.
370+
if (currentByCandidate.size() > 1) {
371+
preloadAncestors(currentByCandidate.values(), cache);
372+
for (Long id : currentByCandidate.keySet()) {
373+
Team t = cache.get(id);
374+
if (t != null && equalsVisiblePath(t, parts, cache)) {
375+
return Optional.of(t);
376+
}
377+
}
378+
logger.warn(
379+
"Ambiguous team path '{}' resolved to multiple candidates; picking first.",
380+
sanitizeForLog(path)
381+
);
382+
}
383+
384+
Long anyId = currentByCandidate.keySet().iterator().next();
385+
return Optional.ofNullable(cache.get(anyId));
386+
}
387+
388+
private boolean equalsVisiblePath(Team team, String[] parts, Map<Long, Team> cache) {
389+
int index = parts.length - 1;
390+
Team current = team;
391+
while (current != null) {
392+
if (!current.isHidden()) {
393+
if (index < 0 || !Objects.equals(parts[index], current.getName())) {
394+
return false;
395+
}
396+
index--;
397+
}
398+
Long parentId = current.getParentId();
399+
current = parentId != null ? cache.get(parentId) : null;
400+
}
401+
return index < 0;
402+
}
403+
404+
private void preloadAncestors(Collection<Team> teams, Map<Long, Team> cache) {
405+
Set<Long> pending = teams
406+
.stream()
407+
.map(Team::getParentId)
408+
.filter(Objects::nonNull)
409+
.filter(id -> !cache.containsKey(id))
410+
.collect(Collectors.toSet());
411+
412+
while (!pending.isEmpty()) {
413+
Set<Long> nextRound = new HashSet<>();
414+
teamRepository
415+
.findAllById(pending)
416+
.forEach(parent -> {
417+
cache.putIfAbsent(parent.getId(), parent);
418+
Long ancestorId = parent.getParentId();
419+
if (ancestorId != null && !cache.containsKey(ancestorId)) {
420+
nextRound.add(ancestorId);
421+
}
422+
});
423+
pending = nextRound;
424+
}
425+
}
426+
427+
private String sanitizeForLog(String input) {
428+
if (input == null) {
429+
return null;
430+
}
431+
return input.replaceAll("[\\r\\n]", "");
432+
}
433+
276434
private int calculateTotalScore(List<PullRequestReview> reviews, List<IssueComment> issueComments) {
277435
int numberOfIssueComments = issueComments
278436
.stream()

‎webapp/src/routes/_authenticated/index.tsx‎

Lines changed: 26 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
} from "@tanstack/react-router";
77
import { zodValidator } from "@tanstack/zod-adapter";
88
import { endOfISOWeek, formatISO, startOfISOWeek } from "date-fns";
9-
import { useEffect, useMemo } from "react";
9+
import { useEffect } from "react";
1010
import { z } from "zod";
1111
import {
1212
getLeaderboardOptions,
@@ -90,39 +90,33 @@ function LeaderboardContainer() {
9090
parentId?: number;
9191
hidden?: boolean;
9292
};
93-
const visibleTeams = useMemo<string[]>(() => {
94-
const teams = (metaQuery.data?.teams ?? []) as MetaTeam[];
95-
return teams.filter((t) => !t.hidden).map((t) => t.name);
96-
}, [metaQuery.data]);
9793

9894
// Build a map for id->team to compute visible-only path labels
99-
const teamById = useMemo(() => {
100-
const teams = (metaQuery.data?.teams ?? []) as MetaTeam[];
101-
const m = new Map<number, MetaTeam>();
102-
teams.forEach((t) => m.set(t.id, t));
103-
return m;
104-
}, [metaQuery.data]);
105-
106-
const teamOptions = useMemo(() => {
107-
const teams = (metaQuery.data?.teams ?? []) as MetaTeam[];
108-
const visible = teams.filter((t) => !t.hidden);
109-
const makeLabel = (t: MetaTeam): string => {
110-
const names: string[] = [];
111-
// Walk up the ancestry, but only include visible ancestors
112-
let cur: MetaTeam | undefined = t;
113-
while (cur) {
114-
if (!cur.hidden) names.push(cur.name);
115-
const parent: MetaTeam | undefined =
116-
cur.parentId !== undefined ? teamById.get(cur.parentId) : undefined;
117-
cur = parent;
118-
}
119-
// names currently has child->...->root, reverse to root->child
120-
return names.reverse().join(" / ");
121-
};
122-
return visible
123-
.map((t) => ({ value: t.name, label: makeLabel(t) }))
124-
.sort((a, b) => a.label.localeCompare(b.label));
125-
}, [metaQuery.data, teamById]);
95+
const teamsList = (metaQuery.data?.teams ?? []) as MetaTeam[];
96+
const teamById = new Map<number, MetaTeam>(teamsList.map((t) => [t.id, t]));
97+
98+
// Helper to create the visible-only path label for a team
99+
const makeLabel = (t: MetaTeam): string => {
100+
const names: string[] = [];
101+
let cur: MetaTeam | undefined = t;
102+
while (cur) {
103+
if (!cur.hidden) names.push(cur.name);
104+
const parent: MetaTeam | undefined =
105+
cur.parentId !== undefined ? teamById.get(cur.parentId) : undefined;
106+
cur = parent;
107+
}
108+
return names.reverse().join(" / ");
109+
};
110+
111+
// Valid selectable values are the full visible paths
112+
const visibleTeams = teamsList
113+
.filter((t) => !t.hidden)
114+
.map((t) => makeLabel(t));
115+
116+
const teamOptions = teamsList
117+
.filter((t) => !t.hidden)
118+
.map((t) => ({ value: makeLabel(t), label: makeLabel(t) }))
119+
.sort((a, b) => a.label.localeCompare(b.label));
126120

127121
// If current selected team is hidden (or no longer present), reset to 'all'
128122
useEffect(() => {

0 commit comments

Comments
 (0)