You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2020/10/05 14:56:05 UTC
[GitHub] [logging-log4j2] SiegfriedTobias1 opened a new pull request #430: Fix NPE in MDCContextMap
SiegfriedTobias1 opened a new pull request #430:
URL: https://github.com/apache/logging-log4j2/pull/430
PR for https://issues.apache.org/jira/browse/LOG4J2-2939
Accomodate for the fact that MDC.getCopyOfContextMap() may return null.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] SiegfriedTobias1 commented on pull request #430: Fix NPE in MDCContextMap
Posted by GitBox <gi...@apache.org>.
SiegfriedTobias1 commented on pull request #430:
URL: https://github.com/apache/logging-log4j2/pull/430#issuecomment-703748839
Thanks for the review. I added a test, as requested.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak merged pull request #430: Fix NPE in MDCContextMap
Posted by GitBox <gi...@apache.org>.
carterkozak merged pull request #430:
URL: https://github.com/apache/logging-log4j2/pull/430
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] SiegfriedTobias1 commented on a change in pull request #430: Fix NPE in MDCContextMap
Posted by GitBox <gi...@apache.org>.
SiegfriedTobias1 commented on a change in pull request #430:
URL: https://github.com/apache/logging-log4j2/pull/430#discussion_r500188677
##########
File path: log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java
##########
@@ -165,5 +166,12 @@ public void mdc() {
assertThat(list.strList, hasSize(2));
assertTrue("Incorrect year", list.strList.get(0).startsWith("2010"));
}
+
+ @Test
+ public void mdcNullBacked() {
+ assertNull("Setup wrong", MDC.getCopyOfContextMap());
+ assertTrue(ThreadContext.isEmpty());
+ assertFalse(ThreadContext.containsKey("something"));
+ }
Review comment:
Thank you for the feedback! I split the tests and added some tests concerning null keys.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] SiegfriedTobias1 edited a comment on pull request #430: Fix NPE in MDCContextMap
Posted by GitBox <gi...@apache.org>.
SiegfriedTobias1 edited a comment on pull request #430:
URL: https://github.com/apache/logging-log4j2/pull/430#issuecomment-703748839
Thank you very much!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #430: Fix NPE in MDCContextMap
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #430:
URL: https://github.com/apache/logging-log4j2/pull/430#discussion_r499833503
##########
File path: log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java
##########
@@ -165,5 +166,12 @@ public void mdc() {
assertThat(list.strList, hasSize(2));
assertTrue("Incorrect year", list.strList.get(0).startsWith("2010"));
}
+
+ @Test
+ public void mdcNullBacked() {
+ assertNull("Setup wrong", MDC.getCopyOfContextMap());
+ assertTrue(ThreadContext.isEmpty());
+ assertFalse(ThreadContext.containsKey("something"));
+ }
Review comment:
An edge case worth testing IMO is getting a null key.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on pull request #430: Fix NPE in MDCContextMap
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #430:
URL: https://github.com/apache/logging-log4j2/pull/430#issuecomment-703748778
Thanks! I'll handle the merge and cherry-pick to relevant release branches.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #430: Fix NPE in MDCContextMap
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #430:
URL: https://github.com/apache/logging-log4j2/pull/430#discussion_r499833116
##########
File path: log4j-to-slf4j/src/test/java/org/apache/logging/slf4j/LoggerTest.java
##########
@@ -165,5 +166,12 @@ public void mdc() {
assertThat(list.strList, hasSize(2));
assertTrue("Incorrect year", list.strList.get(0).startsWith("2010"));
}
+
+ @Test
+ public void mdcNullBacked() {
+ assertNull("Setup wrong", MDC.getCopyOfContextMap());
+ assertTrue(ThreadContext.isEmpty());
+ assertFalse(ThreadContext.containsKey("something"));
+ }
Review comment:
I would separate these into two tests, one for each API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org