Skip to content

Commit 102e45a

Browse files
nukembergnukemberg
committed
Fixed too eager field/method fetch, add tests
1 parent c976d67 commit 102e45a

File tree

4 files changed

+66
-21
lines changed

4 files changed

+66
-21
lines changed

‎src/main/java/net/logstash/log4j/JSONEventLayoutV0.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ public String format(LoggingEvent loggingEvent) {
119119
}
120120

121121
private void addObjectFieldData(Object messageObj) {
122-
Field[] fields = messageObj.getClass().getFields();
123-
Object value = null;
122+
Field[] fields = messageObj.getClass().getDeclaredFields();
123+
Object value;
124124

125125
for(Field f : fields) {
126126
try {
@@ -129,17 +129,17 @@ private void addObjectFieldData(Object messageObj) {
129129
} catch (IllegalAccessException e) {
130130
}
131131
}
132-
Method[] methods = messageObj.getClass().getMethods();
132+
Method[] methods = messageObj.getClass().getDeclaredMethods();
133133
for(Method m : methods)
134134
{
135135
if(m.getName().startsWith("get"))
136136
{
137137
try {
138138
value = m.invoke(messageObj);
139+
if (value != null) fieldData.put(m.getName().substring(3), value);
139140
} catch (IllegalAccessException e) {
140141
} catch (InvocationTargetException e) {
141142
}
142-
if (value != null) fieldData.put(m.getName().substring(3), value);
143143
}
144144
}
145145
}
@@ -174,7 +174,11 @@ public void setLocationInfo(boolean locationInfo) {
174174
public void setRenderObjectFields(boolean renderObjectFields) {
175175
this.renderObjectFields = renderObjectFields;
176176
}
177-
177+
178+
public boolean getRenderObjectFields() {
179+
return renderObjectFields;
180+
}
181+
178182
public void activateOptions() {
179183
activeIgnoreThrowable = ignoreThrowable;
180184
}

‎src/main/java/net/logstash/log4j/JSONEventLayoutV1.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,10 @@ public void setRenderObjectFields(boolean renderObjectFields) {
184184
this.renderObjectFields = renderObjectFields;
185185
}
186186

187+
public boolean getRenderObjectFields() {
188+
return renderObjectFields;
189+
}
190+
187191
public String getUserFields() { return customUserFields; }
188192
public void setUserFields(String userFields) { this.customUserFields = userFields; }
189193

@@ -211,27 +215,24 @@ private void addEventData(String keyname, Object keyval) {
211215
}
212216

213217
private void addObjectFieldData(Object messageObj) {
214-
Field[] fields = messageObj.getClass().getFields();
215-
Object value = null;
218+
Field[] fields = messageObj.getClass().getDeclaredFields();
216219

217220
for(Field f : fields) {
218221
try {
219-
value = f.get(messageObj);
220-
if (value != null) addEventData(f.getName(), value);
222+
addEventData(f.getName(), f.get(messageObj));
221223
} catch (IllegalAccessException e) {
222224
}
223225
}
224-
Method[] methods = messageObj.getClass().getMethods();
226+
Method[] methods = messageObj.getClass().getDeclaredMethods();
225227
for(Method m : methods)
226228
{
227229
if(m.getName().startsWith("get"))
228230
{
229231
try {
230-
value = m.invoke(messageObj);
232+
addEventData(m.getName().substring(3), m.invoke(messageObj));
231233
} catch (IllegalAccessException e) {
232234
} catch (InvocationTargetException e) {
233235
}
234-
if (value != null) addEventData(m.getName().substring(3), value);
235236
}
236237
}
237238
}

‎src/test/java/net/logstash/log4j/JSONEventLayoutV0Test.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import org.junit.Ignore;
1313
import org.junit.Test;
1414

15+
import java.io.Serializable;
16+
1517
/**
1618
* Created with IntelliJ IDEA.
1719
* User: jvincent
@@ -158,6 +160,25 @@ public void testJSONEventHasThreadName() {
158160
Assert.assertNotNull("ThreadName value is missing", atFields.get("threadName"));
159161
}
160162

163+
@Test
164+
public void testMessageObjectRendering() {
165+
JSONEventLayoutV0 layout = (JSONEventLayoutV0) appender.getLayout();
166+
boolean prevRenderObjectFields = layout.getRenderObjectFields();
167+
layout.setRenderObjectFields(true);
168+
logger.info(new Serializable() {
169+
String test = "TEST";
170+
int testNum = 1123;
171+
});
172+
String message = appender.getMessages()[0];
173+
Object obj = JSONValue.parse(message);
174+
JSONObject jsonObject = (JSONObject) obj;
175+
JSONObject atFields = (JSONObject) jsonObject.get("@fields");
176+
Assert.assertTrue(atFields.containsKey("testNum"));
177+
Assert.assertTrue(atFields.get("testNum") instanceof Integer);
178+
Assert.assertEquals(atFields.get("testNum"), 1123);
179+
layout.setRenderObjectFields(prevRenderObjectFields);
180+
}
181+
161182
@Test
162183
public void testJSONEventLayoutNoLocationInfo() {
163184
JSONEventLayoutV0 layout = (JSONEventLayoutV0) appender.getLayout();

‎src/test/java/net/logstash/log4j/JSONEventLayoutV1Test.java‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,16 @@
33
import junit.framework.Assert;
44
import net.minidev.json.JSONObject;
55
import net.minidev.json.JSONValue;
6-
import org.apache.log4j.*;
7-
import org.apache.log4j.or.ObjectRenderer;
6+
import org.apache.log4j.Level;
7+
import org.apache.log4j.Logger;
8+
import org.apache.log4j.MDC;
9+
import org.apache.log4j.NDC;
810
import org.junit.After;
9-
import org.junit.Before;
10-
import org.junit.AfterClass;
1111
import org.junit.BeforeClass;
1212
import org.junit.Ignore;
1313
import org.junit.Test;
1414

15+
import java.io.Serializable;
1516
import java.util.HashMap;
1617

1718
/**
@@ -69,7 +70,7 @@ public void testJSONEventLayoutHasUserFieldsFromProps() {
6970
Assert.assertTrue("Event is not valid JSON", JSONValue.isValidJsonStrict(message));
7071
Object obj = JSONValue.parse(message);
7172
JSONObject jsonObject = (JSONObject) obj;
72-
Assert.assertTrue("Event does not contain field 'field1'" , jsonObject.containsKey("field1"));
73+
Assert.assertTrue("Event does not contain field 'field1'", jsonObject.containsKey("field1"));
7374
Assert.assertEquals("Event does not contain value 'value1'", "propval1", jsonObject.get("field1"));
7475
System.clearProperty(JSONEventLayoutV1.ADDITIONAL_DATA_PROPERTY);
7576
}
@@ -125,7 +126,7 @@ public void testJSONEventLayoutUserFieldsPropOverride() {
125126
Assert.assertTrue("Event is not valid JSON", JSONValue.isValidJsonStrict(message));
126127
Object obj = JSONValue.parse(message);
127128
JSONObject jsonObject = (JSONObject) obj;
128-
Assert.assertTrue("Event does not contain field 'field1'" , jsonObject.containsKey("field1"));
129+
Assert.assertTrue("Event does not contain field 'field1'", jsonObject.containsKey("field1"));
129130
Assert.assertEquals("Event does not contain value 'propval1'", "propval1", jsonObject.get("field1"));
130131

131132
layout.setUserFields(prevUserData);
@@ -144,6 +145,24 @@ public void testJSONEventLayoutHasKeys() {
144145
}
145146
}
146147

148+
@Test
149+
public void testMessageObjectRendering() {
150+
JSONEventLayoutV1 layout = (JSONEventLayoutV1) appender.getLayout();
151+
boolean prevRenderObjectFields = layout.getRenderObjectFields();
152+
layout.setRenderObjectFields(true);
153+
logger.info(new Serializable() {
154+
String test = "TEST";
155+
int testNum = 1123;
156+
});
157+
String message = appender.getMessages()[0];
158+
Object obj = JSONValue.parse(message);
159+
JSONObject jsonObject = (JSONObject) obj;
160+
Assert.assertTrue(jsonObject.containsKey("testNum"));
161+
Assert.assertTrue(jsonObject.get("testNum") instanceof Integer);
162+
Assert.assertEquals(jsonObject.get("testNum"), 1123);
163+
layout.setRenderObjectFields(prevRenderObjectFields);
164+
}
165+
147166
@Test
148167
public void testJSONEventLayoutHasNDC() {
149168
String ndcData = new String("json-layout-test");
@@ -165,14 +184,14 @@ public void testJSONEventLayoutHasMDC() {
165184
JSONObject jsonObject = (JSONObject) obj;
166185
JSONObject mdc = (JSONObject) jsonObject.get("mdc");
167186

168-
Assert.assertEquals("MDC is wrong","bar", mdc.get("foo"));
187+
Assert.assertEquals("MDC is wrong", "bar", mdc.get("foo"));
169188
}
170189

171190
@Test
172191
public void testJSONEventLayoutHasNestedMDC() {
173192
HashMap nestedMdc = new HashMap<String, String>();
174-
nestedMdc.put("bar","baz");
175-
MDC.put("foo",nestedMdc);
193+
nestedMdc.put("bar", "baz");
194+
MDC.put("foo", nestedMdc);
176195
logger.warn("I should have nested MDC data in my log");
177196
String message = appender.getMessages()[0];
178197
Object obj = JSONValue.parse(message);

0 commit comments

Comments
 (0)