Skip to content

Commit 2df1bf1

Browse files
brendanx67claude
brendanx67
and
claude
committed
Refactored RetrainAllAction per code review feedback
* Added RetrainAllForm class for automatic parameter binding * Moved SQL operations to TestResultsManager class * Added TabNames constants for tab identifiers and attribute name * Added getTabClass() helper to reduce repetitive code in menu.jsp * Updated all JSPs to use TabNames constants Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 009c0f1 commit 2df1bf1

File tree

12 files changed

+318
-193
lines changed

12 files changed

+318
-193
lines changed

‎testresults/src/org/labkey/testresults/TestResultsController.java‎

Lines changed: 78 additions & 175 deletions
Large diffs are not rendered by default.

‎testresults/src/org/labkey/testresults/TestResultsManager.java‎

Lines changed: 223 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,20 @@
1616

1717
package org.labkey.testresults;
1818

19+
import org.labkey.api.data.Container;
20+
import org.labkey.api.data.DbScope;
21+
import org.labkey.api.data.SQLFragment;
22+
import org.labkey.api.data.SqlExecutor;
23+
import org.labkey.api.data.SqlSelector;
24+
25+
import java.sql.Timestamp;
26+
import java.util.ArrayList;
27+
import java.util.HashMap;
28+
import java.util.HashSet;
29+
import java.util.List;
30+
import java.util.Map;
31+
import java.util.Set;
32+
1933
/**
2034
* User: Yuval Boss, yuval(at)uw.edu
2135
* Date: 1/14/2015
@@ -33,4 +47,212 @@ public static TestResultsManager get()
3347
{
3448
return _instance;
3549
}
36-
}
50+
51+
/**
52+
* Get distinct user IDs from recent test runs in a container.
53+
*/
54+
public List<Integer> getRecentUserIds(Container container, Timestamp cutoffDate)
55+
{
56+
DbScope scope = TestResultsSchema.getSchema().getScope();
57+
SQLFragment sql = new SQLFragment();
58+
sql.append(
59+
"SELECT DISTINCT userid FROM " + TestResultsSchema.getTableInfoTestRuns() +
60+
" WHERE container = ? AND posttime >= ?");
61+
sql.add(container.getEntityId());
62+
sql.add(cutoffDate);
63+
64+
List<Integer> userIds = new ArrayList<>();
65+
new SqlSelector(scope, sql).forEach(rs -> userIds.add(rs.getInt("userid")));
66+
return userIds;
67+
}
68+
69+
/**
70+
* Get trainrun counts per user for a container.
71+
*/
72+
public Map<Integer, Integer> getTrainRunCounts(Container container)
73+
{
74+
DbScope scope = TestResultsSchema.getSchema().getScope();
75+
SQLFragment sql = new SQLFragment();
76+
sql.append(
77+
"SELECT r.userid, COUNT(tr.runid) as traincount" +
78+
" FROM " + TestResultsSchema.getTableInfoTrain() + " tr" +
79+
" JOIN " + TestResultsSchema.getTableInfoTestRuns() + " r ON tr.runid = r.id" +
80+
" WHERE r.container = ?" +
81+
" GROUP BY r.userid");
82+
sql.add(container.getEntityId());
83+
84+
Map<Integer, Integer> counts = new HashMap<>();
85+
new SqlSelector(scope, sql).forEach(rs ->
86+
counts.put(rs.getInt("userid"), rs.getInt("traincount")));
87+
return counts;
88+
}
89+
90+
/**
91+
* Delete all trainruns for a container.
92+
*/
93+
public void deleteTrainRunsForContainer(Container container)
94+
{
95+
DbScope scope = TestResultsSchema.getSchema().getScope();
96+
SQLFragment sql = new SQLFragment();
97+
sql.append(
98+
"DELETE FROM " + TestResultsSchema.getTableInfoTrain() +
99+
" WHERE runid IN (SELECT id FROM " + TestResultsSchema.getTableInfoTestRuns() +
100+
" WHERE container = ?)");
101+
sql.add(container.getEntityId());
102+
new SqlExecutor(scope).execute(sql);
103+
}
104+
105+
/**
106+
* Delete all userdata for a container.
107+
*/
108+
public void deleteUserDataForContainer(Container container)
109+
{
110+
DbScope scope = TestResultsSchema.getSchema().getScope();
111+
SQLFragment sql = new SQLFragment();
112+
sql.append("DELETE FROM " + TestResultsSchema.getTableInfoUserData() + " WHERE container = ?");
113+
sql.add(container.getEntityId());
114+
new SqlExecutor(scope).execute(sql);
115+
}
116+
117+
/**
118+
* Get existing trainrun IDs for a user in a container.
119+
*/
120+
public Set<Integer> getExistingTrainRunIds(int userId, Container container)
121+
{
122+
DbScope scope = TestResultsSchema.getSchema().getScope();
123+
SQLFragment sql = new SQLFragment();
124+
sql.append(
125+
"SELECT tr.runid FROM " + TestResultsSchema.getTableInfoTrain() + " tr" +
126+
" JOIN " + TestResultsSchema.getTableInfoTestRuns() + " r ON tr.runid = r.id" +
127+
" WHERE r.userid = ? AND r.container = ?");
128+
sql.add(userId);
129+
sql.add(container.getEntityId());
130+
131+
Set<Integer> runIds = new HashSet<>();
132+
new SqlSelector(scope, sql).forEach(rs -> runIds.add(rs.getInt("runid")));
133+
return runIds;
134+
}
135+
136+
/**
137+
* Get clean run IDs for a user within lookback period.
138+
* Clean = 0 failures, 0 leaks, passedtests > 0, not flagged, full duration, no hangs.
139+
*/
140+
public List<Integer> getCleanRunIds(int userId, Container container, Timestamp cutoffDate, int expectedDuration)
141+
{
142+
DbScope scope = TestResultsSchema.getSchema().getScope();
143+
SQLFragment sql = new SQLFragment();
144+
sql.append(
145+
"SELECT tr.id FROM " + TestResultsSchema.getTableInfoTestRuns() + " tr" +
146+
" WHERE tr.userid = ? AND tr.container = ?" +
147+
" AND tr.posttime >= ?" +
148+
" AND tr.failedtests = 0 AND tr.leakedtests = 0" +
149+
" AND tr.passedtests > 0 AND tr.flagged = false" +
150+
" AND tr.duration >= ?" +
151+
" AND NOT EXISTS (SELECT 1 FROM " + TestResultsSchema.getTableInfoHangs() +
152+
" h WHERE h.testrunid = tr.id)" +
153+
" ORDER BY tr.posttime DESC");
154+
sql.add(userId);
155+
sql.add(container.getEntityId());
156+
sql.add(cutoffDate);
157+
sql.add(expectedDuration);
158+
159+
List<Integer> runIds = new ArrayList<>();
160+
new SqlSelector(scope, sql).forEach(rs -> runIds.add(rs.getInt("id")));
161+
return runIds;
162+
}
163+
164+
/**
165+
* Get combined candidate run IDs (existing + recent) sorted by posttime, limited to maxRuns.
166+
*/
167+
public List<Integer> getCandidateRunIds(int userId, Container container,
168+
Set<Integer> existingIds, List<Integer> recentIds, int maxRuns)
169+
{
170+
if (existingIds.isEmpty() && recentIds.isEmpty())
171+
return new ArrayList<>();
172+
173+
DbScope scope = TestResultsSchema.getSchema().getScope();
174+
SQLFragment sql = new SQLFragment();
175+
sql.append(
176+
"SELECT id FROM " + TestResultsSchema.getTableInfoTestRuns() +
177+
" WHERE userid = ? AND container = ?" +
178+
" AND (id = ANY(?) OR id = ANY(?))" +
179+
" ORDER BY posttime DESC");
180+
sql.add(userId);
181+
sql.add(container.getEntityId());
182+
sql.add(existingIds.toArray(new Integer[0]));
183+
sql.add(recentIds.toArray(new Integer[0]));
184+
185+
List<Integer> result = new ArrayList<>();
186+
new SqlSelector(scope, sql).forEach(rs -> {
187+
if (result.size() < maxRuns)
188+
result.add(rs.getInt("id"));
189+
});
190+
return result;
191+
}
192+
193+
/**
194+
* Delete trainruns by run IDs.
195+
*/
196+
public void removeTrainRuns(List<Integer> runIds)
197+
{
198+
if (runIds.isEmpty())
199+
return;
200+
201+
DbScope scope = TestResultsSchema.getSchema().getScope();
202+
for (int runId : runIds)
203+
{
204+
SQLFragment sql = new SQLFragment();
205+
sql.append("DELETE FROM " + TestResultsSchema.getTableInfoTrain() + " WHERE runid = ?");
206+
sql.add(runId);
207+
new SqlExecutor(scope).execute(sql);
208+
}
209+
}
210+
211+
/**
212+
* Insert trainruns by run IDs.
213+
*/
214+
public void addTrainRuns(List<Integer> runIds)
215+
{
216+
if (runIds.isEmpty())
217+
return;
218+
219+
DbScope scope = TestResultsSchema.getSchema().getScope();
220+
for (int runId : runIds)
221+
{
222+
SQLFragment sql = new SQLFragment();
223+
sql.append("INSERT INTO " + TestResultsSchema.getTableInfoTrain() + " (runid) VALUES (?)");
224+
sql.add(runId);
225+
new SqlExecutor(scope).execute(sql);
226+
}
227+
}
228+
229+
/**
230+
* Upsert userdata with calculated stats from specified run IDs.
231+
*/
232+
public void upsertUserData(int userId, Container container, List<Integer> runIds, boolean active)
233+
{
234+
if (runIds.isEmpty())
235+
return;
236+
237+
DbScope scope = TestResultsSchema.getSchema().getScope();
238+
SQLFragment sql = new SQLFragment();
239+
sql.append(
240+
"INSERT INTO " + TestResultsSchema.getTableInfoUserData() +
241+
" (userid, container, meantestsrun, meanmemory, stddevtestsrun, stddevmemory, active)" +
242+
" SELECT ?, ?, avg(passedtests), avg(averagemem)," +
243+
" stddev_pop(passedtests), stddev_pop(averagemem), ?" +
244+
" FROM " + TestResultsSchema.getTableInfoTestRuns() +
245+
" WHERE id = ANY(?)" +
246+
" ON CONFLICT(userid, container) DO UPDATE SET" +
247+
" meantestsrun = excluded.meantestsrun," +
248+
" meanmemory = excluded.meanmemory," +
249+
" stddevtestsrun = excluded.stddevtestsrun," +
250+
" stddevmemory = excluded.stddevmemory," +
251+
" active = excluded.active");
252+
sql.add(userId);
253+
sql.add(container.getEntityId());
254+
sql.add(active);
255+
sql.add(runIds.toArray(new Integer[0]));
256+
new SqlExecutor(scope).execute(sql);
257+
}
258+
}

