-
Notifications
You must be signed in to change notification settings - Fork 260
Feature/testing visibility #3169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/testing_visibility
Are you sure you want to change the base?
Feature/testing visibility #3169
Conversation
Bson filter = Filters.and( | ||
Filters.eq("testRunResultSummaryId", testingRunResultSummaryId), | ||
Filters.eq("vulnerable", false), | ||
Filters.exists("apiErrors." + key, true) | ||
); | ||
|
||
long count = TestingRunResultDao.instance.getRawCollection().countDocuments(filter); | ||
return count > 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace this with findOne.
The point here is to check if it has any apiErrors
key, if it has one, it likely will have more as well.
Also, we don't need to check for specific keys, "apiErrors." + key
checking just the top level opject apiErrors
would work
|
||
MongoCursor<BasicDBObject> cursor = TestingRunResultDao.instance.getMCollection() | ||
.aggregate(pipeline, BasicDBObject.class).cursor(); | ||
MongoCursor<Document> cursor = TestingRunResultDao.instance.getRawCollection() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we using rawCollection ?
MCollection.createIndexIfAbsent(getDBName(), getCollName(), fieldNames, false); | ||
|
||
// Index on apiErrors map to speed up existence and aggregation queries | ||
MCollection.createIndexIfAbsent(getDBName(), getCollName(), new String[]{TestingRunResult.API_ERRORS}, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make this index composite. i.e. in the future we should be able to extend it.
and make it more useful [ brings the least no. of documents required ]
I think if we add summaryId
and vulnerable
field to it, along as well, should be good.
check for usage of index in all cases.
@BsonIgnore | ||
private String testRunResultSummaryHexId; | ||
@BsonIgnore | ||
private List<TestResult> singleTestResults; | ||
@BsonIgnore | ||
private List<MultiExecTestResult> multiExecTestResults; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need these ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements feature testing visibility by adding API error tracking capabilities to the testing framework. The changes enhance the TestingRunResult class with new fields and methods for better visibility into API errors during testing, along with improved aggregation queries for error statistics.
Key changes include:
- Added API error tracking fields and methods to TestingRunResult
- Improved database indexing for API error queries in TestingRunResultDao
- Fixed test code to use proper MongoDB Filters and Updates APIs
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
TestingRunResult.java | Added apiErrors map field, hex ID getters, and new BsonIgnore fields for better testing visibility |
TestingRunResultDao.java | Added database index for apiErrors field to optimize aggregation queries |
TestResultsStatsActionTest.java | Fixed test method to use proper MongoDB Filters instead of raw Document objects |
TestResultsStatsAction.java | Improved aggregation methods to use proper MongoDB APIs and better error handling |
Comments suppressed due to low confidence (1)
apps/dashboard/src/main/java/com/akto/action/testing/TestResultsStatsAction.java:1
- This line appears to be removed but the comment remains on the next line, creating a mismatch. Either restore the line or remove the corresponding comment.
package com.akto.action.testing;
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
} | ||
|
||
public String getTestRunResultSummaryHexId() { | ||
if (testRunResultSummaryHexId == null) return this.testRunResultSummaryId.toHexString(); |
Copilot
AI
Sep 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null pointer exception if testRunResultSummaryId is null. Add null check before calling toHexString().
if (testRunResultSummaryHexId == null) return this.testRunResultSummaryId.toHexString(); | |
if (testRunResultSummaryHexId == null) { | |
return this.testRunResultSummaryId != null ? this.testRunResultSummaryId.toHexString() : null; | |
} |
Copilot uses AI. Check for mistakes.
No description provided.