Skip to content

Commit c90e3e8

Browse files
joschlijoschli
committed
new navigation logic to allow access outside of review scope
1 parent 9fc05c0 commit c90e3e8

2 files changed

Lines changed: 146 additions & 79 deletions

File tree

‎mubench.reviewsite/src/Controllers/ReviewsController.php‎

Lines changed: 95 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use MuBench\ReviewSite\Models\Misuse;
1111
use MuBench\ReviewSite\Models\Review;
1212
use MuBench\ReviewSite\Models\Reviewer;
13-
use MuBench\ReviewSite\Models\ReviewState;
1413
use MuBench\ReviewSite\Models\Run;
1514
use MuBench\ReviewSite\Models\Tag;
1615
use MuBench\ReviewSite\Models\Violation;
@@ -39,26 +38,10 @@ public function getReview(Request $request, Response $response, array $args)
3938
$resolution_reviewer = Reviewer::where(['name' => 'resolution'])->first();
4039
$is_reviewer = ($this->user && $reviewer && $this->user->id == $reviewer->id) || ($reviewer && $reviewer->id == $resolution_reviewer->id);
4140

42-
$runs = RunsController::getRuns($detector, $experiment, $ex2_review_size);
43-
44-
$limited_all_misuses = $this->collectAllMisuses($runs);
41+
$runs = Run::of($detector)->in($experiment)->orderBy('project_muid')->orderBy('version_muid')->get();
4542

4643
list($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse) =
47-
$this->determineNavigationTargets($limited_all_misuses, $project_muid, $version_muid, $misuse_muid, $reviewer);
48-
49-
if(!$misuse){
50-
$runs = Run::of($detector)->in($experiment)->orderBy('project_muid')->orderBy('version_muid')->get();
51-
52-
$all_misuses = $this->collectAllMisuses($runs);
53-
54-
list($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse) =
55-
$this->determineNavigationTargets($all_misuses, $project_muid, $version_muid, $misuse_muid, $reviewer);
56-
57-
$next_misuse = $all_misuses->first();
58-
if($limited_all_misuses->where('id', $next_reviewable_misuse->id)->isEmpty()){
59-
$next_reviewable_misuse = null;
60-
}
61-
}
44+
$this->determineNavigationTargets($runs, $experiment, $project_muid, $version_muid, $misuse_muid, $reviewer, $ex2_review_size);
6245

6346
$all_violations = Violation::all();
6447
$all_tags = Tag::all()->sortBy('name');
@@ -165,52 +148,115 @@ public function updateOrCreateReview($misuse_id, $reviewer_id, $comment, $findin
165148
}
166149
}
167150

