Skip to content

Commit dad8110

Browse files
komamitsukomamitsu
committed
Refactor the previous commit
1 parent a6ba413 commit dad8110

File tree

8 files changed

+87
-29
lines changed

8 files changed

+87
-29
lines changed

‎src/main/java/org/fluentd/logger/FluentLogger.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.HashMap;
2121
import java.util.Map;
2222

23+
import org.fluentd.logger.errorhandler.ErrorHandler;
2324
import org.fluentd.logger.sender.Reconnector;
2425
import org.fluentd.logger.sender.Sender;
2526

@@ -133,9 +134,22 @@ public boolean isConnected() {
133134
return sender != null && sender.isConnected();
134135
}
135136

136-
public synchronized void setServerErrorHandler(ServerErrorHandler serverErrorHandler) {
137+
public synchronized void setErrorHandler(ErrorHandler errorHandler) {
138+
if (errorHandler == null) {
139+
throw new IllegalArgumentException("errorHandler is null");
140+
}
141+
137142
if (sender != null) {
138-
sender.setServerErrorHandler(serverErrorHandler);
143+
sender.setErrorHandler(errorHandler);
139144
}
140145
}
146+
147+
public synchronized void removeErrorHandler() {
148+
if (sender != null) {
149+
sender.removeErrorHandler();
150+
}
151+
}
152+
153+
154+
141155
}

‎src/main/java/org/fluentd/logger/ServerErrorHandler.java‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎src/main/java/org/fluentd/logger/errorhandler/ErrorHandler.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package org.fluentd.logger.errorhandler;
2+
3+
import java.io.IOException;
4+
5+
public interface ErrorHandler {
6+
void handleNetworkError(IOException ex);
7+
}

‎src/main/java/org/fluentd/logger/errorhandler/NullErrorHandler.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package org.fluentd.logger.errorhandler;
2+
3+
import java.io.IOException;
4+
5+
public class NullErrorHandler implements ErrorHandler {
6+
@Override
7+
public void handleNetworkError(IOException ex) {
8+
// Do nothing
9+
}
10+
}

‎src/main/java/org/fluentd/logger/sender/NullSender.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
//
1818
package org.fluentd.logger.sender;
1919

20-
import org.fluentd.logger.ServerErrorHandler;
20+
import org.fluentd.logger.errorhandler.ErrorHandler;
2121

2222
import java.util.Map;
2323

@@ -59,6 +59,10 @@ public boolean isConnected() {
5959
}
6060

6161
@Override
62-
public void setServerErrorHandler(ServerErrorHandler serverErrorHandler) {
62+
public void setErrorHandler(ErrorHandler errorHandler) {
63+
}
64+
65+
@Override
66+
public void removeErrorHandler() {
6367
}
6468
}

‎src/main/java/org/fluentd/logger/sender/RawSocketSender.java‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
//
1818
package org.fluentd.logger.sender;
1919

