Merge branch '1.4' into pulls/1/thumbnail-helper

.travis.yml

@@ -4,7 +4,7 @@ sudo: false
 
 env:
   global:
-    - COMPOSER_ROOT_VERSION=1.x-dev
+    - COMPOSER_ROOT_VERSION=1.5.x-dev
     - CORE_RELEASE=master
 
 matrix:

.upgrade.yml

@@ -141,14 +141,6 @@ warnings:
     'SilverStripe\Assets\Image::AssetLibraryThumbnail()':
       message: 'Renamed to CMSThumbnail()'
       replacement: 'CMSThumbnail'
-    'SilverStripe\Assets\Flysystem\FlysystemAssetStore->parseFileID()':
-      message: 'Replace with getDefaultFileIDHelper()->parseFileID()'
-    'SilverStripe\Assets\Flysystem\FlysystemAssetStore->getFileID()':
-      message: 'Replace with getDefaultFileIDHelper()->buildFileID()'
-    'SilverStripe\Assets\Flysystem\FlysystemAssetStore->getOriginalFilename()':
-      message: 'Replace with getDefaultFileIDHelper()->parseFileID()->getFilename()'
-    'SilverStripe\Assets\Flysystem\FlysystemAssetStore->getVariant()':
-      message: 'Replace with getDefaultFileIDHelper()->parseFileID()->getVariant()'
 renameWarnings:
   - File
   - Image

composer.json

@@ -33,7 +33,7 @@
     },
     "extra": {
         "branch-alias": {
-            "1.x-dev": "1.4.x-dev"
+            "1.x-dev": "1.5.x-dev"
         },
         "installer-name": "silverstripe-assets"
     },

src/AssetControlExtension.php

@@ -283,7 +283,7 @@ protected function swapAll($assets)
         $store = $this->getAssetStore();
 
         // The `swap` method was introduced in the 1.4 release. It wasn't added to the interface to avoid breaking
