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

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

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



##########
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:
       @cschneider Yes, its a wasted effort to put the binry in memory but the important thing is that the length is being calculated from that and being used to build the PackageMessageBuilder. I couldn't find out for sure whether the DistributionPackage.getSize() always returns the length/size. 




----------------------------------------------------------------
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