feat: add 429 awareness

.github/dependabot.yml

@@ -10,6 +10,20 @@ updates:
     directory: "/"
     registries:
       - maven-github
+    schedule:
+      interval: "weekly"
+    commit-message:
+      prefix: "deps"
+    groups:
+      minor-patch-dependencies:
+        update-types:
+          - "minor"
+          - "patch"
+      major-dependencies:
+        update-types:
+          - "major"
+  - package-ecosystem: "github-actions"
+    directory: "/"
     schedule:
       interval: "weekly"
     commit-message:

pom.xml

@@ -4,7 +4,7 @@
 
   <groupId>com.bigboxer23</groupId>
   <artifactId>switchbotapi-java</artifactId>
-  <version>1.2.5</version>
+  <version>1.2.6</version>
 
   <name>switchbotapi-java</name>
   <url>https://github.com/bigboxer23/switchbotapi-java</url>
@@ -44,7 +44,7 @@
     <dependency>
       <groupId>com.bigboxer23</groupId>
       <artifactId>utils</artifactId>
-      <version>2.4.0</version>
+      <version>2.5.0</version>
     </dependency>
     <dependency>
       <groupId>org.projectlombok</groupId>
@@ -192,13 +192,8 @@
         <version>3.9.0</version>
         <configuration>
           <java>
-            <googleJavaFormat>
-              <version>1.17.0</version>
-              <style>AOSP</style>
-              <reflowLongStrings>true</reflowLongStrings>
-            </googleJavaFormat>
             <palantirJavaFormat>
-              <version>2.35.0</version>
+              <version>2.96.0</version>
             </palantirJavaFormat>
             <indent>
               <tabs>true</tabs>

src/main/java/com/bigboxer23/switch_bot/SwitchBotApi.java

@@ -36,7 +36,7 @@ private SwitchBotApi(String token, String secret) {
 		deviceApi = new SwitchBotDeviceApi(this);
 	}
 
