Skip to content

Commit 336a40a

Browse files
fix: address PR review blockers — remove cache endpoint, fix casts, revert ThreadLocal
Review fixes for PR #3: BLOCKER 1: Remove POST /api/cache/invalidate endpoint — violates read-only serving architecture and has no auth (DoS vector via cache stampede). Cache invalidation now only happens on server restart. BLOCKER 2: Add blank-string filter to countDistinctFiles() Cypher query (AND n.filePath <> '') to match the Java-side behavior it replaced. MUST FIX: Revert PARSER.remove() deletion in AbstractJavaParserDetector — the finally block is the correct ThreadLocal cleanup pattern to prevent memory leaks with pooled threads. MEDIUM: Replace unchecked (String) casts with String.valueOf() in computeLanguageStats, computeFrameworkStats, computeAuthStats, computeConnectionStats, and computeInfraStats for robustness. LOW: Read node_count/edge_count from already-computed graph sub-map in QueryService.getStats() instead of re-querying graphStore.count() and countEdges(). All 1395 tests pass (0 failures, 0 errors). Co-Authored-By: Paperclip <noreply@paperclip.ing>
1 parent f17eeba commit 336a40a

6 files changed

Lines changed: 27 additions & 38 deletions

File tree

src/main/java/io/github/randomcodespace/iq/api/GraphController.java

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import io.github.randomcodespace.iq.config.CodeIqConfig;
44
import io.github.randomcodespace.iq.query.QueryService;
5-
import org.springframework.cache.CacheManager;
65
import org.springframework.http.HttpStatus;
76
import org.springframework.http.MediaType;
87
import org.springframework.http.ResponseEntity;
@@ -32,14 +31,11 @@ public class GraphController {
3231

3332
private final QueryService queryService;
3433
private final CodeIqConfig config;
35-
private final CacheManager cacheManager;
3634

3735
public GraphController(@org.springframework.beans.factory.annotation.Autowired(required = false) QueryService queryService,
38-
CodeIqConfig config,
39-
@org.springframework.beans.factory.annotation.Autowired(required = false) CacheManager cacheManager) {
36+
CodeIqConfig config) {
4037
this.queryService = queryService;
4138
this.config = config;
42-
this.cacheManager = cacheManager;
4339
}
4440

4541
@GetMapping("/stats")
@@ -248,21 +244,6 @@ public ResponseEntity<String> readFile(
248244
}
249245
}
250246

251-
@PostMapping("/cache/invalidate")
252-
public Map<String, Object> invalidateCache() {
253-
int cleared = 0;
254-
if (cacheManager != null) {
255-
for (String name : cacheManager.getCacheNames()) {
256-
var cache = cacheManager.getCache(name);
257-
if (cache != null) {
258-
cache.clear();
259-
cleared++;
260-
}
261-
}
262-
}
263-
return Map.of("status", "ok", "caches_cleared", cleared);
264-
}
265-
266247
// POST /api/analyze removed — API/MCP server is read-only.
267248
// Analysis is done locally via CLI: code-iq analyze / code-iq index
268249
// Data is loaded into Neo4j on serve startup (auto-enrich).

src/main/java/io/github/randomcodespace/iq/detector/java/AbstractJavaParserDetector.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ protected Optional<CompilationUnit> parse(DetectorContext ctx) {
2929
// JavaParser may throw AssertionError for unrecognized token kinds
3030
// (e.g. newer Java syntax). Fall back to regex in those cases.
3131
return Optional.empty();
32+
} finally {
33+
PARSER.remove();
3234
}
3335
}
3436

