Fiqare perseo-core improvements#151
Conversation
Merge by Jenkins
Merge by Jenkins
Merge by Jenkins
Merge by Jenkins
Merge by Jenkins
Merge by Jenkins
Merge by Jenkins
…re-src into emptystatements#284630
Merge by Jenkins
Merge by Jenkins
Merge by Jenkins
…Servlet.java src/main/java/com/telefonica/iot/perseo/LogLevelServlet.java src/main/java/com/telefonica/iot/perseo/RulesServlet.java
Merge by Jenkins
…uration.java src/main/resources/perseo-core.properties
Merge by Jenkins
…fiqare-emergya-dev/cdti-fiqare-perseo-core-src into fiqare-perseo-core-improvements
|
Looking to the check style log: However, I understand that should mention the telefonica_checkstyle.xml (https://github.com/telefonicaid/perseo-core/blob/master/telefonica_checkstyle.xml). It seems like you are not using the right rule set for perseo-core. |
The pom.xml file has this configuration: Running the command 'mvn checkstyle:check' gives me the results mentioned above. Is the configuration wrong or am I running it wrong? |
Indeed. Something in the configuration (pom.xml) could be wrong. Need to be investigated (probably out of the scope of this PR). |
Moved to issue #156. Thus, we'll leave checstyle check out of the scope of this PR. |
| String defaultMaxAge; | ||
| String defaultURL; | ||
| String actionPath = "/actions/do"; | ||
| String actionPath = ResourceBundle.getBundle("perseo-core").getString("action.path"); |
There was a problem hiding this comment.
Not sure if this is correct...
I mean, looking above to:
private static final String PATH = ResourceBundle.getBundle("perseo-core").getString("properties.path");
I understand that ResourceBundle.getBundle("...").getString("...") is used to get the path of a filename. However, in case, /actions/do doesn't refer to a filename, but to an URL endpoint...
There was a problem hiding this comment.
Looking to the content of perseo-main/src/main/resources/perseo-core.properties it seems I misunderstand this. Thus ResourceBundle.getBundle("...").getString("...") seems to be a way of getting properties from that file. Is my understanding correct?
There was a problem hiding this comment.
The change is due to a rule that detects hardcoded url. If you consider that the path is not a url we reverse the change.
| PrintWriter out = response.getWriter(); | ||
| try { | ||
| StringBuilder sb = new StringBuilder(); | ||
| response.setContentType("application/json;charset=UTF-8"); |
There was a problem hiding this comment.
Wrong indent... again :)
You can self-discover this kind of problems along all the modified files if you have a carefull look to https://github.com/telefonicaid/perseo-core/pull/151/files. Please, do it.
| response.setContentType("application/json"); | ||
| response.setStatus(HttpServletResponse.SC_OK); | ||
| response.getOutputStream().print("{\"level\":\""+ currentLevel +"\"}"); | ||
| try { |
| properties.path = /etc/perseo-core.properties | ||
| action.path = /actions/do |
There was a problem hiding this comment.
Both parameters should have an explanatory comment above each one (as rule.max_age does). In addition, I understand the new parameters should be included in documentation/config.md.
In addition, it seems that action.url includes a part that is the value off action.path. This is a bit weird, isn't it?
There was a problem hiding this comment.
The change is due to a rule that detects the hardcoded urls and for that reason it was decided to take them out to a configuration file. If the path is considered not to be a url, the change is reversed.
There was a problem hiding this comment.
It is an URL. However, looking how it is used in Configuration.java
actionPath is just a default to complete the URL in the case it the action URL that comes from action.url (in the configuration file) or PERSEO_FE_URL (as env var). Thus, yes, probably better to revert this particular change.
| */ | ||
| public class EventsServletTest { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(EventsServletTest.class); |
| public class LogLevelServletTest { | ||
|
|
||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(LogLevelServletTest.class); |
| jr.put("text", Help.ExampleRules()[0]); | ||
| Help.Res r = Help.sendPost(url, jr.toString(2)); | ||
| assertEquals(200, r.code); | ||
| assertEquals(200,r.getCode()); |
| */ | ||
| public class UtilsTest { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(UtilsTest.class); |
There was a problem hiding this comment.
Should be LOGGER? As in perseo-main/src/test/java/com/telefonica/iot/perseo/TimeRulesStoreTest.java
(The same in general for all the other similar files)
| public static String[] ExampleRules() { | ||
| return new String[]{ | ||
| "select id, price? as Price from iotEvent.win:length(100) group by id", | ||
| "@Audit select *,\"blood_1_action\" as iotcepaction," | ||
| + "ev.BloodPressure? as Pression, ev.id? as Meter from pattern " | ||
| + "[every ev=iotEvent(cast(cast(BloodPressure?,String),float)>1.5" | ||
| + " and type=\"BloodMeter\")]"}; | ||
|
|
||
| } | ||
| public static String[] ExampleNotices() { | ||
| return new String[]{ | ||
| "{\n" | ||
| + "\"BloodPressure\": 2,\n" | ||
| + "\"id\":\"guay!\",\n" | ||
| + "\"otro\":\"mas\",\n" | ||
| + "\"numero\":4,\n" | ||
| + "\"sub\": {\n" | ||
| + " \"subnumero\":18,\n" | ||
| + " \"subcadena\":\"SUB2\",\n" | ||
| + " \"subflotante\": 12.3,\n" | ||
| + " \"sub2\": { \"valor\": 3}\n" | ||
| + " }\n" | ||
| + "}"}; |
| * perseo-core is free software: you can redistribute it and/or modify it under the terms of the GNU | ||
| * General Public License version 2 as published by the Free Software Foundation. | ||
| * perseo-core is free software: you can redistribute it and/or | ||
| * modify it under the terms of the GNU | ||
| * General Public License version 2 as published by the Free Software | ||
| * Foundation. | ||
| * | ||
| * perseo-core is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the | ||
| * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License | ||
| * perseo-core is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the | ||
| * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. | ||
| * See the GNU General Public License | ||
| * for more details. | ||
| * | ||
| * You should have received a copy of the GNU General Public License along with perseo-core. If not, see | ||
| * You should have received a copy of the GNU General Public License | ||
| * along with perseo-core. If not, see | ||
| * http://www.gnu.org/licenses/. | ||
| * | ||
| * For those usages not covered by the GNU General Public License please contact with | ||
| * For those usages not covered by the GNU General Public License | ||
| * please contact with |
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.util.Properties; | ||
| import java.util.ResourceBundle; |
There was a problem hiding this comment.
It seem that after last changes ResourceBunle is no longer used in this file. Thus, this import line should be removed.
| @@ -25,4 +25,5 @@ action.url = http://127.0.0.1:9090/actions/do | |||
|
|
|||
There was a problem hiding this comment.
After the last changes, I understand the changes in this file should be reverted, as properties.path and action.path are no longer used in the code.
| "{\n" | ||
| + "\"BloodPressure\": 2,\n" | ||
| + "\"id\":\"guay!\",\n" | ||
| + "\"otro\":\"mas\",\n" | ||
| + "\"numero\":4,\n" | ||
| + "\"sub\": {\n" | ||
| + " \"subnumero\":18,\n" | ||
| + " \"subcadena\":\"SUB2\",\n" | ||
| + " \"subflotante\": 12.3,\n" | ||
| + " \"sub2\": { \"valor\": 3}\n" | ||
| + " }\n" | ||
| + "}"}; |
There was a problem hiding this comment.
This indent should look like more like the origina one, e.g. see the aligment of "{\n" and + "}"};
| @@ -17,4 +17,3 @@ hs_err_pid* | |||
| target | |||
There was a problem hiding this comment.
It seems the PR is close to the end :)
Thus, please add an entry in the CHANGES_NEXT_RELEASE about the changes. The following is the suggested one:
- Hardening: software quality improvement based on ISO25010 recommendations
4dc0b14
into
telefonicaid:hardening/fiqare-perseo-core-improvements-prelanding



This pull request contains the improvements made by the Emergya team for perseo-core. These improvements are part of the fiQare project, which is based on ISO 25010 to improve software quality. More info: https://fiqare.eu/
There are 95 issues:
Rule | Severity | File
squid:S1075 | MINOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/Configuration.java
squid:S1313 | MINOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/Configuration.java
squid:S1075 | MINOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/Configuration.java
squid:EmptyStatementUsageCheck | MINOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/Constants.java
findsecbugs:XSS_SERVLET | MAJOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/EventsServlet.java
squid:S1854 | MAJOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/GenericListener.java
findsecbugs:XSS_SERVLET | MAJOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/LogLevelServlet.java
squid:S3457 | MAJOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/RulesManager.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/RulesManager.java
findsecbugs:XSS_SERVLET | MAJOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/RulesServlet.java
findsecbugs:XSS_SERVLET | MAJOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/RulesServlet.java
findsecbugs:XSS_SERVLET | MAJOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/RulesServlet.java
findsecbugs:XSS_SERVLET | MAJOR | fiware-perseo-core:src/main/java/com/telefonica/iot/perseo/RulesServlet.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/CDL.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/CDL.java
squid:ForLoopCounterChangedCheck | MAJOR | fiware-perseo-core:src/main/java/org/json/Cookie.java
squid:S2589 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONArray.java
squid:S2225 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONArray.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/JSONML.java
squid:S1854 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONML.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/JSONML.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/JSONML.java
squid:S2975 | BLOCKER | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S1182 | MINOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S1206 | MINOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S2589 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S1168 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S1168 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S1168 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S1181 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S2225 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S2589 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S2159 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S2589 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S2159 | MAJOR | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/JSONObject.java
squid:SwitchLastCaseIsDefaultCheck | CRITICAL | fiware-perseo-core:src/main/java/org/json/JSONTokener.java
squid:ClassVariableVisibilityCheck | MINOR | fiware-perseo-core:src/main/java/org/json/Kim.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/Kim.java
squid:ForLoopCounterChangedCheck | MAJOR | fiware-perseo-core:src/main/java/org/json/Kim.java
squid:ForLoopCounterChangedCheck | MAJOR | fiware-perseo-core:src/main/java/org/json/Kim.java
squid:HiddenFieldCheck | MAJOR | fiware-perseo-core:src/main/java/org/json/Kim.java
squid:S1659 | MINOR | fiware-perseo-core:src/main/java/org/json/XML.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/XML.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/XML.java
squid:S1854 | MAJOR | fiware-perseo-core:src/main/java/org/json/XML.java
squid:S2386 | MINOR | fiware-perseo-core:src/main/java/org/json/XMLTokener.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/XMLTokener.java
squid:SwitchLastCaseIsDefaultCheck | CRITICAL | fiware-perseo-core:src/main/java/org/json/XMLTokener.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/XMLTokener.java
squid:SwitchLastCaseIsDefaultCheck | CRITICAL | fiware-perseo-core:src/main/java/org/json/XMLTokener.java
squid:S3776 | CRITICAL | fiware-perseo-core:src/main/java/org/json/XMLTokener.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/EventsServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/EventsServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/GenericListenerTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/GenericListenerTest.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/LogLevelServletTest.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/LogLevelServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/LogLevelServletTest.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/LogLevelServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/LogLevelServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesManagerTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesManagerTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesManagerTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesManagerTest.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/RulesServletTest.java
squid:ClassVariableVisibilityCheck | MINOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/test/EventBeanMock.java
squid:S2386 | MINOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/test/Help.java
squid:S2386 | MINOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/test/Help.java
squid:ClassVariableVisibilityCheck | MINOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/test/Help.java
squid:ClassVariableVisibilityCheck | MINOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/test/Help.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/test/Help.java
squid:S00112 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/test/Help.java
squid:ClassVariableVisibilityCheck | MINOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/test/ServletContextMock.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/TimeRulesStoreTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/UtilsTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/UtilsTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/UtilsTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/UtilsTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/UtilsTest.java
squid:S106 | MAJOR | fiware-perseo-core:src/test/java/com/telefonica/iot/perseo/UtilsTest.java
All test in Travis has been passed successfully.