If it worked for #2028109: Convert hook_stream_wrappers() to tagged services. it can work here too!

Files: 
CommentFileSizeAuthor
#21 d8-convert-hook-filetransfer-info-plugin-2038271-21.patch76.31 KBsplatio
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 69,621 pass(es), 4 fail(s), and 3 exception(s).
[ View ]
#18 d8-convert-hook-filetransfer-info-plugin-2038271-18.patch89.64 KBsplatio
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
#15 interdiff-2038271-14-15.txt2.44 KBLes Lim
#15 core-2038271-15-filetransfer-to-plugins.patch32.21 KBLes Lim
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch core-2038271-15-filetransfer-to-plugins.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#14 interdiff-2038271-10-14.txt894 bytesLes Lim
#14 core-2038271-14-filetransfer-to-plugins.patch30.23 KBLes Lim
PASSED: [[SimpleTest]]: [MySQL] 64,181 pass(es).
[ View ]
#10 interdiff-2038271-8-9.txt10.09 KBLes Lim
#10 core-2038271-9-filetransfer-to-plugins.patch29.36 KBLes Lim
FAILED: [[SimpleTest]]: [MySQL] 64,233 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#8 interdiff-2038271-1-8.txt7.6 KBLes Lim
#8 core-2038271-8-filetransfer-to-plugins.patch22.23 KBLes Lim
FAILED: [[SimpleTest]]: [MySQL] 64,224 pass(es), 4 fail(s), and 2 exception(s).
[ View ]
#1 filetransfer-2038271-1.patch20.93 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 56,924 pass(es).
[ View ]

Comments

tim.plunkett’s picture

Status:Active» Needs review
StatusFileSize
new20.93 KB
PASSED: [[SimpleTest]]: [MySQL] 56,924 pass(es).
[ View ]

The whole FileTransfer subsystem needs a good deal of refactoring. I'm just doing the discovery parts for now.

damiankloip’s picture

Status:Needs review» Needs work
+++ b/core/lib/Drupal/Core/Annotation/FileTransfer.phpundefined
@@ -0,0 +1,44 @@
+ * Defines an FileTransfer annotation object.

a

+++ b/core/lib/Drupal/Core/FileTransfer/FileTransferManager.phpundefined
@@ -0,0 +1,60 @@
+ * Register information about FileTransfer classes.

Registers?

