Merge pull request #751 from addo37/fix-poll-user-v2

Fix processing updates with no user info

telegrambots-abilities/src/main/java/org/telegram/abilitybots/api/bot/BaseAbilityBot.java

@@ -180,6 +180,7 @@ public abstract class BaseAbilityBot extends DefaultAbsSender implements Ability
                 .filter(this::checkBlacklist)
                 .map(this::addUser)
                 .filter(this::filterReply)
+                .filter(this::hasUser)
                 .map(this::getAbility)
                 .filter(this::validateAbility)
                 .filter(this::checkPrivacy)
@@ -487,6 +488,12 @@ public abstract class BaseAbilityBot extends DefaultAbsSender implements Ability
         return update;
     }
 
+    private boolean hasUser(Update update) {
+        // Valid updates without users should return an empty user
+        // Updates that are not recognized by the getUser method will throw an exception
+        return !AbilityUtils.getUser(update).equals(EMPTY_USER);
+    }
+
     private void updateUserId(User oldUser, User newUser) {
         if (oldUser != null && oldUser.getUserName() != null) {
             // Remove old username -> ID

telegrambots-abilities/src/main/java/org/telegram/abilitybots/api/util/AbilityUtils.java

@@ -24,7 +24,7 @@ import static org.telegram.abilitybots.api.objects.Flag.*;
  * Helper and utility methods
  */
 public final class AbilityUtils {
-  public static User EMPTY_USER = new User();
+  public static User EMPTY_USER = new User(0, "", false, "", "", "");
 
   private AbilityUtils() {
 
@@ -150,6 +150,14 @@ public final class AbilityUtils {
       return update.getEditedMessage().getChatId();
     } else if (CHOSEN_INLINE_QUERY.test(update)) {
       return (long) update.getChosenInlineQuery().getFrom().getId();
+    } else if (SHIPPING_QUERY.test(update)) {
+      return (long) update.getShippingQuery().getFrom().getId();
+    } else if (PRECHECKOUT_QUERY.test(update)) {
+      return (long) update.getPreCheckoutQuery().getFrom().getId();
+    } else if (POLL_ANSWER.test(update)) {
+      return (long) update.getPollAnswer().getUser().getId();
+    } else if (POLL.test(update)) {
+      return (long) EMPTY_USER.getId();
     } else {
       throw new IllegalStateException("Could not retrieve originating chat ID from update");
     }
@@ -170,10 +178,8 @@ public final class AbilityUtils {
       return update.getEditedChannelPost().isUserMessage();
     } else if (EDITED_MESSAGE.test(update)) {
       return update.getEditedMessage().isUserMessage();
-    } else if (CHOSEN_INLINE_QUERY.test(update) || INLINE_QUERY.test(update)) {
-      return true;
     } else {
-      throw new IllegalStateException("Could not retrieve update context origin (user/group)");
+      return true;
     }
   }
 

telegrambots-abilities/src/test/java/org/telegram/abilitybots/api/bot/AbilityBotTest.java

@@ -12,6 +12,7 @@ import org.telegram.abilitybots.api.db.DBContext;
 import org.telegram.abilitybots.api.objects.*;
 import org.telegram.abilitybots.api.sender.MessageSender;
 import org.telegram.abilitybots.api.sender.SilentSender;
+import org.telegram.abilitybots.api.util.AbilityUtils;
 import org.telegram.abilitybots.api.util.Pair;
 import org.telegram.abilitybots.api.util.Trio;
 import org.telegram.telegrambots.meta.api.methods.groupadministration.GetChatAdministrators;
@@ -25,6 +26,7 @@ import java.util.Arrays;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Consumer;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Sets.newHashSet;
@@ -38,8 +40,8 @@ import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
 import static org.mockito.internal.verification.VerificationModeFactory.times;
 import static org.telegram.abilitybots.api.bot.DefaultBot.getDefaultBuilder;
-import static org.telegram.abilitybots.api.bot.TestUtils.*;
 import static org.telegram.abilitybots.api.bot.TestUtils.CREATOR;
+import static org.telegram.abilitybots.api.bot.TestUtils.*;
 import static org.telegram.abilitybots.api.db.MapDBContext.offlineInstance;
 import static org.telegram.abilitybots.api.objects.Flag.DOCUMENT;
 import static org.telegram.abilitybots.api.objects.Flag.MESSAGE;
@@ -120,6 +122,49 @@ public class AbilityBotTest {
     verify(silent, times(1)).send("reply", USER.getId());
   }
 
+  @Test
+  void canProcessUpdatesWithoutUserInfo() {
+    Update update = mock(Update.class);
+    // At the moment, only poll updates carry no user information
+    when(update.hasPoll()).thenReturn(true);
+
+    bot.onUpdateReceived(update);
+  }
+
+  @Test
+  void getUserHasAllMethodsDefined() {
+    Arrays.stream(Update.class.getMethods())
+        // filter to all these methods of hasXXX (hasPoll, hasMessage, etc...)
+        .filter(method -> method.getName().startsWith("has"))
+        // Gotta filter out hashCode
+        .filter(method -> method.getReturnType().getName().equals("boolean"))
+        .forEach(method -> {
+          Update update = mock(Update.class);
+          try {
+            // Mock the method and make sure it returns true so that it gets processed by the following method
+            when(method.invoke(update)).thenReturn(true);
+            // Call the getUser function, throws an IllegalStateException if there's an update that can't be processed
+            AbilityUtils.getUser(update);
+          } catch (IllegalStateException e) {
+            throw new RuntimeException(
+                format("Found an update variation that is not handled by the getUser util method [%s]", method.getName()), e);
+          } catch (NullPointerException | ReflectiveOperationException e) {
+            // This is fine, the mock isn't complete and we're only
+            // looking for IllegalStateExceptions thrown by the method
+          }
+        });
+  }
+
+  @Test
+  void getChatIdCanHandleAllKindsOfUpdates() {
+    handlesAllUpdates(AbilityUtils::getUser);
+  }
+
+  @Test
+  void getUserCanHandleAllKindsOfUpdates() {
+    handlesAllUpdates(AbilityUtils::getChatId);
+  }
+
   @Test
   void canBackupDB() throws TelegramApiException {
     MessageContext context = defaultContext();
@@ -574,6 +619,29 @@ public class AbilityBotTest {
     verify(silent, times(1)).send(expected, GROUP_ID);
   }
 
+  private void handlesAllUpdates(Consumer<Update> utilMethod) {
+    Arrays.stream(Update.class.getMethods())
+        // filter to all these methods of hasXXX (hasPoll, hasMessage, etc...)
+        .filter(method -> method.getName().startsWith("has"))
+        // Gotta filter out hashCode
+        .filter(method -> method.getReturnType().getName().equals("boolean"))
+        .forEach(method -> {
+          Update update = mock(Update.class);
+          try {
+            // Mock the method and make sure it returns true so that it gets processed by the following method
+            when(method.invoke(update)).thenReturn(true);
+            // Call the function, throws an IllegalStateException if there's an update that can't be processed
+            utilMethod.accept(update);
+          } catch (IllegalStateException e) {
+            throw new RuntimeException(
+                format("Found an update variation that is not handled by the getChatId util method [%s]", method.getName()), e);
+          } catch (NullPointerException | ReflectiveOperationException e) {
+            // This is fine, the mock isn't complete and we're only
+            // looking for IllegalStateExceptions thrown by the method
+          }
+        });
+  }
+
   private void mockUser(Update update, Message message, User user) {
     when(update.hasMessage()).thenReturn(true);
     when(update.getMessage()).thenReturn(message);

telegrambots-abilities/src/test/java/org/telegram/abilitybots/api/bot/ReplyFlowTest.java

@@ -5,11 +5,13 @@ import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.telegram.abilitybots.api.db.DBContext;
+import org.telegram.abilitybots.api.objects.Flag;
 import org.telegram.abilitybots.api.objects.Reply;
 import org.telegram.abilitybots.api.objects.ReplyFlow;
 import org.telegram.abilitybots.api.sender.MessageSender;
 import org.telegram.abilitybots.api.sender.SilentSender;
 import org.telegram.telegrambots.meta.api.objects.Update;
+import org.telegram.telegrambots.meta.api.objects.polls.Poll;
 
 import java.io.IOException;
 import java.util.function.Predicate;
@@ -106,6 +108,20 @@ public class ReplyFlowTest {
     assertFalse(db.<Long, Integer>getMap(STATES).containsKey(chatId), "User still has state after terminal reply");
   }
 
+  @Test
+  void repliesHandlePollResponse() {
+    Update update = mock(Update.class);
+    when(update.hasPoll()).thenReturn(true);
+    when(update.hasMessage()).thenReturn(false);
+
+    Poll poll = mock(Poll.class);
+    when(poll.getId()).thenReturn("1");
+    when(update.getPoll()).thenReturn(poll);
+
+    // This should not be processed as a reply, so we wouldn't filter out (true)
+    assertTrue(bot.filterReply(update));
+  }
+
   public static class ReplyFlowBot extends AbilityBot {
 
     private ReplyFlowBot(String botToken, String botUsername, DBContext db) {
@@ -139,7 +155,7 @@ public class ReplyFlowTest {
 
     @NotNull
     private Predicate<Update> hasMessageWith(String msg) {
-      return upd -> upd.getMessage().getText().equalsIgnoreCase(msg);
+      return upd -> Flag.MESSAGE.test(upd) && upd.getMessage().getText().equalsIgnoreCase(msg);
     }
   }
 }