-        // custom implementation. If it's not available on our store, we fallback to a publish/protect
+        // custom implementations. If it's not available on our store, we fall back to a publish/protect
         if (method_exists($store, 'swapPublish')) {
             foreach ($assets as $asset) {
                 $store->swapPublish($asset['Filename'], $asset['Hash']);

src/Dev/TestAssetStore.php

@@ -197,7 +197,7 @@ public function getDefaultConflictResolution($variant)
     protected function isSeekableStream($stream)
     {
         if (isset(self::$seekable_override)) {
-            // Unset the override so we don't get stuck in an efficit loop
+            // Unset the override so we don't get stuck in an infinite loop
             self::$seekable_override = null;
             return self::$seekable_override;
         }

src/FilenameParsing/FileIDHelper.php

@@ -3,7 +3,9 @@
 namespace SilverStripe\Assets\FilenameParsing;
 
 /**
- * Helps build and parse FileIDs according to a predefined format.
+ * Helps build and parse Filename Identifiers (ake: FileIDs) according to a predefined format.
+ *
+ * @internal This is still an evolving API. It may change in the next minor release.
  */
 interface FileIDHelper
 {
@@ -20,7 +22,7 @@ public function buildFileID($filename, $hash = null, $variant = null);
 
 
     /**
-     * Performs filename cleanup before sending it back.
+     * Clean up filename to remove constructs that might clash with the underlying path format of this FileIDHelper.
      *
      * @param string $filename
      * @return string

src/FilenameParsing/FileIDHelperResolutionStrategy.php

@@ -17,6 +17,8 @@
  * older file format to resolve.
  *
  * You may also provide a `VersionedStage` to only look at files that were published.
+ *
+ * @internal This is still an evolving API. It may change in the next minor release.
  */
 class FileIDHelperResolutionStrategy implements FileResolutionStrategy
 {
@@ -42,12 +44,11 @@ public function resolveFileID($fileID, Filesystem $filesystem)
     {
         $parsedFileID = $this->parseFileID($fileID);
 
-        if ($redirect = $this->searchForTuple($parsedFileID, $filesystem, false)) {
-            return $redirect;
+        if ($parsedFileID) {
+            return $this->searchForTuple($parsedFileID, $filesystem, false);
         }
 
-        // If our helper managed to parse the file id, but could not resolve to an actual physical file,
-        // there's nothing else we can do.
+        // If we couldn't resolve the file ID, we bail
         return null;
     }
 
@@ -67,12 +68,11 @@ public function softResolveFileID($fileID, Filesystem $filesystem)
         $hash = $parsedFileID->getHash();
         $tuple = $hash ? $this->resolveWithHash($parsedFileID) : $this->resolveHashless($parsedFileID);
 
-        if ($tuple && $redirect = $this->searchForTuple($tuple, $filesystem, false)) {
-            return $redirect;
+        if ($tuple) {
+            return $this->searchForTuple($tuple, $filesystem, false);
         }
 
-        // If our helper managed to parse the file id, but could not resolve to an actual physical file,
-        // there's nothing else we can do.
+        // If we couldn't resolve the file ID, we bail
         return null;
     }
 
@@ -146,6 +146,8 @@ private function resolveHashless(ParsedFileID $parsedFileID)
                 'Variant' => $variant
             ];
         }
+
+        return null;
     }
 
     public function generateVariantFileID($tuple, Filesystem $fs)
@@ -383,7 +385,6 @@ public function findVariants($tuple, Filesystem $filesystem)
         }
 
         if ($helper) {
-
             $folder = $helper->lookForVariantIn($parsedFileID);
             $possibleVariants = $filesystem->listContents($folder, true);
             foreach ($possibleVariants as $possibleVariant) {
@@ -415,17 +416,22 @@ public function stripVariant($fileID)
     {
         $hash = '';
 
+        // File ID can be a string or a ParsedFileID
         // Normalise our parameters
         if ($fileID instanceof ParsedFileID) {
+            // Let's get data out of our parsed file ID
             $parsedFileID = $fileID;
             $fileID = $parsedFileID->getFileID();
             $hash = $parsedFileID->getHash();
 
+            // Our Parsed File ID has a blank FileID attached to it. This means we are dealing with a file that hasn't
+            // been create yet. Let's used our default file ID helper
             if (empty($fileID)) {
                 return $this->stripVariantFromParsedFileID($parsedFileID, $this->getDefaultFileIDHelper());
             }
         }
 
+        // We don't know what helper was use to build this file ID
         // Let's try to find a helper who can understand our file ID
         foreach ($this->resolutionFileIDHelpers as $fileIDHelper) {
             $parsedFileID = $fileIDHelper->parseFileID($fileID);

src/FilenameParsing/FileResolutionStrategy.php

@@ -5,6 +5,8 @@
 
 /**
  * Represents a strategy for resolving files on a Flysystem Adapter.
+ *
+ * @internal This is still an evolving API. It may change in the next minor release.
  */
 interface FileResolutionStrategy
 {
@@ -17,7 +19,7 @@ interface FileResolutionStrategy
     public function resolveFileID($fileID, Filesystem $filesystem);
 
     /**
-     * Try to resolve a file ID against the provided Filesystem looking at newer version of the file.
+     * Try to resolve a file ID against the provided Filesystem looking at newer versions of the file.
      * @param string $fileID
      * @param Filesystem $filesystem
      * @return ParsedFileID|null Alternative FileID where the user should be redirected to.
@@ -27,6 +29,8 @@ public function softResolveFileID($fileID, Filesystem $filesystem);
     /**
      * Build a file ID for a variant so it follows the pattern of it's original file. The variant may not exists on the
      * Filesystem yet, but the original file has to. This is to make sure that variant file always follow the same
+     * Build a file ID for a variant so it follows the pattern of its original file. The variant may not exist on the
+     * Filesystem yet, but the original file has to. This is to make sure that variant files always follow the same
      * pattern as the original file they are attached to.
      * @param ParsedFileID|array $tuple
      * @param Filesystem $filesystem
@@ -35,10 +39,10 @@ public function softResolveFileID($fileID, Filesystem $filesystem);
     public function generateVariantFileID($tuple, Filesystem $filesystem);
 
     /**
-     * Try to find a file ID for an existing file the provided file tuple.
+     * Try to find a file ID for an existing file using the provided file tuple.
      * @param array|ParsedFileID $tuple
      * @param Filesystem $filesystem
-     * @param boolean $strict Whatever we should enforce a hash check on the file we find
+     * @param boolean $strict Whether we should enforce a hash check on the file we find
      * @return ParsedFileID|null FileID
      */
     public function searchForTuple($tuple, Filesystem $filesystem, $strict = true);
@@ -47,15 +51,15 @@ public function searchForTuple($tuple, Filesystem $filesystem, $strict = true);
     /**
      * Build a file ID for the provided tuple, irrespective of its existence.
      *
-     * Should always return the prefered file ID for this resolution strategy.
+     * Should always return the preferred file ID for this resolution strategy.
      *
      * @param array|ParsedFileID $tuple
      * @return string
      */
     public function buildFileID($tuple);
 
     /**
-     * Try to resolve the provided file ID string irrespective of whatever it exists on the Filesystem or not.
+     * Try to resolve the provided file ID string irrespective of whether it exists on the Filesystem or not.
      * @param $fileID
      * @return ParsedFileID
      */
@@ -70,7 +74,7 @@ public function parseFileID($fileID);
     public function findVariants($tuple, Filesystem $filesystem);
 
     /**
-     * Normalise a filename to be consistent with this file reoslution startegy.
+     * Normalise a filename to be consistent with this file resolution strategy.
      * @param string $filename
      * @return string
      */
@@ -79,7 +83,7 @@ public function cleanFilename($filename);
     /**
      * Given a fileID string or a Parsed File ID, create a matching ParsedFileID without any variant.
      * @param string|ParsedFileID $fileID
-     * @return ParsedFileID|null A ParsedFileID with a the expected FileID of the original file or null if the provided
+     * @return ParsedFileID|null A ParsedFileID with the expected FileID of the original file or null if the provided
      * $fileID could not be understood
      */
     public function stripVariant($fileID);

src/FilenameParsing/HashFileIDHelper.php

@@ -6,13 +6,15 @@
 use SilverStripe\Core\Injector\Injectable;
 
 /**
- * Parsed Hash path URLs. Hash path group a file and its variant under a directory based on an hash generated from the
+ * Parsed Hash path URLs. Hash paths group a file and its variant under a directory based on a hash generated from the
  * content of the original file.
  *
- * Hash path are used by the Protected asset adapter and was the default for the public adapter prior to
+ * Hash paths are used by the Protected asset adapter and was the default for the public adapter prior to
  * SilverStripe 4.4.
  *
  * e.g.: `Uploads/a1312bc34d/sam__ResizedImageWzYwLDgwXQ.jpg`
+ *
+ * @internal This is still an evolving API. It may change in the next minor release.
  */
 class HashFileIDHelper implements FileIDHelper
 {

src/FilenameParsing/LegacyFileIDHelper.php

@@ -9,6 +9,8 @@
  *
  * SS3 legacy paths are no longer used in SilverStripe 4, but a way to parse them is needed for redirecting old SS3
  * urls.
+ *
+ * @internal This is still an evolving API. It may change in the next minor release.
  */
 class LegacyFileIDHelper implements FileIDHelper
 {
@@ -55,14 +57,14 @@ public function buildFileID($filename, $hash = null, $variant = null)
 
     public function cleanFilename($filename)
     {
-        // There's not really any relevant cleaning rule for legacy. It's not important any way because we won't
+        // There's not really any relevant cleaning rule for legacy. It's not important any way because we won't be
         // generating legacy URLs, aside from maybe for testing.
         return $filename;
     }
 
     /**
      * @note LegacyFileIDHelper is meant to fail when parsing newer format fileIDs with a variant e.g.:
-     * `subfolder/abcdef7890/sam__resizeXYZ.jpg`. When parsing fileIDs without variant, it should return the same
+     * `subfolder/abcdef7890/sam__resizeXYZ.jpg`. When parsing fileIDs without a variant, it should return the same
      * results as natural paths.
      */
     public function parseFileID($fileID)

src/FilenameParsing/NaturalFileIDHelper.php

@@ -5,12 +5,14 @@
 use SilverStripe\Core\Injector\Injectable;
 
 /**
- * Parsed Natural path URLs. Natural path are the same hashless path that appear in the CMS.
+ * Parsed Natural path URLs. Natural path is the same hashless path that appears in the CMS.
  *
  * Natural paths are used by the public adapter from SilverStripe 4.4 and on the protected adapter when
  * `legacy_filenames` is enabled.
  *
  * e.g.: `Uploads/sam__ResizedImageWzYwLDgwXQ.jpg`
+ *
+ * @internal This is still an evolving API. It may changed in the next minor release.
  */
 class NaturalFileIDHelper implements FileIDHelper
 {
@@ -63,7 +65,6 @@ public function cleanFilename($filename)
         return preg_replace('/_{2,}/', '_', $filename);
     }
 
-    /** @todo Update unit to test _resampled variants file id */
     public function parseFileID($fileID)
     {
         $pattern = '#^(?<folder>([^/]+/)*)(?<basename>((?<!__)[^/.])+)(__(?<variant>[^.]+))?(?<extension>(\..+)*)$#';

src/FilenameParsing/ParsedFileID.php

@@ -4,6 +4,8 @@
 
 /**
  * Immutable representation of a parsed fileID broken down into its sub-components.
+ *
+ * @internal This is still an evolving API. It may change in the next minor release.
  */
 class ParsedFileID
 {