Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Companion: bring back default upload protocol #3967

Merged
merged 6 commits into from Aug 11, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions e2e/clients/dashboard-xhr/app.js
@@ -0,0 +1,18 @@
import { Uppy } from '@uppy/core'
import Dashboard from '@uppy/dashboard'
import XHRUpload from '@uppy/xhr-upload'
import Unsplash from '@uppy/unsplash'
import Url from '@uppy/url'

import '@uppy/core/dist/style.css'
import '@uppy/dashboard/dist/style.css'

const companionUrl = 'http://localhost:3020'
const uppy = new Uppy()
.use(Dashboard, { target: '#app', inline: true })
.use(XHRUpload, { endpoint: 'https://xhr-server.herokuapp.com/upload', limit: 6 })
.use(Url, { target: Dashboard, companionUrl })
.use(Unsplash, { target: Dashboard, companionUrl })

// Keep this here to access uppy in tests
window.uppy = uppy
11 changes: 11 additions & 0 deletions e2e/clients/dashboard-xhr/index.html
@@ -0,0 +1,11 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8"/>
<title>dashboard-xhr</title>
<script defer type="module" src="app.js"></script>
</head>
<body>
<div id="app"></div>
</body>
</html>
1 change: 1 addition & 0 deletions e2e/clients/index.html
Expand Up @@ -12,6 +12,7 @@ <h1>Test apps</h1>
<li><a href="react/index.html">react</a></li>
<li><a href="dashboard-transloadit/index.html">dashboard-transloadit</a></li>
<li><a href="dashboard-tus/index.html">dashboard-tus</a></li>
<li><a href="dashboard-xhr/index.html">dashboard-xhr</a></li>
<li><a href="dashboard-ui/index.html">dashboard-ui</a></li>
<li><a href="dashboard-vue/index.html">dashboard-vue</a></li>
</ul>
Expand Down
33 changes: 6 additions & 27 deletions e2e/cypress/integration/dashboard-tus.spec.ts
@@ -1,5 +1,7 @@
import type BaseTus from '@uppy/tus'

import { interceptCompanionUrlRequest, interceptCompanionUnsplashRequest, runRemoteUrlImageUploadTest, runRemoteUnsplashUploadTest } from './util.mjs'

type Tus = BaseTus & {
requests: { isPaused: boolean }
}
Expand All @@ -12,8 +14,8 @@ describe('Dashboard with Tus', () => {
cy.visit('/dashboard-tus')
cy.get('.uppy-Dashboard-input:first').as('file-input')
cy.intercept('/files/*').as('tus')
cy.intercept('http://localhost:3020/url/*').as('url')
cy.intercept('http://localhost:3020/search/unsplash/*').as('unsplash')
interceptCompanionUrlRequest()
interceptCompanionUnsplashRequest()
})

