-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fiqare perseo-core improvements #151
Changes from 74 commits
6d25f8d
ab122f2
f651423
a1c8d23
fa2197f
99263f2
5c7825b
cbfad26
18d5393
c4c89f3
04e9cbb
e90be87
9dc7421
032f6b8
c361bf3
8ff7828
8050a3d
86f9044
6fa32ad
7de5632
40094b8
3e386ac
2ad08b7
bc7419d
30ef6b4
4ab8d47
149d591
cd90313
5e43ec8
65ac79d
7ccb457
c0dfb12
dd1ad0e
5852c95
0ac3d7e
33c8328
e53c7eb
4eb356c
7883507
1395a50
b510393
4ddbdb4
6e26193
5adca52
f73d8f1
d08fdcd
b694d53
ed78f2f
8e42d0e
f94f1bf
b38f18b
5491d95
fb3005d
a17edf1
9eb801a
0413a41
5b174d7
6ad2461
97b23e9
7a1e6e5
90a8cba
5c7b891
64f4713
a5ff18a
660056e
654f7bc
d3cd3f4
4d08aff
05982ba
c8154a4
61b8a4c
cff907d
47ab836
b4a4d73
b6d7148
728e600
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,4 +17,3 @@ hs_err_pid* | |
target | ||
nb-configuration.xml | ||
nbactions.xml | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,4 +36,4 @@ install: | |
- mvn test -B | ||
|
||
before_install: | ||
- mvn clean | ||
- mvn clean |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,11 @@ | ||
digraph perseocore { | ||
rankdir=LR | ||
rankdir=LR | ||
node [fontname = "arial"]; | ||
perseo[shape=circle, style=dashed]; | ||
core[label=" core ", shape=circle]; | ||
core[label=" core ", shape=circle]; | ||
|
||
perseo->core[label="rule"] | ||
perseo->core[label="event"] | ||
core->perseo[label="action"] | ||
perseo->core[label="rule"] | ||
perseo->core[label="event"] | ||
core->perseo[label="action"] | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,6 +78,16 @@ | |
<groupId>com.telefonica.iot</groupId> | ||
<version>1.5.0-SNAPSHOT</version> | ||
</dependency> | ||
<dependency> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You explained why org.owasp.encoder dependency is needed at #151 (comment). However, what about org.json? Is this really needed (not sure, but it seem that in the first version of the PR this dependency didn't was included)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We included this dependency because we detected that the package org.json was included directly in the packages of the project. This dependency was outdated and also generated a number of issues (37). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well done! Better to use existing jar files in maven than having the source code of a third-party library in the repository itself. NTC |
||
<groupId>org.json</groupId> | ||
<artifactId>json</artifactId> | ||
<version>20180813</version> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.owasp.encoder</groupId> | ||
<artifactId>encoder</artifactId> | ||
<version>1.2.2</version> | ||
</dependency> | ||
</dependencies> | ||
|
||
</project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,8 @@ | |
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.util.Properties; | ||
import java.util.ResourceBundle; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seem that after last changes ResourceBunle is no longer used in this file. Thus, this import line should be removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in b6d7148 |
||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
|
@@ -53,7 +55,7 @@ private Configuration() { | |
private static long maxAge; | ||
|
||
static { | ||
LOGGER.debug("Configuration init: " + reload()); | ||
LOGGER.debug(String.format("Configuration init: %s",reload())); | ||
} | ||
|
||
/** | ||
|
@@ -95,21 +97,21 @@ public static synchronized boolean reload() { | |
// Add actions/do path if perseoFeURLEnv not contains it yet | ||
actionRule = perseoFeURLEnv.contains(actionPath) ? perseoFeURLEnv : perseoFeURLEnv + actionPath; | ||
} else { | ||
LOGGER.error("Invalid value for " + PERSEO_FE_URL_ENV + ": " + perseoFeURLEnv); | ||
LOGGER.error(String.format("Invalid value for %s: %s",PERSEO_FE_URL_ENV, perseoFeURLEnv)); | ||
return false; | ||
} | ||
LOGGER.info("actionRule configuration is: " + actionRule); | ||
LOGGER.info(String.format("actionRule configuration is: %s",actionRule)); | ||
|
||
// Get MAX_AGE from env var if exist, else default | ||
String maxAgeEnv = System.getenv(PERSEO_MAX_AGE_ENV); | ||
// Check maxAge numerical value | ||
try { | ||
maxAge = maxAgeEnv != null ? Long.parseLong(maxAgeEnv) : Long.parseLong(defaultMaxAge); | ||
} catch (NumberFormatException nfe) { | ||
LOGGER.error("Invalid value for " + PERSEO_MAX_AGE_ENV + ": " + nfe); | ||
LOGGER.error(String.format("Invalid value for %s: %s",PERSEO_MAX_AGE_ENV,nfe)); | ||
return false; | ||
} | ||
LOGGER.info("maxAge configuration is: " + maxAge); | ||
LOGGER.info(String.format("maxAge configuration is: %s",maxAge)); | ||
|
||
return true; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,7 @@ | |
* @author brox | ||
*/ | ||
public final class Constants { | ||
private Constants() { }; | ||
private Constants() {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this change is needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 05982ba There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have had a look to 05982ba and it seems perseo-main/src/main/java/com/telefonica/iot/perseo/Constants.java file is not modified in that commit... The fix seems to be pending. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still pending... |
||
public static final String SERVICE_FIELD = "service"; | ||
public static final String SUBSERVICE_FIELD = "subservice"; | ||
public static final String CORRELATOR_HEADER = "fiware-correlator"; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ | |
import org.apache.log4j.MDC; | ||
import org.json.JSONException; | ||
import org.json.JSONObject; | ||
import org.owasp.encoder.Encode; | ||
|
||
/** | ||
* | ||
|
@@ -42,7 +43,7 @@ | |
public class EventsServlet extends HttpServlet { | ||
|
||
private static final Logger logger = LoggerFactory.getLogger(EventsServlet.class); | ||
EPServiceProvider epService; | ||
private static EPServiceProvider epService; | ||
|
||
@Override | ||
public void init() { | ||
|
@@ -76,25 +77,25 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) | |
Utils.putCorrelatorAndTrans(request); | ||
logger.debug("events doPost"); | ||
response.setCharacterEncoding("UTF-8"); | ||
response.setContentType("application/json;charset=UTF-8"); | ||
PrintWriter out = response.getWriter(); | ||
response.setContentType("application/json;charset=UTF-8"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 47ab836 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Travis ci fixed in b4a4d73 |
||
try { | ||
StringBuilder sb = new StringBuilder(); | ||
String eventText = Utils.getBodyAsString(request); | ||
logger.info("incoming event:" + eventText); | ||
logger.info(String.format("incoming event: %s", eventText)); | ||
org.json.JSONObject jo = new JSONObject(eventText); | ||
logger.debug("event as JSONObject: " + jo); | ||
logger.debug(String.format("event as JSONObject: %s", jo)); | ||
Map<String, Object> eventMap = Utils.JSONObject2Map(jo); | ||
logger.debug("event as map: " + eventMap); | ||
logger.debug(String.format("event as map: %s" , eventMap)); | ||
epService.getEPRuntime().sendEvent(eventMap, Constants.IOT_EVENT); | ||
logger.debug("event was sent: " + eventMap); | ||
} catch (JSONException je) { | ||
logger.error("error: " + je); | ||
response.setStatus(HttpServletResponse.SC_BAD_REQUEST); | ||
out.printf("{\"error\":\"%s\"}\n", je.getMessage()); | ||
|
||
} finally { | ||
out.close(); | ||
logger.debug(String.format("event was sent: %s", eventMap)); | ||
} catch (Exception je) { | ||
try { | ||
logger.error(String.format("error: %s" ,je)); | ||
response.setStatus(HttpServletResponse.SC_BAD_REQUEST); | ||
response.getOutputStream().print(String.format("{\"error\":\"%s\"}%n", Encode.forHtmlContent(je.getMessage()))); | ||
} catch (IOException exception) { | ||
response.setStatus(HttpServletResponse.SC_BAD_REQUEST); | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ | |
import javax.servlet.http.HttpServletResponse; | ||
import org.apache.log4j.Level; | ||
import org.apache.log4j.LogManager; | ||
import org.owasp.encoder.Encode; | ||
|
||
|
||
/** | ||
|
@@ -65,21 +66,28 @@ public class LogLevelServlet extends HttpServlet { | |
@Override | ||
protected void doPut(HttpServletRequest request, HttpServletResponse response) | ||
throws ServletException, IOException { | ||
String levelName = request.getParameter("level"); | ||
logger.info("changing log level to " + levelName); | ||
Level level = levels.get(levelName); | ||
if (level == null) { | ||
logger.error("invalid log level: " + levelName); | ||
response.setCharacterEncoding("UTF-8"); | ||
response.setContentType("application/json"); | ||
try { | ||
String levelName = request.getParameter("level"); | ||
logger.info(String.format("changing log level to %s",levelName)); | ||
Level level = levels.get(levelName); | ||
if (level == null) { | ||
logger.error(String.format("invalid log level: %s",levelName)); | ||
response.setCharacterEncoding("UTF-8"); | ||
response.setContentType("application/json"); | ||
response.setStatus(HttpServletResponse.SC_BAD_REQUEST); | ||
response.getOutputStream().print("{\"errorMessage\":\"invalid log level\"}"); | ||
return; | ||
} | ||
synchronized (mutex) { | ||
LogManager.getRootLogger().setLevel(level); | ||
} | ||
response.setStatus(HttpServletResponse.SC_OK); | ||
} catch (IOException e) { | ||
logger.error("IOException in log level"); | ||
response.setStatus(HttpServletResponse.SC_BAD_REQUEST); | ||
response.getOutputStream().print("{\"errorMessage\":\"invalid log level\"}"); | ||
return; | ||
} | ||
synchronized (mutex) { | ||
LogManager.getRootLogger().setLevel(level); | ||
} | ||
response.setStatus(HttpServletResponse.SC_OK); | ||
|
||
} | ||
|
||
/** | ||
|
@@ -95,11 +103,18 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) | |
throws ServletException, IOException { | ||
logger.debug("getting log level"); | ||
synchronized (mutex) { | ||
String currentLevel = LogManager.getRootLogger().getLevel().toString(); | ||
response.setCharacterEncoding("UTF-8"); | ||
response.setContentType("application/json"); | ||
response.setStatus(HttpServletResponse.SC_OK); | ||
response.getOutputStream().print("{\"level\":\""+ currentLevel +"\"}"); | ||
try { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong indent There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 47ab836 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Travis ci fixed in b4a4d73 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
String currentLevel = LogManager.getRootLogger().getLevel().toString(); | ||
response.setCharacterEncoding("UTF-8"); | ||
response.setContentType("application/json"); | ||
response.setStatus(HttpServletResponse.SC_OK); | ||
response.getOutputStream().print(String.format("{\"level\":\"%s\"}",Encode.forHtmlContent(currentLevel))); | ||
} catch (IOException e) { | ||
logger.error("IOException in log level"); | ||
response.setStatus(HttpServletResponse.SC_BAD_REQUEST); | ||
return; | ||
} | ||
|
||
} | ||
} | ||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b6d7148