+++ b/core/lib/Drupal/Core/FileTransfer/FileTransferManager.phpundefined
@@ -0,0 +1,60 @@
+    uasort($definitions, function ($a, $b) {
+      if ($a['weight'] == $b['weight']) {
+        return 0;
+      }
+      return ($a['weight'] < $b['weight']) ? -1 : 1;

Out of scope here, but this is pretty similar to drupal_sort_weight, so I think we could start reusing that. I would certainly not add a procedural function in here though, so it's good for now.

+++ b/core/lib/Drupal/Core/FileTransfer/Plugin/FileTransfer/FTPExtension.phpundefined
@@ -5,10 +5,21 @@
+ * @FileTransfer(
+ *   id = "ftp",
+ *   title = @Translation("FTP")
+ * )

Should we add the weight in here anyway?

+++ b/core/lib/Drupal/Core/FileTransfer/Plugin/FileTransfer/SSH.phpundefined
@@ -5,12 +5,24 @@
+class SSH extends FileTransferBase implements ChmodInterface {

Maybe we should use this opportunity to rename to 'Ssh', as I think that's the correct camel casing?

+++ b/core/modules/system/system.moduleundefined
@@ -2238,7 +2238,7 @@ function system_authorized_init($callback, $file, $arguments = array(), $page_ti
+  $_SESSION['authorize_filetransfer_info'] = Drupal::service('plugin.manager.filetransfer')->getDefinitions();

I guess this doesn't need to be user specific, wonder if this could be stored elsewhere? Probably not in scope of this conversion though.

+++ b/core/modules/update/tests/modules/update_test/lib/Drupal/update_test/Plugin/FileTransfer/MockFileTransfer.phpundefined
@@ -2,24 +2,33 @@
   public static function factory() {
-    return new FileTransfer;
+    return new static();

If we are changing this method a bit anyway, should we use create() instead? to bring it inline with all of our other 'things'.

damiankloip’s picture

#2028499: drupal_sort_weight should be converted to a class is the issue I was thinking of for drupal_sort_weight

tim.plunkett’s picture

Priority:Normal» Major
Issue tags:+Approved API change
tim.plunkett’s picture

Assigned:tim.plunkett» Unassigned
Issue tags:-Approved API change
dlu’s picture

Component:base system» file system

Moved to filesystem component per #2050763-16: Refine "base system" component (notes on refactoring of "base system" category here: https://docs.google.com/a/acquia.com/spreadsheet/ccc?key=0AusehVccVSq2dF...). Might also involve modules.

tim.plunkett’s picture

This actually has nothing to do with the file system as we define it (file.inc), it's actually used by authorize.php and update.php and is in common.inc, so I think it does belong here.

Les Lim’s picture

Issue summary:View changes
Status:Needs work» Needs review
Parent issue:» #2044203: [meta] Convert info hooks to plugins
StatusFileSize
new22.23 KB
FAILED: [[SimpleTest]]: [MySQL] 64,224 pass(es), 4 fail(s), and 2 exception(s).
[ View ]
new7.6 KB

Rerolled #1 and incorporated the review from #2. Notes:

  • Renamed FTP.php to Ftp.php, SSH.php to Ssh.php, and FTPExtension.php to FtpExtension.php. This may be a problem for people on case-insensitive filesystems.
  • Changed the MockFileTransfer::factory() methods to create() methods. Not sure if this requires changes in any implementing code, since I couldn't figure out where either MockFileTransfer class is used.

Status:Needs review» Needs work

The last submitted patch, 8: core-2038271-8-filetransfer-to-plugins.patch, failed testing.

Les Lim’s picture

Status:Needs work» Needs review
StatusFileSize
new29.36 KB
FAILED: [[SimpleTest]]: [MySQL] 64,233 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new10.09 KB

Changes since #8:

  • Changed factory() method to create() in FileTransferBase and its children
  • Fixed FileTransferManager::__construct() signature
  • @inheritdoc use

Status:Needs review» Needs work

The last submitted patch, 10: core-2038271-9-filetransfer-to-plugins.patch, failed testing.

Les Lim’s picture

The last submitted patch, 10: core-2038271-9-filetransfer-to-plugins.patch, failed testing.

Les Lim’s picture

Status:Needs work» Needs review
StatusFileSize
new30.23 KB
PASSED: [[SimpleTest]]: [MySQL] 64,181 pass(es).
[ View ]
new894 bytes

Missed a factory() method that should be create() now.

Les Lim’s picture

StatusFileSize
new32.21 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch core-2038271-15-filetransfer-to-plugins.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new2.44 KB

Shoot, missed another one. Should've grepped first.

sun’s picture

Status:Needs review» Needs work

The last submitted patch, 15: core-2038271-15-filetransfer-to-plugins.patch, failed testing.

splatio’s picture

StatusFileSize
new89.64 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]

Rerolled the patch from #15.

splatio’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 18: d8-convert-hook-filetransfer-info-plugin-2038271-18.patch, failed testing.

splatio’s picture

Status:Needs work» Needs review
StatusFileSize
new76.31 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 69,621 pass(es), 4 fail(s), and 3 exception(s).
[ View ]

Removes the addition of authorize.php which had been mistakenly added in the reroll. Fixed the installer issue.

Status:Needs review» Needs work

The last submitted patch, 21: d8-convert-hook-filetransfer-info-plugin-2038271-21.patch, failed testing.