20-
import org.fluentd.logger.ServerErrorHandler;
20+
import org.fluentd.logger.errorhandler.ErrorHandler;
21+
import org.fluentd.logger.errorhandler.NullErrorHandler;
2122
import org.msgpack.MessagePack;
2223
import org.slf4j.Logger;
2324
import org.slf4j.LoggerFactory;
@@ -34,6 +35,8 @@ public class RawSocketSender implements Sender {
3435

3536
private static final Logger LOG = LoggerFactory.getLogger(RawSocketSender.class);
3637

38+
private static final ErrorHandler DEFAULT_ERROR_HANLDER = new NullErrorHandler();
39+
3740
private MessagePack msgpack;
3841

3942
private SocketAddress server;
@@ -50,7 +53,7 @@ public class RawSocketSender implements Sender {
5053

5154
private String name;
5255

53-
private ServerErrorHandler serverErrorHandler;
56+
private ErrorHandler errorHandler = DEFAULT_ERROR_HANLDER;
5457

5558
public RawSocketSender() {
5659
this("localhost", 24224);
@@ -189,12 +192,10 @@ public synchronized void flush() {
189192
reconnector.clearErrorHistory();
190193
} catch (IOException e) {
191194
try {
192-
if (serverErrorHandler != null) {
193-
serverErrorHandler.handle(e);
194-
}
195+
errorHandler.handleNetworkError(e);
195196
}
196197
catch (Exception handlerException) {
197-
LOG.warn("ServerErrorHandler.handle error", handlerException);
198+
LOG.warn("ErrorHandler.handleNetworkError failed", handlerException);
198199
}
199200
LOG.error(this.getClass().getName(), "flush", e);
200201
reconnector.addErrorHistory(System.currentTimeMillis());
@@ -230,7 +231,16 @@ public boolean isConnected() {
230231
}
231232

232233
@Override
233-
public void setServerErrorHandler(ServerErrorHandler serverErrorHandler) {
234-
this.serverErrorHandler = serverErrorHandler;
234+
public void setErrorHandler(ErrorHandler errorHandler) {
235+
if (errorHandler == null) {
236+
throw new IllegalArgumentException("errorHandler is null");
237+
}
238+
239+
this.errorHandler = errorHandler;
240+
}
241+
242+
@Override
243+
public void removeErrorHandler() {
244+
this.errorHandler = DEFAULT_ERROR_HANLDER;
235245
}
236246
}

‎src/main/java/org/fluentd/logger/sender/Sender.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
//
1818
package org.fluentd.logger.sender;
1919

20-
import org.fluentd.logger.ServerErrorHandler;
20+
import org.fluentd.logger.errorhandler.ErrorHandler;
2121

2222
import java.util.Map;
2323

@@ -34,5 +34,7 @@ public interface Sender {
3434

3535
boolean isConnected();
3636

37-
void setServerErrorHandler(ServerErrorHandler serverErrorHandler);
37+
void setErrorHandler(ErrorHandler errorHandler);
38+
39+
void removeErrorHandler();
3840
}

‎src/test/java/org/fluentd/logger/TestFluentLogger.java‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.fluentd.logger;
22

3+
import org.fluentd.logger.errorhandler.ErrorHandler;
34
import org.fluentd.logger.sender.Event;
45
import org.fluentd.logger.sender.NullSender;
56
import org.fluentd.logger.util.MockFluentd;
@@ -228,13 +229,15 @@ public void process(MessagePack msgpack, Socket socket) throws IOException {
228229
threadManager.submit(fluentd1);
229230

230231
// start a logger
231-
FluentLogger logger = FluentLogger.getLogger("testtag", host, port);
232-
logger.setServerErrorHandler(new ServerErrorHandler() {
232+
final FluentLogger logger = FluentLogger.getLogger("testtag", host, port);
233+
final ErrorHandler errorHandler = new ErrorHandler() {
233234
@Override
234-
public void handle(IOException ex) {
235+
public void handleNetworkError(IOException ex) {
235236
lastError.set(ex);
236237
}
237-
});
238+
};
239+
logger.setErrorHandler(errorHandler);
240+
238241
assertFalse(logger.isConnected());
239242
{
240243
Map<String, Object> data = new HashMap<String, Object>();
@@ -257,6 +260,7 @@ public void handle(IOException ex) {
257260
// repeat twice to test both behaviors on socket write error and connection error
258261
assertNull(lastError.get());
259262
{
263+
// writing to the closed socket
260264
Map<String, Object> data = new HashMap<String, Object>();
261265
data.put("k3", "v3");
262266
data.put("k4", "v4");
@@ -268,6 +272,20 @@ public void handle(IOException ex) {
268272
TimeUnit.MILLISECONDS.sleep(100);
269273
}
270274

275+
// the logger shouldn't call the error handler after calling removeErrorHandler()
276+
logger.removeErrorHandler();
277+
{
278+
// writing to the closed socket
279+
Map<String, Object> data = new HashMap<String, Object>();
280+
data.put("k3", "v3");
281+
data.put("k4", "v4");
282+
logger.log("test01", data);
283+
}
284+
assertNull(lastError.get());
285+
lastError.set(null); // Clear the last error
286+
287+
logger.setErrorHandler(errorHandler);
288+
271289
// run the fluentd again
272290
final List<Event> elist2 = new ArrayList<Event>();
273291
MockFluentd fluentd2 = new MockFluentd(port, new MockFluentd.MockProcess() {
@@ -313,10 +331,10 @@ public void process(MessagePack msgpack, Socket socket) throws IOException {
313331
assertEquals(1, elist1.size());
314332
assertEquals("testtag.test01", elist1.get(0).tag);
315333

316-
assertEquals(3, elist2.size());
317-
assertEquals("testtag.test01", elist2.get(0).tag);
318-
assertEquals("testtag.test01", elist2.get(1).tag);
319-
assertEquals("testtag.test01", elist2.get(2).tag);
334+
assertEquals(4, elist2.size());
335+
for (int i = 0; i < elist2.size(); i++) {
336+
assertEquals("testtag.test01", elist2.get(i).tag);
337+
}
320338
}
321339

322340
@Test

0 commit comments

Comments
 (0)