‎testresults/src/org/labkey/testresults/view/errorFiles.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
Container c = getContainer();
2525
%>
2626

27-
<% request.setAttribute("activeTab", "errors"); %>
27+
<% request.setAttribute(TestResultsController.TabNames.ACTIVE_TAB_ATTR, TestResultsController.TabNames.ERRORS); %>
2828
<%@include file="menu.jsp" %>
2929

3030
<p>All the files listed below at one point or another failed to post. When a run is successfully posted through this page it gets removed from the list.</p>

‎testresults/src/org/labkey/testresults/view/failureDetail.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@
188188
}
189189
%>
190190

191-
<% request.setAttribute("activeTab", ""); %>
191+
<% request.setAttribute(TestResultsController.TabNames.ACTIVE_TAB_ATTR, ""); %>
192192
<%@include file="menu.jsp" %>
193193

194194
<style>

‎testresults/src/org/labkey/testresults/view/flagged.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
TestsDataBean data = (TestsDataBean)me.getModelBean();
2626
%>
2727

28-
<% request.setAttribute("activeTab", "flags"); %>
28+
<% request.setAttribute(TestResultsController.TabNames.ACTIVE_TAB_ATTR, TestResultsController.TabNames.FLAGS); %>
2929
<%@include file="menu.jsp" %>
3030

