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
FileSize
20.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
FileSize
22.23 KB
FAILED: [[SimpleTest]]: [MySQL] 64,224 pass(es), 4 fail(s), and 2 exception(s). View
7.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
FileSize
29.36 KB
FAILED: [[SimpleTest]]: [MySQL] 64,233 pass(es), 1 fail(s), and 0 exception(s). View
10.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
FileSize
30.23 KB
PASSED: [[SimpleTest]]: [MySQL] 64,181 pass(es). View
894 bytes

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

Les Lim’s picture

FileSize
32.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
2.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

FileSize
89.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
FileSize
76.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.

tim.plunkett’s picture

Version:8.0.x-dev» 8.1.x-dev

We could probably do this in a non-BC-breaking way in 8.1.x

Version:8.1.x-dev» 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.