Skip to content

Commit ab973ad

Browse files
author
Vladimir Kotal
committed
fix comments from Krystof
1 parent abe7b78 commit ab973ad

File tree

8 files changed

+288
-253
lines changed

8 files changed

+288
-253
lines changed

src/org/opensolaris/opengrok/configuration/messages/ConfigMessage.java

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

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

25-
import java.beans.IntrospectionException;
26-
import java.beans.PropertyDescriptor;
2725
import java.io.IOException;
28-
import java.lang.reflect.InvocationTargetException;
29-
import java.lang.reflect.Method;
26+
import java.util.Arrays;
27+
import java.util.Set;
28+
import java.util.TreeSet;
3029
import java.util.regex.Matcher;
3130
import java.util.regex.Pattern;
32-
import org.opensolaris.opengrok.authorization.AuthorizationFramework;
33-
import org.opensolaris.opengrok.configuration.Configuration;
3431
import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
32+
import org.opensolaris.opengrok.util.BooleanUtil;
33+
import org.opensolaris.opengrok.util.ClassUtil;
34+
3535

3636
/**
3737
* A message to retrieve/change the configuration.
@@ -61,7 +61,7 @@ protected byte[] applyMessage(RuntimeEnvironment env) throws IOException {
6161
Matcher matcher = VARIABLE_PATTERN.matcher(getText());
6262
if (matcher.find()) {
6363
// set the property
64-
invokeSetter(
64+
ClassUtil.invokeSetter(
6565
env.getConfiguration(),
6666
matcher.group(1), // field
6767
matcher.group(2) // value
@@ -77,233 +77,23 @@ protected byte[] applyMessage(RuntimeEnvironment env) throws IOException {
7777
getText()));
7878
}
7979
} else if (hasTag("get")) {
80-
return invokeGetter(env.getConfiguration(), getText()).getBytes();
80+
return ClassUtil.invokeGetter(env.getConfiguration(), getText()).getBytes();
8181
} else if (hasTag("setconf")) {
8282
env.applyConfig(this, hasTag("reindex"));
8383
}
8484

8585
return null;
8686
}
8787

88-
/**
89-
* Invokes a setter on the configuration object and passes a value to that
90-
* setter.
91-
*
92-
* The value is passed as string and the function will automatically try to
93-
* convert it to the parameter type in the setter. These conversion are
94-
* available only for
95-
* <a href="https://docs.oracle.com/javase/tutorial/java/nutsandbolts/datatypes.html">java
96-
* primitive datatypes</a>:
97-
* <ul>
98-
* <li>Boolean or boolean</li>
99-
* <li>Short or short</li>
100-
* <li>Integer or integer</li>
101-
* <li>Long or long</li>
102-
* <li>Float or float</li>
103-
* <li>Double or double</li>
104-
* <li>Byte or byte</li>
105-
* <li>Character or char</li>
106-
* <li>String</li>
107-
* </ul>
108-
* Any other parameter type will cause an exception.
109-
*
110-
* @param config the configuration object
111-
* @param field name of the field which will be changed
112-
* @param value desired value
113-
*
114-
* @throws IOException if any error occurs (no suitable method, bad
115-
* conversion, ...)
116-
*/
117-
protected void invokeSetter(Configuration config, String field, String value) throws IOException {
118-
try {
119-
PropertyDescriptor desc = new PropertyDescriptor(field, Configuration.class);
120-
Method setter = desc.getWriteMethod();
121-
122-
if (setter == null) {
123-
throw new IOException(
124-
String.format("No setter for the name \"%s\".",
125-
field));
126-
}
127-
128-
if (setter.getParameterCount() != 1) {
129-
// not a setter
130-
/**
131-
* Actually should not happen as it is not considered as a
132-
* writer method so an exception would be thrown earlier.
133-
*/
134-
throw new IOException(
135-
String.format("The setter \"%s\" for the name \"%s\" does not take exactly 1 parameter.",
136-
setter.getName(), field));
137-
}
138-
139-
Class c = setter.getParameterTypes()[0];
140-
String paramClass = c.getName();
141-
142-
/**
143-
* Java primitive types as per
144-
* <a href="https://docs.oracle.com/javase/tutorial/java/nutsandbolts/datatypes.html">java
145-
* datatypes</a>.
146-
*/
147-
if (paramClass.equals("boolean") || paramClass.equals(Boolean.class.getName())) {
148-
if (!isBoolean(value)) {
149-
throw new IOException(String.format("Unsupported type conversion from String to a boolean for name \"%s\" -"
150-
+ " got \"%s\" - allowed values are [false, off, 0, true, on, 1].",
151-
field, value));
152-
}
153-
Boolean v = Boolean.valueOf(value);
154-
if (!v) {
155-
/**
156-
* The Boolean.valueOf() returns true only for "true" case
157-
* insensitive so now we have either the false values or
158-
* "on" or "1". These are convenient shortcuts for "on", "1"
159-
* to be interpreted as booleans.
160-
*/
161-
v = v || value.equalsIgnoreCase("on");
162-
v = v || value.equals("1");
163-
}
164-
setter.invoke(config, v);
165-
} else if (paramClass.equals("short") || paramClass.equals(Short.class.getName())) {
166-
setter.invoke(config, Short.valueOf(value));
167-
} else if (paramClass.equals("int") || paramClass.equals(Integer.class.getName())) {
168-
setter.invoke(config, Integer.valueOf(value));
169-
} else if (paramClass.equals("long") || paramClass.equals(Long.class.getName())) {
170-
setter.invoke(config, Long.valueOf(value));
171-
} else if (paramClass.equals("float") || paramClass.equals(Float.class.getName())) {
172-
setter.invoke(config, Float.valueOf(value));
173-
} else if (paramClass.equals("double") || paramClass.equals(Double.class.getName())) {
174-
setter.invoke(config, Double.valueOf(value));
175-
} else if (paramClass.equals("byte") || paramClass.equals(Byte.class.getName())) {
176-
setter.invoke(config, Byte.valueOf(value));
177-
} else if (paramClass.equals("char") || paramClass.equals(Character.class.getName())) {
178-
setter.invoke(config, value.charAt(0));
179-
} else if (paramClass.equals(String.class.getName())) {
180-
setter.invoke(config, value);
181-
} else {
182-
// error unknown type
183-
throw new IOException(
184-
String.format("Unsupported type conversion for the name \"%s\". Expecting \"%s\".",
185-
field, paramClass));
186-
}
187-
} catch (NumberFormatException ex) {
188-
throw new IOException(
189-
String.format("Unsupported type conversion from String to a number for name \"%s\" - %s.",
190-
field, ex.getLocalizedMessage()), ex);
191-
} catch (IndexOutOfBoundsException ex) {
192-
throw new IOException(
193-
String.format("The string is not long enough to extract 1 character for name \"%s\" - %s.",
194-
field, ex.getLocalizedMessage()), ex);
195-
} catch (IntrospectionException
196-
| IllegalAccessException
197-
| IllegalArgumentException
198-
/**
199-
* This the case when the invocation failed because the invoked
200-
* method failed with an exception. All exceptions are
201-
* propagated through this exception.
202-
*/
203-
| InvocationTargetException ex) {
204-
throw new IOException(
205-
String.format("Unsupported operation with the configuration for name \"%s\" - %s.",
206-
field,
207-
ex.getCause() == null
208-
? ex.getLocalizedMessage()
209-
: ex.getCause().getLocalizedMessage()),
210-
ex);
211-
}
212-
}
213-
214-
/**
215-
* Invokes a getter of a property on the configuration object.
216-
*
217-
* @param config configuration object
218-
* @param field string with field name
219-
* @return string representation of the field value
220-
* @throws java.io.IOException
221-
*/
222-
protected String invokeGetter(Configuration config, String field) throws IOException {
223-
String val = null;
224-
225-
try {
226-
PropertyDescriptor desc = new PropertyDescriptor(field, Configuration.class);
227-
Method getter = desc.getReadMethod();
228-
229-
if (getter == null) {
230-
throw new IOException(
231-
String.format("No getter for the name \"%s\".", field));
232-
}
233-
234-
if (getter.getParameterCount() != 0) {
235-
/**
236-
* Actually should not happen as it is not considered as a
237-
* read method so an exception would be thrown earlier.
238-
*/
239-
throw new IOException(
240-
String.format("The getter \"%s\" for the name \"%s\" takes a parameter.",
241-
getter.getName(), field));
242-
}
243-
244-
val = getter.invoke(config).toString();
245-
} catch (IntrospectionException
246-
| IllegalAccessException
247-
| InvocationTargetException
248-
| IllegalArgumentException ex) {
249-
throw new IOException(
250-
String.format("Unsupported operation with the configuration for name \"%s\" - %s.",
251-
field,
252-
ex.getCause() == null
253-
? ex.getLocalizedMessage()
254-
: ex.getCause().getLocalizedMessage()),
255-
ex);
256-
}
257-
258-
return val;
259-
}
260-
261-
/**
262-
* Validate the string if it contains a boolean value.
263-
*
264-
* <p>
265-
* Boolean values are (case insensitive):
266-
* <ul>
267-
* <li>false</li>
268-
* <li>off</li>
269-
* <li>0</li>
270-
* <li>true</li>
271-
* <li>on</li>
272-
* <li>1</li>
273-
* </ul>
274-
* </p>
275-
*
276-
* @param value the string value
277-
* @return if the value is boolean or not
278-
*/
279-
private boolean isBoolean(String value) {
280-
return "false".equalsIgnoreCase(value)
281-
|| "off".equalsIgnoreCase(value)
282-
|| "0".equalsIgnoreCase(value)
283-
|| "true".equalsIgnoreCase(value)
284-
|| "on".equalsIgnoreCase(value)
285-
|| "1".equalsIgnoreCase(value);
286-
287-
}
288-
289-
/**
290-
* Cast a boolean value to integer.
291-
*
292-
* @param b boolean value
293-
* @return 0 for false and 1 for true
294-
*/
295-
protected int toInteger(boolean b) {
296-
return b ? 1 : 0;
297-
}
298-
29988
@Override
30089
public void validate() throws Exception {
301-
if (toInteger(hasTag("setconf"))
302-
+ toInteger(hasTag("getconf"))
303-
+ toInteger(hasTag("auth"))
304-
+ toInteger(hasTag("get"))
305-
+ toInteger(hasTag("set")) > 1) {
306-
throw new Exception("The message tag must be either setconf, getconf, auth or set");
90+
Set<String> allowedTags = new TreeSet<>(Arrays.asList("setconf",
91+
"getconf", "auth", "get", "set"));
92+
93+
Set<String> tagCopy = new TreeSet<>(allowedTags);
94+
tagCopy.retainAll(getTags());
95+
if (tagCopy.size() > 1) {
96+
throw new Exception("The message tag must be one of '" + allowedTags.toString() + "'");
30797
}
30898

30999
if (hasTag("setconf")) {

src/org/opensolaris/opengrok/configuration/messages/ProjectMessage.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

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

src/org/opensolaris/opengrok/configuration/messages/RepositoryMessage.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,22 +45,22 @@ protected byte[] applyMessage(RuntimeEnvironment env) throws Exception {
4545

4646
switch (msgtext) {
4747
case "get-repo-type":
48-
List<String> types = new ArrayList<>();
48+
List<String> types = new ArrayList<>(16);
4949

5050
for (String tag: getTags()) {
5151
boolean found = false;
5252
for (RepositoryInfo ri : env.getRepositories()) {
5353
if (ri.getDirectoryNameRelative().equals(tag)) {
54-
types.add(ri.getType());
54+
types.add(tag + ":" + ri.getType());
5555
found = true;
5656
break;
5757
}
5858
}
5959
if (!found) {
60-
types.add("N/A");
60+
types.add(tag + ":N/A");
6161
}
6262
}
63-
ret = types.stream().collect(Collectors.joining("\n")).toString();
63+
ret = types.stream().collect(Collectors.joining("\n"));
6464
break;
6565
}
6666

@@ -75,14 +75,14 @@ protected byte[] applyMessage(RuntimeEnvironment env) throws Exception {
7575
@Override
7676
public void validate() throws Exception {
7777
String command = getText();
78-
Set<String> allowedText = new TreeSet<>(Arrays.asList("get-repo-type"));
78+
Set<String> allowedTexts = new TreeSet<>(Arrays.asList("get-repo-type"));
7979

8080
// The text field carries the command.
8181
if (command == null) {
82-
throw new Exception("The message text must contain one of '" + allowedText.toString() + "'");
82+
throw new Exception("The message text must contain one of '" + allowedTexts.toString() + "'");
8383
}
84-
if (!allowedText.contains(command)) {
85-
throw new Exception("The message text must contain one of '" + allowedText.toString() + "'");
84+
if (!allowedTexts.contains(command)) {
85+
throw new Exception("The message text must contain one of '" + allowedTexts.toString() + "'");
8686
}
8787

8888
if (getTags().size() == 0) {

0 commit comments

Comments
 (0)