src/main/java/io/github/randomcodespace/iq/graph/GraphStore.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ public long countEdges() {
402402
public long countDistinctFiles() {
403403
try (Transaction tx = graphDb.beginTx()) {
404404
var result = tx.execute(
405-
"MATCH (n:CodeNode) WHERE n.filePath IS NOT NULL "
405+
"MATCH (n:CodeNode) WHERE n.filePath IS NOT NULL AND n.filePath <> '' "
406406
+ "RETURN count(DISTINCT n.filePath) AS cnt");
407407
if (result.hasNext()) {
408408
return ((Number) result.next().get("cnt")).longValue();
@@ -699,7 +699,7 @@ private Map<String, Object> computeLanguageStats() {
699699
+ "RETURN toLower(n.prop_language) AS lang, count(n) AS cnt");
700700
while (result.hasNext()) {
701701
var row = result.next();
702-
String lang = ((String) row.get("lang")).trim();
702+
String lang = String.valueOf(row.get("lang")).trim();
703703
if (!lang.isBlank()) {
704704
langCounts.merge(lang, ((Number) row.get("cnt")).longValue(), Long::sum);
705705
}
@@ -725,7 +725,7 @@ private Map<String, Object> computeFrameworkStats() {
725725
+ "RETURN n.prop_framework AS fw, count(n) AS cnt");
726726
while (result.hasNext()) {
727727
var row = result.next();
728-
String fw = ((String) row.get("fw")).trim();
728+
String fw = String.valueOf(row.get("fw")).trim();
729729
if (!fw.isBlank()) {
730730
fwCounts.merge(fw, ((Number) row.get("cnt")).longValue(), Long::sum);
731731
}
@@ -759,7 +759,7 @@ private Map<String, Object> computeInfraStats() {
759759
+ "RETURN coalesce(n.prop_protocol, n.label, 'unknown') AS protocol, count(n) AS cnt");
760760
while (result.hasNext()) {
761761
var row = result.next();
762-
messaging.merge((String) row.get("protocol"), ((Number) row.get("cnt")).longValue(), Long::sum);
762+
messaging.merge(String.valueOf(row.get("protocol")), ((Number) row.get("cnt")).longValue(), Long::sum);
763763
}
764764
}
765765
infra.put("messaging", sortByValueDesc(messaging));
@@ -771,7 +771,7 @@ private Map<String, Object> computeInfraStats() {
771771
+ "RETURN coalesce(n.prop_resource_type, n.label, 'unknown') AS resType, count(n) AS cnt");
772772
while (result.hasNext()) {
773773
var row = result.next();
774-
cloud.merge((String) row.get("resType"), ((Number) row.get("cnt")).longValue(), Long::sum);
774+
cloud.merge(String.valueOf(row.get("resType")), ((Number) row.get("cnt")).longValue(), Long::sum);
775775
}
776776
}
777777
infra.put("cloud", sortByValueDesc(cloud));
@@ -789,7 +789,7 @@ private Map<String, Object> computeConnectionStats() {
789789
Map<String, Long> restByMethod = new TreeMap<>();
790790
while (restResult.hasNext()) {
791791
var row = restResult.next();
792-
restByMethod.put((String) row.get("method"), ((Number) row.get("cnt")).longValue());
792+
restByMethod.put(String.valueOf(row.get("method")), ((Number) row.get("cnt")).longValue());
793793
}
794794
long restTotal = restByMethod.values().stream().mapToLong(Long::longValue).sum();
795795
Map<String, Object> rest = new LinkedHashMap<>();
@@ -824,7 +824,7 @@ private Map<String, Object> computeAuthStats() {
824824
+ "RETURN n.prop_auth_type AS authType, count(n) AS cnt");
825825
while (guardResult.hasNext()) {
826826
var row = guardResult.next();
827-
String authType = ((String) row.get("authType")).trim();
827+
String authType = String.valueOf(row.get("authType")).trim();
828828
if (!authType.isBlank()) {
829829
authCounts.merge(authType, ((Number) row.get("cnt")).longValue(), Long::sum);
830830
}
@@ -834,7 +834,7 @@ private Map<String, Object> computeAuthStats() {
834834
+ "RETURN n.prop_framework AS fw, count(n) AS cnt");
835835
while (fwResult.hasNext()) {
836836
var row = fwResult.next();
837-
String fw = ((String) row.get("fw")).trim();
837+
String fw = String.valueOf(row.get("fw")).trim();
838838
String authType = fw.substring("auth:".length()).trim();
839839
if (!authType.isEmpty()) {
840840
authCounts.merge(authType, ((Number) row.get("cnt")).longValue(), Long::sum);

src/main/java/io/github/randomcodespace/iq/query/QueryService.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,16 @@ public Map<String, Object> getStats() {
4646
nodesByLayer.put((String) row.get("layer"), ((Number) row.get("cnt")).longValue());
4747
}
4848

49-
result.put("node_count", graphStore.count());
50-
result.put("edge_count", graphStore.countEdges());
49+
// Read from already-computed graph sub-map instead of re-querying
50+
@SuppressWarnings("unchecked")
51+
Map<String, Object> graphStats = (Map<String, Object>) result.get("graph");
52+
if (graphStats != null) {
53+
result.put("node_count", graphStats.get("nodes"));
54+
result.put("edge_count", graphStats.get("edges"));
55+
} else {
56+
result.put("node_count", graphStore.count());
57+
result.put("edge_count", graphStore.countEdges());
58+
}
5159
result.put("nodes_by_kind", nodesByKind);
5260
result.put("nodes_by_layer", nodesByLayer);
5361
return result;

src/test/java/io/github/randomcodespace/iq/api/GraphControllerTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ void setUp() {
4545
config.setMaxDepth(10);
4646
config.setMaxRadius(10);
4747
config.setRootPath(".");
48-
var controller = new GraphController(queryService, config, null);
48+
var controller = new GraphController(queryService, config);
4949
mockMvc = MockMvcBuilders.standaloneSetup(controller).build();
5050
}
5151

@@ -416,7 +416,7 @@ void searchGraphShouldReturnResults() throws Exception {
416416
void readFileShouldReturnContent(@TempDir Path tempDir) throws Exception {
417417
Files.writeString(tempDir.resolve("hello.txt"), "Hello World", StandardCharsets.UTF_8);
418418
config.setRootPath(tempDir.toAbsolutePath().toString());
419-
var controller = new GraphController(queryService, config, null);
419+
var controller = new GraphController(queryService, config);
420420
var fileMvc = MockMvcBuilders.standaloneSetup(controller).build();
421421

422422
fileMvc.perform(get("/api/file").param("path", "hello.txt"))
@@ -427,7 +427,7 @@ void readFileShouldReturnContent(@TempDir Path tempDir) throws Exception {
427427
@Test
428428
void readFileShouldReturn404ForMissing(@TempDir Path tempDir) throws Exception {
429429
config.setRootPath(tempDir.toAbsolutePath().toString());
430-
var controller = new GraphController(queryService, config, null);
430+
var controller = new GraphController(queryService, config);
431431
var fileMvc = MockMvcBuilders.standaloneSetup(controller).build();
432432

433433
fileMvc.perform(get("/api/file").param("path", "nonexistent.txt"))
@@ -437,7 +437,7 @@ void readFileShouldReturn404ForMissing(@TempDir Path tempDir) throws Exception {
437437
@Test
438438
void readFileShouldBlockPathTraversal(@TempDir Path tempDir) throws Exception {
439439
config.setRootPath(tempDir.toAbsolutePath().toString());
440-
var controller = new GraphController(queryService, config, null);
440+
var controller = new GraphController(queryService, config);
441441
var fileMvc = MockMvcBuilders.standaloneSetup(controller).build();
442442

443443
fileMvc.perform(get("/api/file").param("path", "../../../etc/passwd"))
@@ -450,7 +450,7 @@ void readFileShouldReturnLineRange(@TempDir Path tempDir) throws Exception {
450450
Files.writeString(tempDir.resolve("multi.txt"), "line1\nline2\nline3\nline4\nline5",
451451
StandardCharsets.UTF_8);
452452
config.setRootPath(tempDir.toAbsolutePath().toString());
453-
var controller = new GraphController(queryService, config, null);
453+
var controller = new GraphController(queryService, config);
454454
var fileMvc = MockMvcBuilders.standaloneSetup(controller).build();
455455

456456
fileMvc.perform(get("/api/file")
@@ -465,7 +465,7 @@ void readFileShouldReturnLineRange(@TempDir Path tempDir) throws Exception {
465465
void readFileShouldReturnFullContentWithoutLineParams(@TempDir Path tempDir) throws Exception {
466466
Files.writeString(tempDir.resolve("full.txt"), "aaa\nbbb\nccc", StandardCharsets.UTF_8);
467467
config.setRootPath(tempDir.toAbsolutePath().toString());
468-
var controller = new GraphController(queryService, config, null);
468+
var controller = new GraphController(queryService, config);
469469
var fileMvc = MockMvcBuilders.standaloneSetup(controller).build();
470470

471471
fileMvc.perform(get("/api/file").param("path", "full.txt"))

src/test/java/io/github/randomcodespace/iq/query/QueryServiceTest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,6 @@ void getStatsShouldReturnNodeAndEdgeCounts() {
8080
aggregateStats.put("architecture", Map.of("classes", 1L));
8181

8282
when(graphStore.computeAggregateStats()).thenReturn(aggregateStats);
83-
when(graphStore.count()).thenReturn(2L);
84-
when(graphStore.countEdges()).thenReturn(1L);
8583
when(graphStore.countNodesByKind()).thenReturn(List.of(
8684
Map.of("kind", "endpoint", "cnt", 1L),
8785
Map.of("kind", "class", "cnt", 1L)));

0 commit comments

Comments
 (0)