Fix mirroring not working with messages that only contain an attachment, fix mirrored attachments having no content

api/src/main/java/com/discordsrv/api/discord/entity/channel/DiscordMessageChannel.java

@@ -32,7 +32,6 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import java.io.InputStream;
-import java.util.Collections;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 
@@ -54,21 +53,7 @@ public interface DiscordMessageChannel extends Snowflake {
      * @return a future returning the message after being sent
      */
     @NotNull
-    default CompletableFuture<ReceivedDiscordMessage> sendMessage(@NotNull SendableDiscordMessage message) {
+    CompletableFuture<ReceivedDiscordMessage> sendMessage(@NotNull SendableDiscordMessage message);
-        return sendMessage(message, Collections.emptyMap());
-    }
-
-    /**
-     * Sends the provided message to the channel with the provided attachments.
-     *
-     * @param message the message to send to the channel
-     * @param attachments the attachments (in a map of file name and input stream pairs) to include in the message, the streams will be closed upon execution
-     * @return a future returning the message after being sent
-     */
-    CompletableFuture<ReceivedDiscordMessage> sendMessage(
-            @NotNull SendableDiscordMessage message,
-            @NotNull Map<String, InputStream> attachments
-    );
 
     /**
      * Deletes the message identified by the id.
@@ -87,7 +72,7 @@ public interface DiscordMessageChannel extends Snowflake {
      * @return a future returning the message after being edited
      */
     @NotNull
-    CompletableFuture<ReceivedDiscordMessage> editMessageById(long id, @NotNull SendableDiscordMessage message, @Nullable Map<String, InputStream> attachments);
+    CompletableFuture<ReceivedDiscordMessage> editMessageById(long id, @NotNull SendableDiscordMessage message);
 
     /**
      * Returns the JDA representation of this object. This should not be used if it can be avoided.

api/src/main/java/com/discordsrv/api/discord/entity/message/ReceivedDiscordMessage.java

@@ -35,9 +35,7 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.Unmodifiable;
 
-import java.io.InputStream;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 
 /**
@@ -151,10 +149,6 @@ public interface ReceivedDiscordMessage extends Snowflake {
     @NotNull
     CompletableFuture<Void> delete();
 
-    default CompletableFuture<ReceivedDiscordMessage> edit(@NotNull SendableDiscordMessage message) {
-        return edit(message, null);
-    }
-
     /**
      * Edits this message to the provided message.
      *
@@ -163,8 +157,7 @@ public interface ReceivedDiscordMessage extends Snowflake {
      * @throws IllegalArgumentException if the message is not a webhook message,
      * but the provided {@link SendableDiscordMessage} specifies a webhook username.
      */
-    @NotNull
+    CompletableFuture<ReceivedDiscordMessage> edit(@NotNull SendableDiscordMessage message);
-    CompletableFuture<ReceivedDiscordMessage> edit(@NotNull SendableDiscordMessage message, @Nullable Map<String, InputStream> attachments);
 
     class Attachment {
 

api/src/main/java/com/discordsrv/api/discord/entity/message/SendableDiscordMessage.java

@@ -31,8 +31,10 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.Unmodifiable;
 
+import java.io.InputStream;
 import java.util.Collection;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -107,6 +109,8 @@ public interface SendableDiscordMessage {
         return getWebhookUsername() != null;
     }
 
+    Map<InputStream, String> getAttachments();
+
     @SuppressWarnings("UnusedReturnValue") // API
     interface Builder {
 
@@ -238,6 +242,20 @@ public interface SendableDiscordMessage {
         @NotNull
         Builder setWebhookAvatarUrl(String webhookAvatarUrl);
 
+        /**
+         * Adds an attachment to this builder.
+         * @param inputStream an input stream containing the file contents
+         * @param fileName the name of the file
+         * @return the builder, useful for chaining
+         */
+        Builder addAttachment(InputStream inputStream, String fileName);
+
+        /**
+         * Checks if this builder has any sendable content.
+         * @return {@code true} if there is no sendable content
+         */
+        boolean isEmpty();
+
         /**
          * Builds a {@link SendableDiscordMessage} from this builder.
          * @return the new {@link SendableDiscordMessage}

api/src/main/java/com/discordsrv/api/discord/entity/message/impl/SendableDiscordMessageImpl.java

@@ -37,6 +37,7 @@ import net.dv8tion.jda.api.entities.MessageEmbed;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.io.InputStream;
 import java.util.*;
 import java.util.function.Function;
 import java.util.regex.Matcher;
@@ -50,6 +51,7 @@ public class SendableDiscordMessageImpl implements SendableDiscordMessage {
     private final Set<AllowedMention> allowedMentions;
     private final String webhookUsername;
     private final String webhookAvatarUrl;
+    private final Map<InputStream, String> attachments;
 
     protected SendableDiscordMessageImpl(
             String content,
@@ -57,7 +59,8 @@ public class SendableDiscordMessageImpl implements SendableDiscordMessage {
             List<MessageActionRow> actionRows,
             Set<AllowedMention> allowedMentions,
             String webhookUsername,
-            String webhookAvatarUrl
+            String webhookAvatarUrl,
+            Map<InputStream, String> attachments
     ) {
         this.content = content;
         this.embeds = Collections.unmodifiableList(embeds);
@@ -65,6 +68,7 @@ public class SendableDiscordMessageImpl implements SendableDiscordMessage {
         this.allowedMentions = Collections.unmodifiableSet(allowedMentions);
         this.webhookUsername = webhookUsername;
         this.webhookAvatarUrl = webhookAvatarUrl;
+        this.attachments = Collections.unmodifiableMap(attachments);
     }
 
     @Override
@@ -98,6 +102,11 @@ public class SendableDiscordMessageImpl implements SendableDiscordMessage {
         return webhookAvatarUrl;
     }
 
+    @Override
+    public Map<InputStream, String> getAttachments() {
+        return attachments;
+    }
+
     public static class BuilderImpl implements SendableDiscordMessage.Builder {
 
         private String content;
@@ -106,6 +115,7 @@ public class SendableDiscordMessageImpl implements SendableDiscordMessage {
         private final Set<AllowedMention> allowedMentions = new LinkedHashSet<>();
         private String webhookUsername;
         private String webhookAvatarUrl;
+        private final Map<InputStream, String> attachments = new LinkedHashMap<>();
 
         @Override
         public String getContent() {
@@ -205,9 +215,20 @@ public class SendableDiscordMessageImpl implements SendableDiscordMessage {
             return this;
         }
 
+        @Override
+        public Builder addAttachment(InputStream inputStream, String fileName) {
+            this.attachments.put(inputStream, fileName);
+            return this;
+        }
+
+        @Override
+        public boolean isEmpty() {
+            return (content == null || content.isEmpty()) && embeds.isEmpty() && attachments.isEmpty() && actionRows.isEmpty();
+        }
+
         @Override
         public @NotNull SendableDiscordMessage build() {
-            return new SendableDiscordMessageImpl(content, embeds, actionRows, allowedMentions, webhookUsername, webhookAvatarUrl);
+            return new SendableDiscordMessageImpl(content, embeds, actionRows, allowedMentions, webhookUsername, webhookAvatarUrl, attachments);
         }
 
         @Override

common/src/main/java/com/discordsrv/common/discord/api/entity/channel/AbstractDiscordGuildMessageChannel.java

@@ -29,18 +29,12 @@ import net.dv8tion.jda.api.entities.Message;
 import net.dv8tion.jda.api.entities.WebhookClient;
 import net.dv8tion.jda.api.entities.channel.middleman.GuildMessageChannel;
 import net.dv8tion.jda.api.requests.RestAction;
-import net.dv8tion.jda.api.utils.FileUpload;
 import net.dv8tion.jda.api.utils.messages.MessageCreateData;
 import net.dv8tion.jda.api.utils.messages.MessageCreateRequest;
 import net.dv8tion.jda.api.utils.messages.MessageEditData;
 import net.dv8tion.jda.api.utils.messages.MessageEditRequest;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 
-import java.io.InputStream;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 
 public abstract class AbstractDiscordGuildMessageChannel<T extends GuildMessageChannel>
@@ -79,14 +73,12 @@ public abstract class AbstractDiscordGuildMessageChannel<T extends GuildMessageC
     }
 
     @Override
-    public CompletableFuture<ReceivedDiscordMessage> sendMessage(
+    public @NotNull CompletableFuture<ReceivedDiscordMessage> sendMessage(@NotNull SendableDiscordMessage message) {
-            @NotNull SendableDiscordMessage message, @NotNull Map<String, InputStream> attachments
+        return sendInternal(message);
-    ) {
-        return sendInternal(message, attachments);
     }
 
     @SuppressWarnings("unchecked") // Generics
-    private <R extends MessageCreateRequest<? extends MessageCreateRequest<?>> & RestAction<Message>> CompletableFuture<ReceivedDiscordMessage> sendInternal(SendableDiscordMessage message, Map<String, InputStream> attachments) {
+    private <R extends MessageCreateRequest<? extends MessageCreateRequest<?>> & RestAction<Message>> CompletableFuture<ReceivedDiscordMessage> sendInternal(SendableDiscordMessage message) {
         MessageCreateData createData = SendableDiscordMessageUtil.toJDASend(message);
 
         CompletableFuture<R> createRequest;
@@ -101,12 +93,6 @@ public abstract class AbstractDiscordGuildMessageChannel<T extends GuildMessageC
         }
 
         return createRequest
-                .thenApply(action -> {
-                    for (Map.Entry<String, InputStream> entry : attachments.entrySet()) {
-                        action = (R) action.addFiles(FileUpload.fromData(entry.getValue(), entry.getKey()));
-                    }
-                    return action;
-                })
                 .thenCompose(RestAction::submit)
                 .thenApply(msg -> ReceivedDiscordMessageImpl.fromJDA(discordSRV, msg));
     }
@@ -114,17 +100,15 @@ public abstract class AbstractDiscordGuildMessageChannel<T extends GuildMessageC
     @Override
     public @NotNull CompletableFuture<ReceivedDiscordMessage> editMessageById(
             long id,
-            @NotNull SendableDiscordMessage message,
+            @NotNull SendableDiscordMessage message
-            @Nullable Map<String, InputStream> attachments
     ) {
-        return editInternal(id, message, attachments);
+        return editInternal(id, message);
     }
 
     @SuppressWarnings("unchecked") // Generics
     private <R extends MessageEditRequest<? extends MessageEditRequest<?>> & RestAction<Message>> CompletableFuture<ReceivedDiscordMessage> editInternal(
             long id,
-            SendableDiscordMessage message,
+            SendableDiscordMessage message
-            Map<String, InputStream> attachments
     ) {
         MessageEditData editData = SendableDiscordMessageUtil.toJDAEdit(message);
 
@@ -136,19 +120,6 @@ public abstract class AbstractDiscordGuildMessageChannel<T extends GuildMessageC
         }
 
         return editRequest
-                .thenApply(action -> {
-                    if (attachments != null) {
-                        List<FileUpload> uploads = new ArrayList<>();
-                        for (Map.Entry<String, InputStream> entry : attachments.entrySet()) {
-                            uploads.add(FileUpload.fromData(entry.getValue(), entry.getKey()));
-                        }
-                        action = (R) action.setFiles(uploads);
-                    } else {
-                        action = (R) action.setAttachments(); // TODO
-                    }
-
-                    return action;
-                })
                 .thenCompose(RestAction::submit)
                 .thenApply(msg -> ReceivedDiscordMessageImpl.fromJDA(discordSRV, msg));
     }

common/src/main/java/com/discordsrv/common/discord/api/entity/channel/DiscordDMChannelImpl.java

@@ -28,12 +28,8 @@ import com.discordsrv.common.discord.api.entity.message.util.SendableDiscordMess
 import net.dv8tion.jda.api.entities.User;
 import net.dv8tion.jda.api.entities.channel.concrete.PrivateChannel;
 import net.dv8tion.jda.api.requests.restaction.MessageCreateAction;
-import net.dv8tion.jda.api.utils.FileUpload;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 
-import java.io.InputStream;
-import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 
@@ -53,18 +49,12 @@ public class DiscordDMChannelImpl extends AbstractDiscordMessageChannel<PrivateC
     }
 
     @Override
-    public CompletableFuture<ReceivedDiscordMessage> sendMessage(
+    public CompletableFuture<ReceivedDiscordMessage> sendMessage(@NotNull SendableDiscordMessage message) {
-            @NotNull SendableDiscordMessage message,
-            @NotNull Map<String, InputStream> attachments
-    ) {
         if (message.isWebhookMessage()) {
             throw new IllegalArgumentException("Cannot send webhook messages to DMChannels");
         }
 
         MessageCreateAction action = channel.sendMessage(SendableDiscordMessageUtil.toJDASend(message));
-        for (Map.Entry<String, InputStream> entry : attachments.entrySet()) {
-            action = action.addFiles(FileUpload.fromData(entry.getValue(), entry.getKey()));
-        }
 
         CompletableFuture<ReceivedDiscordMessage> future = action.submit()
                 .thenApply(msg -> ReceivedDiscordMessageImpl.fromJDA(discordSRV, msg));
@@ -83,8 +73,7 @@ public class DiscordDMChannelImpl extends AbstractDiscordMessageChannel<PrivateC
     @Override
     public @NotNull CompletableFuture<ReceivedDiscordMessage> editMessageById(
             long id,
-            @NotNull SendableDiscordMessage message,
+            @NotNull SendableDiscordMessage message
-            @Nullable Map<String, InputStream> attachments
     ) {
         if (message.isWebhookMessage()) {
             throw new IllegalArgumentException("Cannot send webhook messages to DMChannels");

common/src/main/java/com/discordsrv/common/discord/api/entity/message/ReceivedDiscordMessageClusterImpl.java

@@ -54,7 +54,7 @@ public class ReceivedDiscordMessageClusterImpl implements ReceivedDiscordMessage
     public @NotNull CompletableFuture<ReceivedDiscordMessageCluster> editAll(SendableDiscordMessage newMessage) {
         List<CompletableFuture<ReceivedDiscordMessage>> futures = new ArrayList<>(messages.size());
         for (ReceivedDiscordMessage message : messages) {
-            futures.add(message.edit(newMessage, null));
+            futures.add(message.edit(newMessage));
         }
 
         return CompletableFutureUtil.combine(futures).thenApply(ReceivedDiscordMessageClusterImpl::new);

common/src/main/java/com/discordsrv/common/discord/api/entity/message/ReceivedDiscordMessageImpl.java

@@ -45,10 +45,8 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.Unmodifiable;
 
-import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 
 public class ReceivedDiscordMessageImpl implements ReceivedDiscordMessage {
@@ -238,8 +236,7 @@ public class ReceivedDiscordMessageImpl implements ReceivedDiscordMessage {
 
     @Override
     public @NotNull CompletableFuture<ReceivedDiscordMessage> edit(
-            @NotNull SendableDiscordMessage message,
+            @NotNull SendableDiscordMessage message
-            @Nullable Map<String, InputStream> attachments
     ) {
         if (!webhookMessage && message.isWebhookMessage()) {
             throw new IllegalArgumentException("Cannot edit a non-webhook message into a webhook message");
@@ -250,7 +247,7 @@ public class ReceivedDiscordMessageImpl implements ReceivedDiscordMessage {
             return CompletableFutureUtil.failed(new RestErrorResponseException(ErrorResponse.UNKNOWN_CHANNEL));
         }
 
-        return textChannel.editMessageById(getId(), message, attachments);
+        return textChannel.editMessageById(getId(), message);
     }
 
     //

common/src/main/java/com/discordsrv/common/discord/api/entity/message/util/SendableDiscordMessageUtil.java

@@ -25,11 +25,14 @@ import com.discordsrv.api.discord.entity.message.SendableDiscordMessage;
 import net.dv8tion.jda.api.entities.Message;
 import net.dv8tion.jda.api.entities.MessageEmbed;
 import net.dv8tion.jda.api.interactions.components.ActionRow;
+import net.dv8tion.jda.api.utils.FileUpload;
 import net.dv8tion.jda.api.utils.messages.*;
 import org.jetbrains.annotations.NotNull;
 
+import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 public final class SendableDiscordMessageUtil {
@@ -61,12 +64,18 @@ public final class SendableDiscordMessageUtil {
             embeds.add(embed.toJDA());
         }
 
+        List<FileUpload> uploads = new ArrayList<>();
+        for (Map.Entry<InputStream, String> attachment : message.getAttachments().entrySet()) {
+            uploads.add(FileUpload.fromData(attachment.getKey(), attachment.getValue()));
+        }
+
         return (T) builder
                 .setContent(message.getContent())
                 .setEmbeds(embeds)
                 .setAllowedMentions(allowedTypes)
                 .mentionUsers(allowedUsers.stream().mapToLong(l -> l).toArray())
-                .mentionRoles(allowedRoles.stream().mapToLong(l -> l).toArray());
+                .mentionRoles(allowedRoles.stream().mapToLong(l -> l).toArray())
+                .setFiles(uploads);
     }
 
     public static MessageCreateData toJDASend(@NotNull SendableDiscordMessage message) {

common/src/main/java/com/discordsrv/common/messageforwarding/discord/DiscordMessageMirroringModule.java

@@ -121,7 +121,7 @@ public class DiscordMessageMirroringModule extends AbstractModule<DiscordSRV> {
             }
 
             MirroringConfig.AttachmentConfig attachmentConfig = config.attachments;
-            int maxSize = attachmentConfig.maximumSizeKb;
+            int maxSize = attachmentConfig.maximumSizeKb * 1000;
             boolean embedAttachments = attachmentConfig.embedAttachments;
             if (maxSize >= 0 || embedAttachments) {
                 for (ReceivedDiscordMessage.Attachment attachment : message.getAttachments()) {
@@ -129,10 +129,11 @@ public class DiscordMessageMirroringModule extends AbstractModule<DiscordSRV> {
                         continue;
                     }
 
-                    if (maxSize == 0 || attachment.sizeBytes() <= (maxSize * 1000)) {
+                    if (maxSize == 0 || attachment.sizeBytes() <= maxSize) {
                         Request request = new Request.Builder()
-                                .url(attachment.proxyUrl())
+                                .url(attachment.url())
                                 .get()
+                                .addHeader("Accept", "*/*")
                                 .build();
 
                         byte[] bytes = null;
@@ -187,26 +188,40 @@ public class DiscordMessageMirroringModule extends AbstractModule<DiscordSRV> {
                         messageBuilder.addEmbed(attachmentEmbed.build());
                     }
 
-                    int maxSize = attachmentConfig.maximumSizeKb;
+                    int maxSize = attachmentConfig.maximumSizeKb * 1000;
-                    Map<String, InputStream> currentAttachments;
+                    List<InputStream> streams = new ArrayList<>();
-                    if (!attachments.isEmpty() && maxSize > 0) {
+                    if (!attachments.isEmpty() && maxSize >= 0) {
-                        currentAttachments = new LinkedHashMap<>();
                         attachments.forEach((attachment, bytes) -> {
-                            if (bytes != null && attachment.sizeBytes() <= maxSize) {
+                            if (bytes != null && (maxSize == 0 || attachment.sizeBytes() <= maxSize)) {
-                                currentAttachments.put(attachment.fileName(), new ByteArrayInputStream(bytes));
+                                InputStream stream = new ByteArrayInputStream(bytes);
+                                streams.add(stream);
+                                messageBuilder.addAttachment(stream, attachment.fileName());
                             }
                         });
-                    } else {
+                    }
-                        currentAttachments = Collections.emptyMap();
+
+                    if (messageBuilder.isEmpty()) {
+                        logger().debug("Nothing to mirror to " + mirrorChannel + ", skipping");
+                        for (InputStream stream : streams) {
+                            try {
+                                stream.close();
+                            } catch (IOException ignored) {}
+                        }
+                        return;
                     }
 
                     CompletableFuture<MirroredMessage> future =
-                            mirrorChannel.sendMessage(messageBuilder.build(), currentAttachments)
+                            mirrorChannel.sendMessage(messageBuilder.build())
                                     .thenApply(msg -> new MirroredMessage(msg, config));
 
                     mirrorFutures.add(future);
                     future.exceptionally(t2 -> {
                         logger().error("Failed to mirror message to " + mirrorChannel, t2);
+                        for (InputStream stream : streams) {
+                            try {
+                                stream.close();
+                            } catch (IOException ignored) {}
+                        }
                         return null;
                     });
                 }
@@ -249,7 +264,7 @@ public class DiscordMessageMirroringModule extends AbstractModule<DiscordSRV> {
             }
 
             SendableDiscordMessage sendableMessage = convert(message, channel, reference.config).build();
-            channel.editMessageById(reference.messageId, sendableMessage, null).exceptionally(t -> {
+            channel.editMessageById(reference.messageId, sendableMessage).exceptionally(t -> {
                 logger().error("Failed to update mirrored message in " + channel);
                 return null;
             });