Problem/Motivation

When calling TemporaryStream::getExternalUrl() I get this error:

Fatal error: Call to undefined method Drupal\Core\StreamWrapper\TemporaryStream::url() in /var/www/drupal8/core/lib/Drupal/Core/StreamWrapper/TemporaryStream.php on line 33

Proposed resolution

Fix this by calling Url::fromRoute() instead of non-existent function.

Remaining tasks

- prepare patch
- write tests
- review

User interface changes

None

API changes

None

CommentFileSizeAuthor
#11 interdiff.txt1.06 KBslashrsm
#11 2352855_11.patch2.94 KBslashrsm
#7 interdiff.txt1.36 KBslashrsm
#7 2352855_7.patch2.29 KBslashrsm
#1 2352855_1_TEST_ONLY.patch1.25 KBslashrsm
#1 2352855_1.patch2.04 KBslashrsm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

slashrsm’s picture

slashrsm CreditAttribution: slashrsm commented
Status: Active » Needs review
FileSize
2352855_1.patch2.04 KB
2352855_1_TEST_ONLY.patch1.25 KB
slashrsm’s picture

slashrsm CreditAttribution: slashrsm commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 1: 2352855_1_TEST_ONLY.patch, failed testing.

slashrsm’s picture

slashrsm CreditAttribution: slashrsm commented
Status: Needs work » Needs review
Primsi’s picture

Primsi CreditAttribution: Primsi commented
Status: Needs review » Reviewed & tested by the community

Looks ok.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/system/src/Tests/File/StreamWrapperTest.php
@@ -84,6 +84,15 @@ function testUriFunctions() {
+    $this->assertTrue(file_create_url('temporary://test.txt'), t('Temporary external URL correctly built.'));
+    $this->assertTrue(file_create_url('public://test.txt'), t('Public external URL correctly built.'));
+    $this->assertTrue(file_create_url('private://test.txt'), t('Private external URL correctly built.'));

Not supposed to use t() on test assertion messages and we're not really testing the that the URL is correct here by user assertTrue are we?

slashrsm’s picture

slashrsm CreditAttribution: slashrsm commented
Status: Needs work » Needs review
FileSize
2352855_7.patch2.29 KB
interdiff.txt1.36 KB

Ah, right! Here we go.

Ibn al-Hazardous’s picture

Ibn al-Hazardous CreditAttribution: Ibn al-Hazardous commented

Hello,
is this patch going into the next beta?

I get a white screen of death going to /admin/content/files, and the log shows the same fatal error as in the initial report.

The latest patch works perfectly for me, so who do I pester to see it applied? ;-)

slashrsm queued 7: 2352855_7.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 7: 2352855_7.patch, failed testing.

slashrsm’s picture

slashrsm CreditAttribution: slashrsm commented
Status: Needs work » Needs review
FileSize
2352855_11.patch2.94 KB
interdiff.txt1.06 KB
marcingy’s picture

marcingy CreditAttribution: marcingy commented
Status: Needs review » Reviewed & tested by the community

Looks good

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2352855_11.patch, failed testing.

slashrsm queued 11: 2352855_11.patch for re-testing.

slashrsm’s picture

slashrsm CreditAttribution: slashrsm commented
Status: Needs work » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed d3703f9 on 8.0.x 
    Issue #2352855 by slashrsm: TemporaryStream::getExternalUrl() is broken

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.