From 0f0eadf55dc40bb6c4c9571e275cbc445262573a Mon Sep 17 00:00:00 2001 From: Mattia Iavarone Date: Thu, 26 Oct 2017 19:45:04 +0200 Subject: [PATCH 1/2] Improve CameraLogger, add tests --- .../cameraview/CameraLoggerTest.java | 86 +++++++++- .../cameraview/CameraLogger.java | 156 +++++++++++++----- 2 files changed, 192 insertions(+), 50 deletions(-) diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraLoggerTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraLoggerTest.java index 75ce44329..332cbfa87 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraLoggerTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraLoggerTest.java @@ -4,21 +4,52 @@ import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import static org.junit.Assert.*; +import static org.mockito.Mockito.*; @RunWith(AndroidJUnit4.class) @SmallTest public class CameraLoggerTest extends BaseTest { + private String loggerTag = "myLogger"; + private CameraLogger logger; + + @Before + public void setUp() { + CameraLogger.setLogLevel(CameraLogger.LEVEL_VERBOSE); + logger = CameraLogger.create(loggerTag); + } + + @After + public void tearDown() { + logger = null; + } + @Test public void testLoggerLevels() { - CameraLogger logger = CameraLogger.create("logger"); - // Verbose CameraLogger.setLogLevel(CameraLogger.LEVEL_VERBOSE); + logger.v("v"); + assertEquals(CameraLogger.lastMessage, "v"); + logger.i("i"); + assertEquals(CameraLogger.lastMessage, "i"); + logger.w("w"); + assertEquals(CameraLogger.lastMessage, "w"); + logger.e("e"); + assertEquals(CameraLogger.lastMessage, "e"); + + // Info + CameraLogger.lastMessage = null; + CameraLogger.setLogLevel(CameraLogger.LEVEL_INFO); + logger.v("v"); + assertNull(CameraLogger.lastMessage); logger.i("i"); assertEquals(CameraLogger.lastMessage, "i"); logger.w("w"); @@ -29,6 +60,8 @@ public void testLoggerLevels() { // Warning CameraLogger.lastMessage = null; CameraLogger.setLogLevel(CameraLogger.LEVEL_WARNING); + logger.v("v"); + assertNull(CameraLogger.lastMessage); logger.i("i"); assertNull(CameraLogger.lastMessage); logger.w("w"); @@ -39,6 +72,8 @@ public void testLoggerLevels() { // Error CameraLogger.lastMessage = null; CameraLogger.setLogLevel(CameraLogger.LEVEL_ERROR); + logger.v("v"); + assertNull(CameraLogger.lastMessage); logger.i("i"); assertNull(CameraLogger.lastMessage); logger.w("w"); @@ -48,10 +83,51 @@ public void testLoggerLevels() { } @Test - public void testLoggerObjectArray() { - CameraLogger.setLogLevel(CameraLogger.LEVEL_VERBOSE); - CameraLogger logger = CameraLogger.create("logger"); + public void testMessage() { logger.i("test", "logger", 10, null); + assertEquals(CameraLogger.lastTag, loggerTag); assertEquals(CameraLogger.lastMessage, "test logger 10 null"); } + + @Test + public void testExternal() { + CameraLogger.Logger mock = mock(CameraLogger.Logger.class); + CameraLogger.registerLogger(mock); + logger.e("hey"); + verify(mock, times(1)).log(CameraLogger.LEVEL_ERROR, loggerTag, "hey", null); + + reset(mock); + CameraLogger.unregisterLogger(mock); + logger.e("hey again"); + verify(mock, never()).log(anyInt(), anyString(), anyString(), any(Throwable.class)); + } + + @Test + public void testThrowable() { + CameraLogger.Logger mock = mock(CameraLogger.Logger.class); + CameraLogger.registerLogger(mock); + + final Task task = new Task<>(); + doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + Throwable throwable = (Throwable) args[3]; + task.end(throwable); + return null; + } + }).when(mock).log(anyInt(), anyString(), anyString(), any(Throwable.class)); + + task.listen(); + logger.e("Got no error."); + assertNull(task.await(100)); + + task.listen(); + logger.e("Got error:", new RuntimeException("")); + assertNotNull(task.await(100)); + + task.listen(); + logger.e("Got", new RuntimeException(""), "while starting"); + assertNotNull(task.await(100)); + } } diff --git a/cameraview/src/main/utils/com/otaliastudios/cameraview/CameraLogger.java b/cameraview/src/main/utils/com/otaliastudios/cameraview/CameraLogger.java index e9a69d27a..2358a53bd 100644 --- a/cameraview/src/main/utils/com/otaliastudios/cameraview/CameraLogger.java +++ b/cameraview/src/main/utils/com/otaliastudios/cameraview/CameraLogger.java @@ -1,10 +1,13 @@ package com.otaliastudios.cameraview; import android.support.annotation.IntDef; +import android.support.annotation.Nullable; import android.util.Log; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.ArrayList; +import java.util.List; /** * Utility class that can log traces and info. @@ -12,81 +15,144 @@ public final class CameraLogger { public final static int LEVEL_VERBOSE = 0; - public final static int LEVEL_WARNING = 1; - public final static int LEVEL_ERROR = 2; - - @IntDef({LEVEL_VERBOSE, LEVEL_WARNING, LEVEL_ERROR}) + public final static int LEVEL_INFO = 1; + public final static int LEVEL_WARNING = 2; + public final static int LEVEL_ERROR = 3; + + /** + * Interface of integers representing log levels. + * @see #LEVEL_VERBOSE + * @see #LEVEL_INFO + * @see #LEVEL_WARNING + * @see #LEVEL_ERROR + */ + @IntDef({LEVEL_VERBOSE, LEVEL_INFO, LEVEL_WARNING, LEVEL_ERROR}) @Retention(RetentionPolicy.SOURCE) - @interface LogLevel {} - - private static int level = LEVEL_ERROR; - - public static void setLogLevel(int logLevel) { - level = logLevel; + public @interface LogLevel {} + + /** + * A Logger can listen to internal log events + * and log them to different providers. + * The default logger will simply post to logcat. + */ + public interface Logger { + + /** + * Notifies that an internal log event was just triggered. + * + * @param level the log level + * @param tag the log tag + * @param message the log message + * @param throwable an optional throwable + */ + void log(@LogLevel int level, String tag, String message, @Nullable Throwable throwable); } static String lastMessage; static String lastTag; + private static int sLevel; + private static List sLoggers; + + static { + setLogLevel(LEVEL_ERROR); + sLoggers = new ArrayList<>(); + sLoggers.add(new Logger() { + @Override + public void log(int level, String tag, String message, @Nullable Throwable throwable) { + switch (level) { + case LEVEL_VERBOSE: Log.v(tag, message, throwable); break; + case LEVEL_INFO: Log.i(tag, message, throwable); break; + case LEVEL_WARNING: Log.w(tag, message, throwable); break; + case LEVEL_ERROR: Log.e(tag, message, throwable); break; + } + } + }); + } + static CameraLogger create(String tag) { return new CameraLogger(tag); } - private String mTag; - - private CameraLogger(String tag) { - mTag = tag; + /** + * Sets the log sLevel for logcat events. + * + * @see #LEVEL_VERBOSE + * @see #LEVEL_INFO + * @see #LEVEL_WARNING + * @see #LEVEL_ERROR + * @param logLevel the desired log sLevel + */ + public static void setLogLevel(@LogLevel int logLevel) { + sLevel = logLevel; } - void i(String message) { - if (should(LEVEL_VERBOSE)) { - Log.i(mTag, message); - lastMessage = message; - lastTag = mTag; - } + /** + * Registers an external {@link Logger} for log events. + * Make sure to unregister using {@link #unregisterLogger(Logger)}. + * + * @param logger logger to add + */ + public static void registerLogger(Logger logger) { + sLoggers.add(logger); } - void w(String message) { - if (should(LEVEL_WARNING)) { - Log.w(mTag, message); - lastMessage = message; - lastTag = mTag; - } + /** + * Unregisters a previously registered {@link Logger} for log events. + * This is needed in order to avoid leaks. + * + * @param logger logger to remove + */ + public static void unregisterLogger(Logger logger) { + sLoggers.remove(logger); } - void e(String message) { - if (should(LEVEL_ERROR)) { - Log.w(mTag, message); - lastMessage = message; - lastTag = mTag; - } + private String mTag; + + private CameraLogger(String tag) { + mTag = tag; } private boolean should(int messageLevel) { - return level <= messageLevel; + return sLevel <= messageLevel && sLoggers.size() > 0; } - private String string(int messageLevel, Object... ofData) { - String message = ""; - if (should(messageLevel)) { - for (Object o : ofData) { - message += String.valueOf(o); - message += " "; - } - } - return message.trim(); + void v(Object... data) { + log(LEVEL_VERBOSE, data); } void i(Object... data) { - i(string(LEVEL_VERBOSE, data)); + log(LEVEL_INFO, data); } void w(Object... data) { - w(string(LEVEL_WARNING, data)); + log(LEVEL_WARNING, data); } void e(Object... data) { - e(string(LEVEL_ERROR, data)); + log(LEVEL_ERROR, data); + } + + private void log(@LogLevel int level, Object... data) { + if (!should(level)) return; + + String message = ""; + Throwable throwable = null; + final int size = data.length; + for (Object object : data) { + if (object instanceof Throwable) { + throwable = (Throwable) object; + } + message += String.valueOf(object); + message += " "; + } + message = message.trim(); + for (Logger logger : sLoggers) { + logger.log(level, mTag, message, throwable); + } + + lastMessage = message; + lastTag = mTag; } } From 1e49f023ba86043598296daa82a7a09a1a28c146 Mon Sep 17 00:00:00 2001 From: Mattia Iavarone Date: Thu, 26 Oct 2017 19:51:41 +0200 Subject: [PATCH 2/2] Add log docs --- README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/README.md b/README.md index e6bea8bfd..4b3a36edf 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,7 @@ See below for a [list of what was done](#roadmap) and [licensing info](#contribu - [Other APIs](#other-apis) - [Permissions Behavior](#permissions-behavior) - [Manifest file](#manifest-file) +- [Logging](#logging) - [Roadmap](#roadmap) - [Device-specific issues](#device-specific-issues) @@ -491,6 +492,26 @@ The library manifest file is not strict and only asks for camera permissions. Th If you don't request this feature, you can use `CameraUtils.hasCameras()` to detect if current device has cameras, and then start the camera view. +## Logging + +`CameraView` will log a lot of interesting events related to the camera lifecycle. These are important +to identify bugs. The default logger will simply use Android `Log` methods posting to logcat. + +You can attach and detach external loggers using `CameraLogger.registerLogger()`: + +```java +CameraLogger.registerLogger(new Logger() { + @Override + public void log(@LogLevel int level, String tag, String message, @Nullable Throwable throwable) { + // For example... + Crashlytics.log(message); + } +}); +``` + +Make sure you enable the logger using `CameraLogger.setLogLevel(@LogLevel int)`. The default will only +log error events. + ## Roadmap This is what was done since the library was forked. I have kept the original structure, but practically all the code was changed.