168-
private function collectAllMisuses($runs)
169-
{
170-
$all_misuses = new Collection;
171-
foreach ($runs as $run) {
172-
$all_misuses = $all_misuses->merge($run->misuses);
173-
}
174-
return $all_misuses;
175-
}
176-
177-
function determineNavigationTargets(Collection $all_misuses, $project_muid, $version_muid, $misuse_muid, $reviewer)
151+
function determineNavigationTargets(Collection $runs, $experiment, $project_muid, $version_muid, $misuse_muid, $reviewer, $ex2_review_size)
178152
{
179-
$previous_misuse = $all_misuses->last();
153+
$previous_misuse = NULL;
180154
$misuse = NULL;
181155
$next_misuse = NULL;
182156
$next_reviewable_misuse = NULL;
157+
$last_run = $runs->last();
183158

184-
foreach ($all_misuses as $current_misuse) { /** @var Misuse $current_misuse */
185-
if (!$misuse && $current_misuse->getProject() === $project_muid
186-
&& $current_misuse->getVersion() === $version_muid
187-
&& $current_misuse->misuse_muid === $misuse_muid) {
188-
$misuse = $current_misuse;
189-
} else {
190-
if ($misuse && !$next_misuse) {
191-
$next_misuse = $current_misuse;
159+
foreach ($runs as $run) {
160+
if ($misuse && !$next_misuse) {
161+
$next_misuse = $run->getMisuses($experiment, $ex2_review_size)->first();
162+
}
163+
if (!$misuse && $run->project_muid == $project_muid && $run->version_muid == $version_muid) {
164+
$this->findMisuse($run->getMisuses($experiment, $ex2_review_size), $misuse_muid, $reviewer, $previous_misuse, $misuse, $next_misuse, $next_reviewable_misuse);
165+
if($misuse && !$previous_misuse){
166+
$previous_misuse = $last_run->getMisuses($experiment, $ex2_review_size)->last();
192167
}
193-
$is_current_misuse_reviewable = $current_misuse->getReviewState() === ReviewState::NEEDS_REVIEW
194-
&& !$current_misuse->hasReviewed($reviewer);
195-
$first_reviewable_misuse = !$next_reviewable_misuse && $is_current_misuse_reviewable;
196-
$first_reviewable_misuse_after = $misuse && $is_current_misuse_reviewable;
197-
if ($first_reviewable_misuse || $first_reviewable_misuse_after) {
198-
$next_reviewable_misuse = $current_misuse;
199-
if ($misuse) {
200-
break;
168+
if(!$misuse){
169+
// search for misuse out of review scope
170+
foreach($run->misuses as $current_misuse){
171+
if($current_misuse->misuse_muid == $misuse_muid){
172+
$misuse = $current_misuse;
173+
break;
174+
}
175+
$previous_misuse = $current_misuse;
201176
}
202177
}
178+
} elseif ($misuse && !$next_reviewable_misuse) {
179+
$next_reviewable_misuse = $this->findNextReviewableMisuse($run->getMisuses($experiment, $ex2_review_size), $reviewer);
180+
}
181+
182+
if ($next_misuse && $next_reviewable_misuse) {
183+
break;
184+
}
185+
$last_run = $run;
186+
}
187+
if ($misuse && !$next_misuse) {
188+
$next_misuse = $runs->first()->getMisuses($experiment, $ex2_review_size)->first();
189+
}
190+
if(!$next_reviewable_misuse){
191+
$next_reviewable_misuse = $this->findNextReviewableBeforeMisuse($runs, $experiment, $project_muid, $version_muid, $misuse, $reviewer, $ex2_review_size);
192+
}
193+
if(!$previous_misuse){
194+
$previous_misuse = $this->findPreviousMisuse($runs, $misuse);
195+
}
196+
return array($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse);
197+
}
198+
199+
private function findMisuse(Collection $misuses, $misuse_muid, $reviewer, &$previous_misuse, &$misuse, &$next_misuse, &$next_reviewable_misuse)
200+
{
201+
foreach ($misuses as $current_misuse) {
202+
if ($misuse && !$next_misuse) {
203+
$next_misuse = $current_misuse;
204+
}
205+
if ($misuse && !$current_misuse->hasReviewed($reviewer) && !$current_misuse->hasSufficientReviews()) {
206+
$next_reviewable_misuse = $current_misuse;
207+
break;
208+
}
209+
if (!$misuse && $current_misuse->misuse_muid == $misuse_muid) {
210+
$misuse = $current_misuse;
211+
203212
}
204213
if (!$misuse) {
205214
$previous_misuse = $current_misuse;
206215
}
207216
}
217+
}
208218

209-
if (!$next_misuse) {
210-
$next_misuse = $all_misuses->first();
219+
private function findPreviousMisuse(Collection $runs, $misuse)
220+
{
221+
$previous_misuse = NULL;
222+
$idx = sizeof($runs) - 1;
223+
while(!$previous_misuse && $idx >= 0){
224+
$run = $runs->get($idx);
225+
$previous_misuse = $run->misuses->last();
226+
if($previous_misuse && ($misuse->id == $previous_misuse->id)){
227+
$previous_misuse = NULL;
228+
}
229+
$idx--;
211230
}
231+
return $previous_misuse;
232+
}
212233

213-
return array($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse);
234+
private function findNextReviewableMisuse(Collection $misuses, $reviewer)
235+
{
236+
$next_reviewable_misuse = NULL;
237+
foreach ($misuses as $current_misuse) {
238+
if (!$current_misuse->hasReviewed($reviewer) && !$current_misuse->hasSufficientReviews()) {
239+
$next_reviewable_misuse = $current_misuse;
240+
break;
241+
}
242+
}
243+
return $next_reviewable_misuse;
244+
}
245+
246+
private function findNextReviewableBeforeMisuse(Collection $runs, $experiment, $project_muid, $version_muid, $misuse, $reviewer, $ex2_review_size)
247+
{
248+
$next_reviewable_misuse = NULL;
249+
foreach($runs as $run){
250+
$next_reviewable_misuse = $this->findNextReviewableMisuse($run->getMisuses($experiment, $ex2_review_size), $reviewer);
251+
252+
if($run->project_muid == $project_muid && $run->version_muid == $version_muid){
253+
break;
254+
}
255+
}
256+
if($next_reviewable_misuse && $misuse->id == $next_reviewable_misuse->id) {
257+
return NULL;
258+
}
259+
return $next_reviewable_misuse;
214260
}
215261

216262
}

‎mubench.reviewsite/tests/ReviewControllerTest.php‎

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
require_once 'SlimTestCase.php';
66

77
use MuBench\ReviewSite\Models\Decision;
8+
use MuBench\ReviewSite\Models\Detector;
9+
use MuBench\ReviewSite\Models\Experiment;
810
use MuBench\ReviewSite\Models\Misuse;
911
use MuBench\ReviewSite\Models\Reviewer;
12+
use MuBench\ReviewSite\Models\Run;
1013
use MuBench\ReviewSite\Models\Violation;
1114
use SlimTestCase;
1215

@@ -72,13 +75,15 @@ function test_stores_violations()
7275

7376
function test_determine_next_misuse_current_and_next_are_reviewed()
7477
{
75-
list($misuse1, $misuse2, $misuse3) = $this->createRunWithThreeMisuses();
78+
$this->createRunWithMisuses("-p-", "-v-", 3);
79+
list($misuse1, $misuse2, $misuse3) = Misuse::all()->all();
80+
$runs = Run::of(Detector::find('test-detector'))->in(Experiment::find(2))->get();
7681

7782
$this->createConclusiveReviewState($misuse1);
7883
$this->createConclusiveReviewState($misuse2);
7984

8085
// current misuse = 1, reviewer = 1
81-
list($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse) = $this->reviewController->determineNavigationTargets(Misuse::all(), '-p-', '-v-', $misuse1->misuse_muid, $this->reviewer1);
86+
list($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse) = $this->reviewController->determineNavigationTargets($runs, Experiment::find(2), '-p-', '-v-', $misuse1->misuse_muid, $this->reviewer1, 30);
8287

8388
self::assertEquals($misuse3->misuse_muid, $previous_misuse->misuse_muid);
8489
self::assertEquals($misuse2->misuse_muid, $next_misuse->misuse_muid);
@@ -88,13 +93,15 @@ function test_determine_next_misuse_current_and_next_are_reviewed()
8893

8994
function test_determine_next_misuse_current_is_only_reviewable()
9095
{
91-
list($misuse1, $misuse2, $misuse3) = $this->createRunWithThreeMisuses();
96+
$this->createRunWithMisuses("-p-", "-v-", 3);
97+
list($misuse1, $misuse2, $misuse3) = Misuse::all()->all();
98+
$runs = Run::of(Detector::find('test-detector'))->in(Experiment::find(2))->get();
9299

93100
$this->createConclusiveReviewState($misuse1);
94101
$this->createConclusiveReviewState($misuse2);
95102

96103
// current misuse = 3, reviewer = 1
97-
list($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse) = $this->reviewController->determineNavigationTargets(Misuse::all(), '-p-', '-v-', $misuse3->misuse_muid, $this->reviewer1);
104+
list($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse) = $this->reviewController->determineNavigationTargets($runs, Experiment::find(2), '-p-', '-v-', $misuse3->misuse_muid, $this->reviewer1, 30);
98105

99106
self::assertEquals($misuse2->misuse_muid, $previous_misuse->misuse_muid);
100107
self::assertEquals($misuse1->misuse_muid, $next_misuse->misuse_muid);
@@ -104,47 +111,61 @@ function test_determine_next_misuse_current_is_only_reviewable()
104111

105112
function test_determine_next_misuse_not_fully_reviewed()
106113
{
107-
list($misuse1, $misuse2, $misuse3) = $this->createRunWithThreeMisuses();
114+
$this->createRunWithMisuses("-p-", "-v-", 3);
115+
list($misuse1, $misuse2, $misuse3) = Misuse::all()->all();
116+
$runs = Run::of(Detector::find('test-detector'))->in(Experiment::find(2))->get();
108117

109118
$this->createReview($misuse1, $this->reviewer2, "Yes");
110119
$this->createReview($misuse2, $this->reviewer1, "Yes");
111120
$this->createReview($misuse3, $this->reviewer2, "Yes");
112121

113122
// current misuse = 1, reviewer = 1
114-
list($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse) = $this->reviewController->determineNavigationTargets(Misuse::all(), '-p-', '-v-', $misuse1->misuse_muid, $this->reviewer1);
123+
list($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse) = $this->reviewController->determineNavigationTargets($runs, Experiment::find(2), '-p-', '-v-', $misuse1->misuse_muid, $this->reviewer1, 30);
115124

116125
self::assertEquals($misuse3->misuse_muid, $previous_misuse->misuse_muid);
117126
self::assertEquals($misuse2->misuse_muid, $next_misuse->misuse_muid);
118127
self::assertEquals($misuse3->misuse_muid, $next_reviewable_misuse->misuse_muid);
119128
self::assertEquals($misuse1->misuse_muid, $misuse->misuse_muid);
120129
}
121130

122-
private function createRunWithThreeMisuses()
131+
function test_determine_nav_multiple_runs()
123132
{
124-
$runsController = new RunsController($this->container);
125-
$runsController->addRun(1, 'test-detector', '-p-', '-v-',
126-
[
127-
"result" => "success",
128-
"runtime" => 42.1,
129-
"number_of_findings" => 23,
130-
"timestamp" => 12,
131-
"potential_hits" => [
132-
[
133-
"misuse" => "0",
134-
"rank" => 0,
135-
],
136-
[
137-
"misuse" => "1",
138-
"rank" => 0,
139-
],
140-
[
141-
"misuse" => "2",
142-
"rank" => 0,
143-
]
144-
]
133+
$this->createRunWithMisuses("-p-", "-v-", 1);
134+
$this->createRunWithMisuses("-p1-", "-v-", 1);
135+
list($misuse1, $misuse2) = Misuse::all()->all();
136+
$runs = Run::of(Detector::find('test-detector'))->in(Experiment::find(2))->get();
137+
138+
$this->createConclusiveReviewState($misuse1);
139+
140+
// current misuse = 2, reviewer = 1
141+
list($previous_misuse, $next_misuse, $next_reviewable_misuse, $misuse) = $this->reviewController->determineNavigationTargets($runs, Experiment::find(2), '-p1-', '-v-', $misuse2->misuse_muid, $this->reviewer1, 30);
142+
143+
self::assertEquals($misuse1->misuse_muid, $previous_misuse->misuse_muid);
144+
self::assertEquals($misuse1->misuse_muid, $next_misuse->misuse_muid);
145+
self::assertNull($next_reviewable_misuse);
146+
self::assertEquals($misuse2->misuse_muid, $misuse->misuse_muid);
147+
}
148+
149+
private function createRunWithMisuses($project, $version, $amount)
150+
{
151+
$findings = [
152+
"result" => "success",
153+
"runtime" => 42.1,
154+
"number_of_findings" => 23,
155+
"timestamp" => 12,
156+
"potential_hits" => [
145157
]
146-
);
147-
return Misuse::all()->all();
158+
];
159+
for($i = 0; $i < $amount; $i++){
160+
$findings["potential_hits"][] = [
161+
"misuse" => "{$i}",
162+
"rank" => 0,
163+
"file" => "f",
164+
"target_snippets" => []
165+
];
166+
}
167+
$runsController = new RunsController($this->container);
168+
$runsController->addRun(2, 'test-detector', $project, $version, $findings);
148169
}
149170

150171
private function createConclusiveReviewState($misuse)

0 commit comments

Comments
 (0)