-	public static SwitchBotApi getInstance(String token, String secret) {
+	public static synchronized SwitchBotApi getInstance(String token, String secret) {
 		if (token == null || secret == null) {
 			log.error("need to define token and secret values.");
 			throw new RuntimeException("need to define token and secret values.");

src/main/java/com/bigboxer23/switch_bot/SwitchBotDeviceApi.java

@@ -1,6 +1,7 @@
 package com.bigboxer23.switch_bot;
 
 import com.bigboxer23.switch_bot.data.*;
+import com.bigboxer23.utils.command.NonRetryableException;
 import com.bigboxer23.utils.http.OkHttpUtil;
 import com.bigboxer23.utils.time.ITimeConstants;
 import java.io.IOException;
@@ -20,10 +21,16 @@
 public class SwitchBotDeviceApi {
 	private final SwitchBotApi provider;
 
+	protected static final int HTTP_TOO_MANY_REQUESTS = 429;
+
+	protected static final long DEVICE_NAME_REFRESH_BACKOFF = ITimeConstants.MINUTE * 5;
+
 	private Map<String, String> deviceIdToNames;
 
 	protected long deviceIdToNamesCacheTime = -1;
 
+	protected long deviceIdToNamesRetryTime = -1;
+
 	protected SwitchBotDeviceApi(SwitchBotApi provider) {
 		this.provider = provider;
 	}
@@ -39,15 +46,19 @@ private synchronized void refreshDeviceNameMap() {
 		if (deviceIdToNames != null && (System.currentTimeMillis() - ITimeConstants.HOUR) < deviceIdToNamesCacheTime) {
 			return;
 		}
+		if (System.currentTimeMillis() < deviceIdToNamesRetryTime) {
+			return;
+		}
 		try {
 			log.info("Refreshing device id/name map...");
 			deviceIdToNames = Collections.unmodifiableMap(
 					getDevices().stream().collect(Collectors.toMap(Device::getDeviceId, Device::getDeviceName)));
 			deviceIdToNamesCacheTime = System.currentTimeMillis();
+			deviceIdToNamesRetryTime = -1;
 		} catch (IOException e) {
-			log.error("Failed to refresh device names.", e);
-			deviceIdToNames = null;
-			deviceIdToNamesCacheTime = -1;
+			log.error(
+					"Failed to refresh device names, retrying no sooner than " + DEVICE_NAME_REFRESH_BACKOFF + "ms", e);
+			deviceIdToNamesRetryTime = System.currentTimeMillis() + DEVICE_NAME_REFRESH_BACKOFF;
 		}
 	}
 
@@ -105,6 +116,10 @@ public IApiResponse sendDeviceControlCommands(String deviceId, DeviceCommand com
 	 * @throws IOException
 	 */
 	private <T extends IApiResponse> T parseResponse(Response response, Class<T> clazz) throws IOException {
+		if (response != null && response.code() == HTTP_TOO_MANY_REQUESTS) {
+			log.error("rate limited by the switchbot api: " + response.code() + " " + response.message());
+			throw new NonRetryableException("rate limited " + response.code() + " " + response.message());
+		}
 		IApiResponse apiResponse =
 				provider.checkForError(response, (Optional<IApiResponse>) OkHttpUtil.getBody(response, clazz));
 		if (!apiResponse.isSuccess()) {

src/test/java/com/bigboxer23/switch_bot/SwitchBotDeviceApiTest.java

@@ -264,6 +264,57 @@ public void testGetDeviceNameFromIdWithCacheRefreshFailure() throws IOException
 
 		String result = spyDeviceApi.getDeviceNameFromId("test-device");
 		assertEquals("test-device", result);
-		assertEquals(-1, spyDeviceApi.deviceIdToNamesCacheTime);
+		assertTrue(spyDeviceApi.deviceIdToNamesRetryTime > System.currentTimeMillis());
+	}
+
+	/**
+	 * A failed refresh used to drop the cache, so every later lookup asked for the device list
+	 * again. That turns a rate limited api into a request loop, exactly when we can least afford it.
+	 */
+	@Test
+	public void testCacheRefreshFailureDoesNotRequestDeviceListOnEveryLookup() throws IOException {
+		SwitchBotDeviceApi spyDeviceApi = spy(deviceApi);
+		doThrow(new IOException("429 rate limited")).when(spyDeviceApi).getDevices();
+
+		for (int i = 0; i < 10; i++) {
+			assertEquals("test-device", spyDeviceApi.getDeviceNameFromId("test-device"));
+		}
+
+		verify(spyDeviceApi, times(1)).getDevices();
+	}
+
+	@Test
+	public void testCacheRefreshFailureRetainsPreviouslyCachedNames() throws IOException {
+		Device device = new Device();
+		device.setDeviceId("device1");
+		device.setDeviceName("Living Room Light");
+
+		SwitchBotDeviceApi spyDeviceApi = spy(deviceApi);
+		doReturn(List.of(device)).when(spyDeviceApi).getDevices();
+		assertEquals("Living Room Light", spyDeviceApi.getDeviceNameFromId("device1"));
+
+		// expire the cache, then fail the refresh
+		spyDeviceApi.deviceIdToNamesCacheTime = System.currentTimeMillis() - (ITimeConstants.HOUR * 2);
+		doThrow(new IOException("429 rate limited")).when(spyDeviceApi).getDevices();
+
+		assertEquals("Living Room Light", spyDeviceApi.getDeviceNameFromId("device1"));
+	}
+
+	@Test
+	public void testCacheRefreshResumesAfterBackoffWindow() throws IOException {
+		Device device = new Device();
+		device.setDeviceId("device1");
+		device.setDeviceName("Living Room Light");
+
+		SwitchBotDeviceApi spyDeviceApi = spy(deviceApi);
+		doThrow(new IOException("429 rate limited")).when(spyDeviceApi).getDevices();
+		assertEquals("device1", spyDeviceApi.getDeviceNameFromId("device1"));
+
+		// pretend the backoff window has passed
+		spyDeviceApi.deviceIdToNamesRetryTime = System.currentTimeMillis() - 1;
+		doReturn(List.of(device)).when(spyDeviceApi).getDevices();
+
+		assertEquals("Living Room Light", spyDeviceApi.getDeviceNameFromId("device1"));
+		assertEquals(-1, spyDeviceApi.deviceIdToNamesRetryTime);
 	}
 }