3131
<p>Runs which are flagged will not show up in the Overview breakdown, Long Term, and Failure pages. This includes graphs, charts, and any other sort of data visualization.</p>

‎testresults/src/org/labkey/testresults/view/longTerm.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
}
3232
%>
3333

34-
<% request.setAttribute("activeTab", "longterm"); %>
34+
<% request.setAttribute(TestResultsController.TabNames.ACTIVE_TAB_ATTR, TestResultsController.TabNames.LONGTERM); %>
3535
<%@include file="menu.jsp" %>
3636
<form action="<%=h(new ActionURL(TestResultsController.LongTermAction.class, c))%>">
3737
View Type: <select id="view-type-combobox" name="viewType">

‎testresults/src/org/labkey/testresults/view/menu.jsp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
final String menuContextPath = AppProps.getInstance().getContextPath();
99
Container menuContainer = getViewContext().getContainer();
1010
// activeTab is set by parent JSP via request attribute before including menu.jsp
11-
String activeTab = (String) request.getAttribute("activeTab");
11+
String activeTab = (String) request.getAttribute(TestResultsController.TabNames.ACTIVE_TAB_ATTR);
1212
if (activeTab == null) activeTab = "";
1313
%>
1414

@@ -67,13 +67,13 @@
6767
<img src="<%=getWebappURL("TestResults/img/uw.png")%>" id="uw" alt="UW">
6868
<span id="stats"></span>
6969
<ul>
70-
<li><a href="<%=h(new ActionURL(TestResultsController.BeginAction.class, menuContainer))%>" class="<%=h("nav-tab" + (activeTab.equals("overview") ? " active" : ""))%>">Overview</a></li>
71-
<li><a href="<%=h(new ActionURL(TestResultsController.ShowUserAction.class, menuContainer))%>" class="<%=h("nav-tab" + (activeTab.equals("user") ? " active" : ""))%>">User</a></li>
72-
<li><a href="<%=h(new ActionURL(TestResultsController.ShowRunAction.class, menuContainer))%>" class="<%=h("nav-tab" + (activeTab.equals("run") ? " active" : ""))%>">Run</a></li>
73-
<li><a href="<%=h(new ActionURL(TestResultsController.LongTermAction.class, menuContainer))%>" class="<%=h("nav-tab" + (activeTab.equals("longterm") ? " active" : ""))%>">Long Term</a></li>
74-
<li><a href="<%=h(new ActionURL(TestResultsController.ShowFlaggedAction.class, menuContainer))%>" class="<%=h("nav-tab" + (activeTab.equals("flags") ? " active" : ""))%>">Flags</a></li>
75-
<li><a href="<%=h(new ActionURL(TestResultsController.TrainingDataViewAction.class, menuContainer))%>" class="<%=h("nav-tab" + (activeTab.equals("trainingdata") ? " active" : ""))%>">Training Data</a></li>
76-
<li><a href="<%=h(new ActionURL(TestResultsController.ErrorFilesAction.class, menuContainer))%>" class="<%=h("nav-tab" + (activeTab.equals("errors") ? " active" : ""))%>">Posting Errors</a></li>
70+
<li><a href="<%=h(new ActionURL(TestResultsController.BeginAction.class, menuContainer))%>" class="<%=h(TestResultsController.TabNames.getTabClass(TestResultsController.TabNames.OVERVIEW, activeTab))%>">Overview</a></li>
71+
<li><a href="<%=h(new ActionURL(TestResultsController.ShowUserAction.class, menuContainer))%>" class="<%=h(TestResultsController.TabNames.getTabClass(TestResultsController.TabNames.USER, activeTab))%>">User</a></li>
72+
<li><a href="<%=h(new ActionURL(TestResultsController.ShowRunAction.class, menuContainer))%>" class="<%=h(TestResultsController.TabNames.getTabClass(TestResultsController.TabNames.RUN, activeTab))%>">Run</a></li>
73+
<li><a href="<%=h(new ActionURL(TestResultsController.LongTermAction.class, menuContainer))%>" class="<%=h(TestResultsController.TabNames.getTabClass(TestResultsController.TabNames.LONGTERM, activeTab))%>">Long Term</a></li>
74+
<li><a href="<%=h(new ActionURL(TestResultsController.ShowFlaggedAction.class, menuContainer))%>" class="<%=h(TestResultsController.TabNames.getTabClass(TestResultsController.TabNames.FLAGS, activeTab))%>">Flags</a></li>
75+
<li><a href="<%=h(new ActionURL(TestResultsController.TrainingDataViewAction.class, menuContainer))%>" class="<%=h(TestResultsController.TabNames.getTabClass(TestResultsController.TabNames.TRAINING_DATA, activeTab))%>">Training Data</a></li>
76+
<li><a href="<%=h(new ActionURL(TestResultsController.ErrorFilesAction.class, menuContainer))%>" class="<%=h(TestResultsController.TabNames.getTabClass(TestResultsController.TabNames.ERRORS, activeTab))%>">Posting Errors</a></li>
7777
<li><a href="/home/issues/project-begin.view" target="_blank" title="Report bugs/Request features. Use 'TestResults' as area when creating new issue" class="nav-tab">Issues</a></li>
7878
</ul>
7979
</div>

