Skip to content

Commit 06573a2

Browse files
tulinkryVladimir Kotal
authored andcommitted
Suppressing the double render problem on error (#2725)
1 parent 1161ac1 commit 06573a2

File tree

2 files changed

+32
-60
lines changed

2 files changed

+32
-60
lines changed

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

Lines changed: 11 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@
2323
*/
2424
package org.opengrok.indexer.web;
2525

26-
import java.util.ArrayList;
27-
import java.util.Collections;
2826
import java.util.Comparator;
2927
import java.util.Iterator;
3028
import java.util.List;
3129
import java.util.Map;
30+
import java.util.NavigableSet;
3231
import java.util.TreeMap;
32+
import java.util.TreeSet;
3333

3434
/**
3535
* A list-like container for javascripts in the page.
@@ -111,11 +111,14 @@ public String toHtml() {
111111
SCRIPTS.put("jquery-caret", new FileScript("js/jquery.caret-1.5.2.min.js", 25));
112112
}
113113

114+
private static final Comparator<Script> SCRIPTS_COMPARATOR = Comparator
115+
.comparingInt(Script::getPriority)
116+
.thenComparing(Script::getScriptData);
117+
114118
/**
115-
* Scripts which will be written to the page. We assume that the length
116-
* could be the same as for {@link #SCRIPTS}.
119+
* Scripts which will be written to the page.
117120
*/
118-
private final List<Script> outputScripts = new ArrayList<>(SCRIPTS.size());
121+
private final NavigableSet<Script> outputScripts = new TreeSet<>(SCRIPTS_COMPARATOR);
119122

120123
/**
121124
* Convert the page scripts into HTML.
@@ -134,7 +137,6 @@ public String toHtml() {
134137
* Return the HTML representation of the page scripts.
135138
*
136139
* @return the HTML
137-
*
138140
* @see #toHtml()
139141
*/
140142
@Override
@@ -146,7 +148,6 @@ public String toString() {
146148
* Return the size of the page scripts.
147149
*
148150
* @return the size
149-
*
150151
* @see List#size()
151152
*/
152153
public int size() {
@@ -157,7 +158,6 @@ public int size() {
157158
* Check if there is any script for this page.
158159
*
159160
* @return true if there is not; false otherwise
160-
*
161161
* @see List#isEmpty()
162162
*/
163163
public boolean isEmpty() {
@@ -179,7 +179,7 @@ public Iterator<Script> iterator() {
179179
* Add a script which is identified by the name.
180180
*
181181
* @param contextPath given context path for the used URL
182-
* @param scriptName name of the script
182+
* @param scriptName name of the script
183183
* @return true if script was added; false otherwise
184184
*/
185185
public boolean addScript(String contextPath, String scriptName) {
@@ -195,46 +195,11 @@ public boolean addScript(String contextPath, String scriptName) {
195195
}
196196

197197
/**
198-
* Add a script to the page, taking the script priority into account. The
199-
* position is determined as the upper bound for the given priority.
198+
* Add a script to the page, taking the script priority into account.
200199
*
201200
* @param script the script
202201
*/
203202
public void addScript(Script script) {
204-
int index = Collections.binarySearch(outputScripts, script, new Comparator<Script>() {
205-
@Override
206-
public int compare(Script a, Script b) {
207-
return a.getPriority() - b.getPriority();
208-
}
209-
});
210-
if (index < 0) {
211-
/**
212-
* Key is not found in the list the index<br>
213-
* equals to -(insertion index) - 1.
214-
*/
215-
this.outputScripts.add(Math.abs(index + 1), script);
216-
} else {
217-
/**
218-
* Key found in the list, append it after the last element with the
219-
* same priority => insert it at the upper bound index.
220-
*/
221-
while (index < this.outputScripts.size()
222-
&& this.outputScripts.get(index).getPriority() == script.getPriority()) {
223-
index++;
224-
}
225-
this.outputScripts.add(index, script);
226-
}
227-
}
228-
229-
/**
230-
* Get the page script by the index
231-
*
232-
* @param index index of the script
233-
* @return the script
234-
*
235-
* @see List#get(int)
236-
*/
237-
public Script get(int index) {
238-
return outputScripts.get(index);
203+
this.outputScripts.add(script);
239204
}
240205
}

opengrok-indexer/src/test/java/org/opengrok/indexer/web/ScriptsTest.java

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,17 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2017, 2018 Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2017, 2019 Oracle and/or its affiliates. All rights reserved.
2222
*/
2323
package org.opengrok.indexer.web;
2424

2525
import static org.junit.Assert.assertEquals;
2626
import static org.junit.Assert.assertTrue;
2727
import static org.junit.Assert.fail;
2828

29+
import java.util.ArrayList;
30+
import java.util.List;
2931
import java.util.Map.Entry;
30-
3132
import org.junit.Before;
3233
import org.junit.Test;
3334
import org.opengrok.indexer.web.Scripts.Script;
@@ -53,12 +54,15 @@ public void testInstance() {
5354

5455
assertEquals(3, scripts.size());
5556

56-
assertEquals(scripts.get(0).getScriptData(), "http://example.com/main1.js");
57-
assertEquals(scripts.get(0).getPriority(), 0);
58-
assertEquals(scripts.get(1).getScriptData(), "http://example.com/main2.js");
59-
assertEquals(scripts.get(1).getPriority(), 0);
60-
assertEquals(scripts.get(2).getScriptData(), "http://example.com/main3.js");
61-
assertEquals(scripts.get(2).getPriority(), 0);
57+
List<Script> listScripts = new ArrayList<>();
58+
scripts.forEach(listScripts::add);
59+
60+
assertEquals(listScripts.get(0).getScriptData(), "http://example.com/main1.js");
61+
assertEquals(listScripts.get(0).getPriority(), 0);
62+
assertEquals(listScripts.get(1).getScriptData(), "http://example.com/main2.js");
63+
assertEquals(listScripts.get(1).getPriority(), 0);
64+
assertEquals(listScripts.get(2).getScriptData(), "http://example.com/main3.js");
65+
assertEquals(listScripts.get(2).getPriority(), 0);
6266
}
6367

6468
@Test
@@ -69,12 +73,15 @@ public void testSorted() {
6973

7074
assertEquals(3, scripts.size());
7175

72-
assertEquals(scripts.get(0).getScriptData(), "http://example.com/main2.js");
73-
assertEquals(scripts.get(0).getPriority(), 1);
74-
assertEquals(scripts.get(1).getScriptData(), "http://example.com/main3.js");
75-
assertEquals(scripts.get(1).getPriority(), 2);
76-
assertEquals(scripts.get(2).getScriptData(), "http://example.com/main1.js");
77-
assertEquals(scripts.get(2).getPriority(), 3);
76+
List<Script> listScripts = new ArrayList<>();
77+
scripts.forEach(listScripts::add);
78+
79+
assertEquals(listScripts.get(0).getScriptData(), "http://example.com/main2.js");
80+
assertEquals(listScripts.get(0).getPriority(), 1);
81+
assertEquals(listScripts.get(1).getScriptData(), "http://example.com/main3.js");
82+
assertEquals(listScripts.get(1).getPriority(), 2);
83+
assertEquals(listScripts.get(2).getScriptData(), "http://example.com/main1.js");
84+
assertEquals(listScripts.get(2).getPriority(), 3);
7885
}
7986

8087
@Test

0 commit comments

Comments
 (0)