it('should upload cat image successfully', () => {
Expand Down Expand Up @@ -53,33 +55,10 @@ describe('Dashboard with Tus', () => {
)

it('should upload remote image with URL plugin', () => {
cy.get('[data-cy="Url"]').click()
cy.get('.uppy-Url-input').type('https://raw.githubusercontent.com/transloadit/uppy/main/e2e/cypress/fixtures/images/cat.jpg')
cy.get('.uppy-Url-importButton').click()
cy.get('.uppy-StatusBar-actionBtn--upload').click()
cy.wait('@url')
cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete')
runRemoteUrlImageUploadTest()
})

it('should upload remote image with Unsplash plugin', () => {
cy.get('[data-cy="Unsplash"]').click()
cy.get('.uppy-SearchProvider-input').type('book')
cy.get('.uppy-SearchProvider-searchButton').click()
cy.wait('@unsplash')
// Test that the author link is visible
cy.get('.uppy-ProviderBrowserItem')
.first()
.within(() => {
cy.root().click()
// We have hover states that show the author
// but we don't have hover in e2e, so we focus after the click
// to get the same effect. Also tests keyboard users this way.
cy.get('input[type="checkbox"]').focus()
cy.get('a').should('have.css', 'display', 'block')
})
cy.get('.uppy-c-btn-primary').click()
cy.get('.uppy-StatusBar-actionBtn--upload').click()
cy.wait('@unsplash')
cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete')
runRemoteUnsplashUploadTest()
})
})
17 changes: 17 additions & 0 deletions e2e/cypress/integration/dashboard-xhr.spec.ts
@@ -0,0 +1,17 @@
import { interceptCompanionUrlRequest, interceptCompanionUnsplashRequest, runRemoteUrlImageUploadTest, runRemoteUnsplashUploadTest } from './util.mjs'
mifi marked this conversation as resolved.
Show resolved Hide resolved

describe('Dashboard with XHR', () => {
beforeEach(() => {
cy.visit('/dashboard-xhr')
interceptCompanionUrlRequest()
interceptCompanionUnsplashRequest()
})

it('should upload remote image with URL plugin', () => {
runRemoteUrlImageUploadTest()
})

it('should upload remote image with Unsplash plugin', () => {
runRemoteUnsplashUploadTest()
})
})
35 changes: 35 additions & 0 deletions e2e/cypress/integration/util.mjs
@@ -0,0 +1,35 @@
/* global cy */

export const interceptCompanionUrlRequest = () => cy.intercept('http://localhost:3020/url/*').as('url')
export const interceptCompanionUnsplashRequest = () => cy.intercept('http://localhost:3020/search/unsplash/*').as('unsplash')

export function runRemoteUrlImageUploadTest () {
cy.get('[data-cy="Url"]').click()
cy.get('.uppy-Url-input').type('https://raw.githubusercontent.com/transloadit/uppy/main/e2e/cypress/fixtures/images/cat.jpg')
cy.get('.uppy-Url-importButton').click()
cy.get('.uppy-StatusBar-actionBtn--upload').click()
cy.wait('@url')
cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete')
}

export function runRemoteUnsplashUploadTest () {
cy.get('[data-cy="Unsplash"]').click()
cy.get('.uppy-SearchProvider-input').type('book')
cy.get('.uppy-SearchProvider-searchButton').click()
cy.wait('@unsplash')
// Test that the author link is visible
cy.get('.uppy-ProviderBrowserItem')
.first()
.within(() => {
cy.root().click()
// We have hover states that show the author
// but we don't have hover in e2e, so we focus after the click
// to get the same effect. Also tests keyboard users this way.
cy.get('input[type="checkbox"]').focus()
cy.get('a').should('have.css', 'display', 'block')
})
cy.get('.uppy-c-btn-primary').click()
cy.get('.uppy-StatusBar-actionBtn--upload').click()
cy.wait('@unsplash')
cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete')
}
9 changes: 6 additions & 3 deletions packages/@uppy/companion/src/server/Uploader.js
Expand Up @@ -92,8 +92,9 @@ function validateOptions (options) {
}

// validate protocol
if (options.protocol == null || !Object.keys(PROTOCOLS).some((key) => PROTOCOLS[key] === options.protocol)) {
throw new ValidationError('please specify a valid protocol')
// @todo this validation should not be conditional once the protocol field is mandatory
if (options.protocol && !Object.keys(PROTOCOLS).some((key) => PROTOCOLS[key] === options.protocol)) {
throw new ValidationError('unsupported protocol specified')
}

// s3 uploads don't require upload destination
Expand Down Expand Up @@ -206,7 +207,9 @@ class Uploader {
}

async _uploadByProtocol () {
const { protocol } = this.options
// todo a default protocol should not be set. We should ensure that the user specifies their protocol.
// after we drop old versions of uppy client we can remove this
mifi marked this conversation as resolved.
Show resolved Hide resolved
const protocol = this.options.protocol || PROTOCOLS.multipart

switch (protocol) {
case PROTOCOLS.multipart:
Expand Down
2 changes: 1 addition & 1 deletion packages/@uppy/companion/test/__tests__/companion.js
Expand Up @@ -36,7 +36,7 @@ describe('validate upload data', () => {
protocol: 'tusInvalid',
})
.expect(400)
.then((res) => expect(res.body.message).toBe('please specify a valid protocol'))
.then((res) => expect(res.body.message).toBe('unsupported protocol specified'))
})

test('invalid upload fieldname gets rejected', () => {
Expand Down
7 changes: 0 additions & 7 deletions packages/@uppy/companion/test/__tests__/uploader.js
Expand Up @@ -20,13 +20,10 @@ process.env.COMPANION_DATADIR = './test/output'
process.env.COMPANION_DOMAIN = 'localhost:3020'
const { companionOptions } = standalone()

const protocol = 'tus'

describe('uploader with tus protocol', () => {
test('uploader respects uploadUrls', async () => {
const opts = {
endpoint: 'http://localhost/files',
protocol,
companionOptions: { ...companionOptions, uploadUrls: [/^http:\/\/url.myendpoint.com\//] },
}

Expand All @@ -36,7 +33,6 @@ describe('uploader with tus protocol', () => {
test('uploader respects uploadUrls, valid', async () => {
const opts = {
endpoint: 'http://url.myendpoint.com/files',
protocol,
companionOptions: { ...companionOptions, uploadUrls: [/^http:\/\/url.myendpoint.com\//] },
}

Expand All @@ -47,7 +43,6 @@ describe('uploader with tus protocol', () => {
test('uploader respects uploadUrls, localhost', async () => {
const opts = {
endpoint: 'http://localhost:1337/',
protocol,
companionOptions: { ...companionOptions, uploadUrls: [/^http:\/\/localhost:1337\//] },
}

Expand Down Expand Up @@ -231,7 +226,6 @@ describe('uploader with tus protocol', () => {
const opts = {
companionOptions,
endpoint: 'http://localhost',
protocol,
}

// eslint-disable-next-line no-new
Expand All @@ -253,7 +247,6 @@ describe('uploader with tus protocol', () => {
test('uploader respects maxFileSize correctly', async () => {
const opts = {
endpoint: 'http://url.myendpoint.com/files',
protocol,
companionOptions: { ...companionOptions, maxFileSize: 100 },
size: 99,
}
Expand Down