‎testresults/src/org/labkey/testresults/view/multiFailureDetail.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@
6363
}
6464
%>
6565

66-
<% request.setAttribute("activeTab", ""); %>
66+
<% request.setAttribute(TestResultsController.TabNames.ACTIVE_TAB_ATTR, ""); %>
6767
<%@include file="menu.jsp" %>
6868
<form action="<%=h(new ActionURL(TestResultsController.ShowFailures.class, c))%>">
6969
View Type: <select name="viewType">

‎testresults/src/org/labkey/testresults/view/runDetail.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
String runId = getViewContext().getRequest().getParameter("runId");
3838
%>
3939

40-
<% request.setAttribute("activeTab", "run"); %>
40+
<% request.setAttribute(TestResultsController.TabNames.ACTIVE_TAB_ATTR, TestResultsController.TabNames.RUN); %>
4141
<%@include file="menu.jsp" %>
4242
<script type="text/javascript" nonce="<%=getScriptNonce()%>">
4343
LABKEY.requiresCss("/TestResults/css/style.css");

‎testresults/src/org/labkey/testresults/view/rundown.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@
9292
</style>
9393

9494
<div id="container">
95-
<% request.setAttribute("activeTab", "overview"); %>
95+
<% request.setAttribute(TestResultsController.TabNames.ACTIVE_TAB_ATTR, TestResultsController.TabNames.OVERVIEW); %>
9696
<%@include file="menu.jsp" %>
9797
<div id="content">
9898
<p>

0 commit comments

Comments
 (0)