[voice] Simplify lifecycle by using constructor injection (#1343)

* [voice] Simplify lifecycle by using constructor injection
* Removed usage of org.apache.commons.lang.ArrayUtils

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioConsoleCommandExtension.java

@@ -19,7 +19,6 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
-import org.apache.commons.lang.ArrayUtils;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.core.audio.AudioException;
@@ -83,7 +82,7 @@ public class AudioConsoleCommandExtension extends AbstractConsoleCommandExtensio
             switch (subCommand) {
                 case SUBCMD_PLAY:
                     if (args.length > 1) {
-                        play((String[]) ArrayUtils.subarray(args, 1, args.length), console);
+                        play(Arrays.copyOfRange(args, 1, args.length), console);
                     } else {
                         console.println(
                                 "Specify file to play, and optionally the sink(s) to use (e.g. 'play javasound hello.mp3')");
@@ -91,7 +90,7 @@ public class AudioConsoleCommandExtension extends AbstractConsoleCommandExtensio
                     return;
                 case SUBCMD_STREAM:
                     if (args.length > 1) {
-                        stream((String[]) ArrayUtils.subarray(args, 1, args.length), console);
+                        stream(Arrays.copyOfRange(args, 1, args.length), console);
                     } else {
                         console.println("Specify url to stream from, and optionally the sink(s) to use");
                     }

bundles/org.openhab.core.audio/src/main/java/org/openhab/core/audio/internal/AudioServlet.java

@@ -61,6 +61,11 @@ public class AudioServlet extends SmartHomeServlet implements AudioHTTPServer {
 
     private final Map<String, Long> streamTimeouts = new ConcurrentHashMap<>();
 
+    @Activate
+    public AudioServlet(final @Reference HttpService httpService) {
+        this.httpService = httpService;
+    }
+
     @Activate
     protected void activate() {
         super.activate(SERVLET_NAME);
@@ -71,17 +76,6 @@ public class AudioServlet extends SmartHomeServlet implements AudioHTTPServer {
         super.deactivate(SERVLET_NAME);
     }
 
-    @Override
-    @Reference
-    protected void setHttpService(HttpService httpService) {
-        super.setHttpService(httpService);
-    }
-
-    @Override
-    public void unsetHttpService(HttpService httpService) {
-        super.unsetHttpService(httpService);
-    }
-
     private @Nullable InputStream prepareInputStream(final String streamId, final HttpServletResponse resp)
             throws AudioException {
         final AudioStream stream;

bundles/org.openhab.core.audio/src/test/java/org/openhab/core/audio/internal/AbstractAudioServletTest.java

@@ -13,6 +13,7 @@
 package org.openhab.core.audio.internal;
 
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
 
 import java.util.concurrent.CompletableFuture;
 
@@ -30,6 +31,7 @@ import org.openhab.core.audio.FixedLengthAudioStream;
 import org.openhab.core.test.TestPortUtil;
 import org.openhab.core.test.TestServer;
 import org.openhab.core.test.java.JavaTest;
+import org.osgi.service.http.HttpService;
 
 /**
  * Base class for tests using the {@link AudioServlet}.
@@ -52,7 +54,7 @@ public abstract class AbstractAudioServletTest extends JavaTest {
 
     @Before
     public void setupServerAndClient() {
-        audioServlet = new AudioServlet();
+        audioServlet = new AudioServlet(mock(HttpService.class));
 
         ServletHolder servletHolder = new ServletHolder(audioServlet);
 

bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceConsoleCommandExtension.java

@@ -16,7 +16,6 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
 
-import org.apache.commons.lang.ArrayUtils;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.core.i18n.LocaleProvider;
 import org.openhab.core.io.console.Console;
@@ -30,6 +29,7 @@ import org.openhab.core.voice.TTSService;
 import org.openhab.core.voice.Voice;
 import org.openhab.core.voice.VoiceManager;
 import org.openhab.core.voice.text.InterpretationException;
+import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
 
@@ -46,12 +46,17 @@ public class VoiceConsoleCommandExtension extends AbstractConsoleCommandExtensio
     private static final String SUBCMD_INTERPRET = "interpret";
     private static final String SUBCMD_VOICES = "voices";
 
-    private ItemRegistry itemRegistry;
+    private final ItemRegistry itemRegistry;
-    private LocaleProvider localeProvider;
+    private final VoiceManager voiceManager;
-    private VoiceManager voiceManager;
+    private final LocaleProvider localeProvider;
 
-    public VoiceConsoleCommandExtension() {
+    @Activate
+    public VoiceConsoleCommandExtension(final @Reference VoiceManager voiceManager,
+            final @Reference LocaleProvider localeProvider, final @Reference ItemRegistry itemRegistry) {
         super("voice", "Commands around voice enablement features.");
+        this.voiceManager = voiceManager;
+        this.localeProvider = localeProvider;
+        this.itemRegistry = itemRegistry;
     }
 
     @Override
@@ -68,14 +73,14 @@ public class VoiceConsoleCommandExtension extends AbstractConsoleCommandExtensio
             switch (subCommand) {
                 case SUBCMD_SAY:
                     if (args.length > 1) {
-                        say((String[]) ArrayUtils.subarray(args, 1, args.length), console);
+                        say(Arrays.copyOfRange(args, 1, args.length), console);
                     } else {
                         console.println("Specify text to say (e.g. 'say hello')");
                     }
                     return;
                 case SUBCMD_INTERPRET:
                     if (args.length > 1) {
-                        interpret((String[]) ArrayUtils.subarray(args, 1, args.length), console);
+                        interpret(Arrays.copyOfRange(args, 1, args.length), console);
                     } else {
                         console.println("Specify text to interpret (e.g. 'interpret turn all lights off')");
                     }
@@ -152,31 +157,4 @@ public class VoiceConsoleCommandExtension extends AbstractConsoleCommandExtensio
         voiceManager.say(msg.toString());
     }
 
-    @Reference
-    protected void setItemRegistry(ItemRegistry itemRegistry) {
-        this.itemRegistry = itemRegistry;
-    }
-
-    protected void unsetItemRegistry(ItemRegistry itemRegistry) {
-        this.itemRegistry = null;
-    }
-
-    @Reference
-    protected void setLocaleProvider(LocaleProvider localeProvider) {
-        this.localeProvider = localeProvider;
-    }
-
-    protected void unsetLocaleProvider(LocaleProvider localeProvider) {
-        this.localeProvider = null;
-    }
-
-    @Reference
-    protected void setVoiceManager(VoiceManager voiceManager) {
-        this.voiceManager = voiceManager;
-    }
-
-    protected void unsetVoiceManager(VoiceManager voiceManager) {
-        this.voiceManager = null;
-    }
-
 }

bundles/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceManagerImpl.java

@@ -54,7 +54,6 @@ import org.openhab.core.voice.text.InterpretationException;
 import org.osgi.framework.Constants;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
-import org.osgi.service.component.annotations.Deactivate;
 import org.osgi.service.component.annotations.Modified;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.component.annotations.ReferenceCardinality;
@@ -103,31 +102,35 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider {
     private final Map<String, TTSService> ttsServices = new HashMap<>();
     private final Map<String, HumanLanguageInterpreter> humanLanguageInterpreters = new HashMap<>();
 
-    private LocaleProvider localeProvider = null;
+    private final LocaleProvider localeProvider;
+    private final AudioManager audioManager;
+    private final EventPublisher eventPublisher;
 
     /**
      * default settings filled through the service configuration
      */
     private String keyword = DEFAULT_KEYWORD;
-    private String listeningItem = null;
+    private String listeningItem;
-    private String defaultTTS = null;
+    private String defaultTTS;
-    private String defaultSTT = null;
+    private String defaultSTT;
-    private String defaultKS = null;
+    private String defaultKS;
-    private String defaultHLI = null;
+    private String defaultHLI;
-    private String defaultVoice = null;
+    private String defaultVoice;
     private final Map<String, String> defaultVoices = new HashMap<>();
-    private AudioManager audioManager;
+
-    private EventPublisher eventPublisher;
+    @Activate
+    public VoiceManagerImpl(final @Reference LocaleProvider localeProvider, final @Reference AudioManager audioManager,
+            final @Reference EventPublisher eventPublisher) {
+        this.localeProvider = localeProvider;
+        this.audioManager = audioManager;
+        this.eventPublisher = eventPublisher;
+    }
 
     @Activate
     protected void activate(Map<String, Object> config) {
         modified(config);
     }
 
-    @Deactivate
-    protected void deactivate() {
-    }
-
     @Modified
     protected void modified(Map<String, Object> config) {
         if (config != null) {
@@ -360,8 +363,8 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider {
 
             // If required set BigEndian, BitDepth, BitRate, and Frequency to default values
             if (null == format.isBigEndian()) {
-                format = new AudioFormat(format.getContainer(), format.getCodec(), new Boolean(true),
+                format = new AudioFormat(format.getContainer(), format.getCodec(), Boolean.TRUE, format.getBitDepth(),
-                        format.getBitDepth(), format.getBitRate(), format.getFrequency());
+                        format.getBitRate(), format.getFrequency());
             }
             if (null == format.getBitDepth() || null == format.getBitRate() || null == format.getFrequency()) {
                 // Define default values
@@ -376,19 +379,19 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider {
                 // These values must be interdependent (bitRate = bitDepth * frequency)
                 if (null == bitRate) {
                     if (null == bitDepth) {
-                        bitDepth = new Integer(defaultBitDepth);
+                        bitDepth = Integer.valueOf(defaultBitDepth);
                     }
                     if (null == frequency) {
-                        frequency = new Long(defaultFrequency);
+                        frequency = Long.valueOf(defaultFrequency);
                     }
-                    bitRate = new Integer(bitDepth.intValue() * frequency.intValue());
+                    bitRate = Integer.valueOf(bitDepth.intValue() * frequency.intValue());
                 } else if (null == bitDepth) {
                     if (null == frequency) {
-                        frequency = new Long(defaultFrequency);
+                        frequency = Long.valueOf(defaultFrequency);
                     }
-                    bitDepth = new Integer(bitRate.intValue() / frequency.intValue());
+                    bitDepth = Integer.valueOf(bitRate.intValue() / frequency.intValue());
                 } else if (null == frequency) {
-                    frequency = new Long(bitRate.longValue() / bitDepth.longValue());
+                    frequency = Long.valueOf(bitRate.longValue() / bitDepth.longValue());
                 }
 
                 format = new AudioFormat(format.getContainer(), format.getCodec(), format.isBigEndian(), bitDepth,
@@ -485,15 +488,6 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider {
         }
     }
 
-    @Reference
-    protected void setLocaleProvider(LocaleProvider localeProvider) {
-        this.localeProvider = localeProvider;
-    }
-
-    protected void unsetLocaleProvider(LocaleProvider localeProvider) {
-        this.localeProvider = null;
-    }
-
     @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
     protected void addKSService(KSService ksService) {
         this.ksServices.put(ksService.getId(), ksService);
@@ -530,24 +524,6 @@ public class VoiceManagerImpl implements VoiceManager, ConfigOptionProvider {
         this.humanLanguageInterpreters.remove(humanLanguageInterpreter.getId());
     }
 
-    @Reference
-    protected void setAudioManager(AudioManager audioManager) {
-        this.audioManager = audioManager;
-    }
-
-    protected void unsetAudioManager(AudioManager audioManager) {
-        this.audioManager = null;
-    }
-
-    @Reference
-    protected void setEventPublisher(EventPublisher eventPublisher) {
-        this.eventPublisher = eventPublisher;
-    }
-
-    protected void unsetEventPublisher(EventPublisher eventPublisher) {
-        this.eventPublisher = null;
-    }
-
     @Override
     public TTSService getTTS() {
         TTSService tts = null;

itests/org.openhab.core.voice.tests/src/main/java/org/openhab/core/voice/javavoicemanager/VoiceManagerTest.java

@@ -33,7 +33,6 @@ import org.openhab.core.test.java.JavaOSGiTest;
 import org.openhab.core.voice.VoiceManager;
 import org.openhab.core.voice.internal.AudioManagerStub;
 import org.openhab.core.voice.internal.AudioSourceStub;
-import org.openhab.core.voice.internal.ConsoleStub;
 import org.openhab.core.voice.internal.HumanLanguageInterpreterStub;
 import org.openhab.core.voice.internal.KSServiceStub;
 import org.openhab.core.voice.internal.STTServiceStub;
@@ -59,8 +58,7 @@ public class VoiceManagerTest extends JavaOSGiTest {
     private static final String CONFIG_DEFAULT_VOICE = "defaultVoice";
     private static final String CONFIG_DEFAULT_TTS = "defaultTTS";
     private static final String CONFIG_KEYWORD = "keyword";
-    private VoiceManagerImpl voiceManager = new VoiceManagerImpl();
+    private VoiceManagerImpl voiceManager;
-    private ConsoleStub stubConsole;
     private SinkStub sink;
     private TTSServiceStub ttsService;
     private VoiceStub voice;
@@ -134,7 +132,6 @@ public class VoiceManagerTest extends JavaOSGiTest {
 
     @Test
     public void interpretSomethingWithGivenHliIdWhenTheHliIsARegisteredService() throws InterpretationException {
-        stubConsole = new ConsoleStub();
         hliStub = new HumanLanguageInterpreterStub();
         registerService(hliStub);
 
@@ -144,7 +141,6 @@ public class VoiceManagerTest extends JavaOSGiTest {
 
     @Test
     public void interpretSomethingWithGivenHliIdEhenTheHliIsNotARegisteredService() throws InterpretationException {
-        stubConsole = new ConsoleStub();
         hliStub = new HumanLanguageInterpreterStub();
         String result;
         exception.expect(InterpretationException.class);
@@ -156,7 +152,6 @@ public class VoiceManagerTest extends JavaOSGiTest {
     @Test
     public void interpretSomethingWhenTheDefaultHliIsSetAndItIsARegisteredService()
             throws IOException, InterpretationException {
-        stubConsole = new ConsoleStub();
         hliStub = new HumanLanguageInterpreterStub();
         registerService(hliStub);