Skip to content

Commit 2f0c0f8

Browse files
ginoaugustineginoaugustine
authored
opengrok web project code smell fixes (#4457)
--------- Signed-off-by: Gino Augustine <[email protected]>
1 parent 781cced commit 2f0c0f8

15 files changed

+110
-177
lines changed

‎opengrok-web/src/main/java/org/opengrok/web/Scripts.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,11 @@
2929
import java.util.List;
3030
import java.util.Map;
3131
import java.util.NavigableSet;
32+
import java.util.Optional;
3233
import java.util.TreeMap;
3334
import java.util.TreeSet;
3435

36+
import org.jetbrains.annotations.NotNull;
3537
import org.webjars.WebJarAssetLocator;
3638

3739
/**
@@ -116,9 +118,9 @@ public String toHtml() {
116118
putFromWebJar("jquery", "3.6.4/jquery.min.js", 10);
117119
putjs("jquery-ui", "js/jquery-ui-1.12.1-custom", 11);
118120
putFromWebJar("jquery-tablesorter", "2.31.3/dist/js/jquery.tablesorter.min.js", 12);
119-
putjs("tablesorter-parsers", "js/tablesorter-parsers-0.0.3", 13, true);
120-
putjs("searchable-option-list", "js/searchable-option-list-2.0.15", 14, true);
121-
putjs("utils", "js/utils-0.0.46", 15, true);
121+
putjs("tablesorter-parsers", "js/tablesorter-parsers-0.0.4", 13, true);
122+
putjs("searchable-option-list", "js/searchable-option-list-2.0.16", 14, true);
123+
putjs("utils", "js/utils-0.0.47", 15, true);
122124
putjs("repos", "js/repos-0.0.3", 20, true);
123125
putjs("diff", "js/diff-0.0.5", 20, true);
124126
putjs("jquery-caret", "js/jquery.caret-1.5.2", 25);
@@ -203,7 +205,7 @@ public boolean isEmpty() {
203205
* @see List#iterator()
204206
*/
205207
@Override
206-
public Iterator<Script> iterator() {
208+
public @NotNull Iterator<Script> iterator() {
207209
return outputScripts.iterator();
208210
}
209211

@@ -216,7 +218,8 @@ public Iterator<Script> iterator() {
216218
* @return true if script was added; false otherwise
217219
*/
218220
public boolean addScript(String contextPath, String scriptName, Type type) {
219-
contextPath = contextPath == null || contextPath.isEmpty() ? "/" : contextPath + "/";
221+
contextPath = Optional.ofNullable(contextPath)
222+
.orElse("").concat("/");
220223
if (type == Type.DEBUG && SCRIPTS.containsKey(scriptName + DEBUG_SUFFIX)) {
221224
addScript(contextPath, scriptName + DEBUG_SUFFIX);
222225
return true;

‎opengrok-web/src/main/java/org/opengrok/web/StatisticsFilter.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
public class StatisticsFilter implements Filter {
4646

4747
static final String REQUESTS_METRIC = "requests";
48+
private static final String CATEGORY_TAG = "category";
4849

4950
private final DistributionSummary requests = Metrics.getPrometheusRegistry().summary(REQUESTS_METRIC);
5051

@@ -76,7 +77,7 @@ private void measure(HttpServletResponse httpResponse, HttpServletRequest httpRe
7677
category = getCategory(httpReq, config);
7778

7879
Timer categoryTimer = Timer.builder("requests.latency").
79-
tags("category", category, "code", String.valueOf(httpResponse.getStatus())).
80+
tags(CATEGORY_TAG, category, "code", String.valueOf(httpResponse.getStatus())).
8081
register(Metrics.getPrometheusRegistry());
8182
categoryTimer.record(duration);
8283

@@ -85,12 +86,12 @@ private void measure(HttpServletResponse httpResponse, HttpServletRequest httpRe
8586
if (helper != null && registry != null) {
8687
if (helper.getHits() == null || helper.getHits().length == 0) {
8788
Timer.builder("search.latency").
88-
tags("category", "ui", "outcome", "empty").
89+
tags(CATEGORY_TAG, "ui", "outcome", "empty").
8990
register(registry).
9091
record(duration);
9192
} else {
9293
Timer.builder("search.latency").
93-
tags("category", "ui", "outcome", "success").
94+
tags(CATEGORY_TAG, "ui", "outcome", "success").
9495
register(registry).
9596
record(duration);
9697
}

‎opengrok-web/src/main/java/org/opengrok/web/api/v1/controller/ProjectsController.java

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,11 @@
2929
import java.io.IOException;
3030
import java.nio.file.Paths;
3131
import java.util.ArrayList;
32+
import java.util.Collection;
3233
import java.util.Collections;
3334
import java.util.List;
3435
import java.util.Map;
36+
import java.util.Optional;
3537
import java.util.Set;
3638
import java.util.TreeSet;
3739
import java.util.concurrent.CompletableFuture;
@@ -74,8 +76,9 @@
7476
public class ProjectsController {
7577

7678
private static final Logger LOGGER = LoggerFactory.getLogger(ProjectsController.class);
79+
private static final String NO_REPO_LOG_MSG = "no repositories found for project ''{0}''";
7780

78-
public static final String PROJECTS_PATH = "/projects";
81+
public static final String PROJECTS_PATH = "projects";
7982

8083
private final RuntimeEnvironment env = RuntimeEnvironment.getInstance();
8184

@@ -131,19 +134,14 @@ private void addProjectWorkHorse(String projectName) {
131134
List<RepositoryInfo> allrepos = env.getRepositories();
132135
synchronized (allrepos) {
133136
// newly added repository
134-
for (RepositoryInfo repo : repos) {
135-
if (!allrepos.contains(repo)) {
136-
allrepos.add(repo);
137-
}
138-
}
137+
repos.stream()
138+
.filter(repo -> !allrepos.contains(repo))
139+
.forEach(allrepos::add);
139140
// deleted repository
140-
if (map.containsKey(project)) {
141-
for (RepositoryInfo repo : map.get(project)) {
142-
if (!repos.contains(repo)) {
143-
allrepos.remove(repo);
144-
}
145-
}
146-
}
141+
Optional.ofNullable(map.get(project))
142+
.stream().flatMap(Collection::stream)
143+
.filter(repo -> !repos.contains(repo))
144+
.forEach(allrepos::remove);
147145
}
148146

149147
map.put(project, repos);
@@ -249,7 +247,7 @@ private void deleteProjectDataWorkHorse(Project project, boolean clearHistoryGur
249247

250248
List<RepositoryInfo> repos = env.getProjectRepositoriesMap().get(project);
251249
if (repos == null || repos.isEmpty()) {
252-
LOGGER.log(Level.INFO, "no repositories found for project ''{0}''", projectName);
250+
LOGGER.log(Level.INFO, NO_REPO_LOG_MSG, projectName);
253251
return;
254252
}
255253

@@ -274,7 +272,7 @@ public Response deleteAnnotationCache(@Context HttpServletRequest request,
274272
Project project = getProjectFromName(projectNameParam);
275273
List<RepositoryInfo> repos = env.getProjectRepositoriesMap().get(project);
276274
if (repos == null || repos.isEmpty()) {
277-
LOGGER.log(Level.INFO, "no repositories found for project ''{0}''", project.getName());
275+
LOGGER.log(Level.INFO, NO_REPO_LOG_MSG, project.getName());
278276
return null;
279277
}
280278

@@ -307,7 +305,7 @@ public Response deleteHistoryCache(@Context HttpServletRequest request,
307305
Project project = getProjectFromName(projectNameParam);
308306
List<RepositoryInfo> repos = env.getProjectRepositoriesMap().get(project);
309307
if (repos == null || repos.isEmpty()) {
310-
LOGGER.log(Level.INFO, "no repositories found for project ''{0}''", project.getName());
308+
LOGGER.log(Level.INFO, NO_REPO_LOG_MSG, project.getName());
311309
return null;
312310
}
313311

‎opengrok-web/src/main/java/org/opengrok/web/api/v1/controller/SuggesterController.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ public void addSearchCountsQueries(final List<String> urls) {
221221
}
222222
}
223223
} catch (MalformedURLException e) {
224-
logger.log(Level.WARNING, "Could not add search counts for " + urlStr, e);
224+
logger.log(Level.WARNING, e, () -> "Could not add search counts for " + urlStr);
225225
}
226226
}
227227
}

‎opengrok-web/src/main/java/org/opengrok/web/api/v1/filter/PathAuthorizationFilter.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ public void filter(final ContainerRequestContext context) {
8484
if (!isPathAuthorized(path, request)) {
8585
// TODO: this should probably update statistics for denied requests like in AuthorizationFilter
8686
context.abortWith(Response.status(Response.Status.FORBIDDEN).build());
87-
return; // for good measure
8887
}
8988
}
9089
}

‎opengrok-web/src/main/webapp/default/jquery.autocomplete.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88

99
.ac_results ul {
1010
width: 100%;
11-
list-style-position: outside;
1211
list-style: none;
12+
list-style-position: outside;
1313
padding: 0;
1414
margin: 0;
1515
}

‎opengrok-web/src/main/webapp/default/jquery.combo.css

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
cursor: pointer;
2323
height: 25px;
2424
width: 22px;
25-
cursor: pointer;
2625
}
2726

2827
.combo_td2 img {

‎opengrok-web/src/main/webapp/default/mandoc-1.0.0.css

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ table.head { width: 100%;
6464
td.head-vol { text-align: center; }
6565
td.head-rtitle {
6666
text-align: right; }
67-
span.Nd { }
6867

6968
table.foot { width: 100%;
7069
border-top: 1px dotted #808080;
@@ -85,44 +84,30 @@ h2.Ss { margin-top: 2ex;
8584
margin-left: -2ex;
8685
font-size: 105%; }
8786
div.Pp { margin: 1ex 0ex; }
88-
a.Sx { }
89-
a.Xr { }
9087

9188
/* Displays and lists. */
9289

93-
div.Bd { }
9490
div.D1 { margin-left: 5ex; }
9591

9692
ul.Bl-bullet { list-style-type: disc;
9793
padding-left: 1em; }
98-
li.It-bullet { }
9994
ul.Bl-dash { list-style-type: none;
10095
padding-left: 0em; }
10196
li.It-dash:before {
10297
content: "\2014 "; }
10398
ul.Bl-item { list-style-type: none;
10499
padding-left: 0em; }
105-
li.It-item { }
106100
ul.Bl-compact > li {
107101
margin-top: 0ex; }
108102

109103
ol.Bl-enum { padding-left: 2em; }
110-
li.It-enum { }
111104
ol.Bl-compact > li {
112105
margin-top: 0ex; }
113106

114-
dl.Bl-diag { }
115-
dt.It-diag { }
116107
dd.It-diag { margin-left: 0ex; }
117108
b.It-diag { font-style: normal; }
118-
dl.Bl-hang { }
119-
dt.It-hang { }
120109
dd.It-hang { margin-left: 10.2ex; }
121-
dl.Bl-inset { }
122-
dt.It-inset { }
123110
dd.It-inset { margin-left: 0ex; }
124-
dl.Bl-ohang { }
125-
dt.It-ohang { }
126111
dd.It-ohang { margin-left: 0ex; }
127112
dl.Bl-tag { margin-left: 10.2ex; }
128113
dt.It-tag { float: left;
@@ -139,41 +124,24 @@ dd.It-tag { clear: right;
139124
dl.Bl-compact > dt {
140125
margin-top: 0ex; }
141126

142-
table.Bl-column { }
143-
tr.It-column { }
144127
td.It-column { margin-top: 1em; }
145128
table.Bl-compact > tbody > tr > td {
146129
margin-top: 0ex; }
147130

148131
cite.Rs { font-style: normal;
149132
font-weight: normal; }
150-
span.RsA { }
151133
i.RsB { font-weight: normal; }
152-
span.RsC { }
153-
span.RsD { }
154134
i.RsI { font-weight: normal; }
155135
i.RsJ { font-weight: normal; }
156-
span.RsN { }
157-
span.RsO { }
158-
span.RsP { }
159-
span.RsQ { }
160-
span.RsR { }
161136
span.RsT { text-decoration: underline; }
162-
a.RsU { }
163-
span.RsV { }
164-
165-
span.eqn { }
166-
table.tbl { }
167137

168138
/* Semantic markup for command line utilities. */
169139

170-
table.Nm { }
171140
b.Nm { font-style: normal; }
172141
b.Fl { font-style: normal; }
173142
b.Cm { font-style: normal; }
174143
var.Ar { font-style: italic;
175144
font-weight: normal; }
176-
span.Op { }
177145
b.Ic { font-style: normal; }
178146
code.Ev { font-style: normal;
179147
font-weight: normal;
@@ -182,9 +150,7 @@ i.Pa { font-weight: normal; }
182150

183151
/* Semantic markup for function libraries. */
184152

185-
span.Lb { }
186153
b.In { font-style: normal; }
187-
a.In { }
188154
b.Fd { font-style: normal; }
189155
var.Ft { font-style: italic;
190156
font-weight: normal; }
@@ -204,14 +170,9 @@ code.Er { font-style: normal;
204170

205171
/* Various semantic markup. */
206172

207-
span.An { }
208-
a.Lk { }
209-
a.Mt { }
210173
b.Cd { font-style: normal; }
211174
i.Ad { font-weight: normal; }
212175
b.Ms { font-style: normal; }
213-
span.St { }
214-
a.Ux { }
215176

216177
/* Physical markup. */
217178

‎opengrok-web/src/main/webapp/default/print-1.0.2.css

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@ label {
7777
display: block;
7878
}
7979

80-
#page { }
81-
8280
.error { /* error messages */
8381
color: #a52a2a;
8482
}
@@ -335,9 +333,6 @@ table#dirlist { /* the "Name" column */
335333

336334

337335
/* file display */
338-
#src {
339-
}
340-
341336
#src pre {
342337
margin: 0;
343338
font-size: small;
@@ -422,9 +417,6 @@ a.xsr { /* subroutine */ color: #00f; font-weight: bold;
422417
}
423418

424419
/* search result page */
425-
#results {
426-
}
427-
428420
#results p { /* pagetitle and slider */
429421
padding: 0.1em;
430422
}

0 commit comments

Comments
 (0)