[GitHub] [sling-org-apache-sling-distribution-journal] cschneider commented on a change in pull request #54: SLING-9593: Abstract out binary handling to allow plugging of differe…

[GitBox <gi...@apache.org>]

cschneider commented on a change in pull request #54:
URL: https://github.com/apache/sling-org-apache-sling-distribution-journal/pull/54#discussion_r460015685



##########
File path: src/main/java/org/apache/sling/distribution/journal/impl/publisher/PackageMessageFactory.java
##########
@@ -116,27 +112,18 @@ private PackageMessage createAdd(DistributionPackageBuilder packageBuilder, Reso
                 .pkgLength(pkgLength)
                 .userId(resourceResolver.getUserID())
                 .pkgType(packageBuilder.getType());
-        if (pkgLength > MAX_INLINE_PKG_BINARY_SIZE) {
-
-            /*
-             * Rather than pro-actively (and somewhat arbitrarily)
-             * decide to avoid sending a package inline based on
-             * its size, we could simply try to send packages of
-             * any size and only avoiding to inline as a fallback.
-             * However, this approach requires the messaging
-             * implementation to offer a mean to distinguish
-             * size issues when sending messages, which is not
-             * always the case.
-             */
-            InputStream binaryStream;
-            try {
-                binaryStream = disPkg.createInputStream();
-            } catch (IOException e) {
-                throw new DistributionException("Error creating stream for package " + disPkg.getId(), e);
-            }
-            LOG.info("Package {} too large ({}B) to be sent inline", disPkg.getId(), pkgLength);
-            String pkgBinRef = packageRepo.store(disPkg.getId(), binaryStream);
-            pkgBuilder.pkgBinaryRef(pkgBinRef);
+
+        String storeRef;
+        try {
+            String id = UUID.randomUUID().toString();
+            LOG.info("Creating package binary with id [{}] for package [{}], length [{}]", id, disPkg.getId(), pkgLength);
+            storeRef =  binaryStore.put(id, disPkg.createInputStream(), pkgLength);
+        } catch (IOException e) {
+            throw new DistributionException(e.getMessage(), e);
+        }
+
+        if (StringUtils.isNotEmpty(storeRef)) {
+            pkgBuilder.pkgBinaryRef(storeRef);
         } else {
             pkgBuilder.pkgBinary(pkgBinary);

Review comment:
       On top of the method we always create the pkgBinary. I propose we only create it when the binaryStore does not return a storeRef.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org