Foundation for media API testing (#136)

* cmd/mediaapi-integration-tests: Add foundation for testing

* common/test: Add some server init and client request utilities

* common/test/client: Handle timed out requests for tests that passed

* cmd/syncserver-integration-tests: Port to new common/test infra

* common/test/client: Remove stray debug print

* cmd/mediaapi-integration-tests: Simplify slice initialisation

* cmd/mediaapi-integration-tests: Simplify getMediaURL argument

* cmd/mediaapi-integration-tests: Make startMediaAPI return listen address

* common/test/client: Fix uninitialised LastRequestErr

* common/test/server: Remove redundant argument

* common/test/server: Add StartProxy to create a reverse proxy

* cmd/mediaapi-integration-tests: Add proxies in front of servers

This is needed so that origins can be correctly configured and used for
remote media.

* travis: Enable media API integration tests

* travis: Build the client-api-proxy for media tests

* common/test/client: Don't panic on EOF in CanonicalJSONInput

* cmd/mediaapi-integration-tests: Add upload/download/thumbnail tests

* mediaapi/thumbnailer: Store thumbnail according to requested size

* cmd/mediaapi-integration-tests: Add totem.jpg test file

* cmd/client-api-proxy: Optionally listen for HTTPS

* common/test/client: Do not verify TLS certs for testing

We will commonly use self-signed certs.

* cmd/mediaapi-integration-tests: Make HTTPS requests

* cmd/mediaapi-integration-tests: Log size and method for thumbnails

* mediaapi/thumbnailer: Factor out isThumbnailExists

Appease gocyclo^w^w simplify

* mediaapi/thumbnailer: Check if request is larger than original

* travis: Install openssl and generate server.{crt,key}

* cmd/mediaapi-integration-tests: Add valid dynamic thumbnail test

* cmd/mediaapi-integration-tests: Document state of tests

* cmd/mediaapi-integration-tests: Test remote thumbnail before download

This ordering also exercises the cold cache immediate generation of a
size configured for pregeneration.

* travis: Explain openssl key+cert generation

* common/test/server: Clarify postgresContainerName

.travis.yml

@@ -8,6 +8,9 @@ sudo: false
 dist: trusty
 
 addons:
+  apt:
+    packages:
+    - openssl
   postgresql: "9.5"
 
 services:
@@ -18,6 +21,10 @@ install:
  - go get github.com/golang/lint/golint
  - go get github.com/fzipp/gocyclo
 
+# Generate a self-signed X.509 certificate for TLS.
+before_script:
+ - openssl req -x509 -newkey rsa:4096 -keyout server.key -out server.crt -days 365 -nodes -subj /CN=localhost
+
 script:
  - ./travis-install-kafka.sh
  - ./travis-test.sh
@@ -29,4 +36,3 @@ notifications:
         on_success: change  # always|never|change
         on_failure: always
         on_start: never
-

src/github.com/matrix-org/dendrite/cmd/client-api-proxy/main.go

@@ -51,6 +51,8 @@ var (
 	clientAPIURL  = flag.String("client-api-server-url", "", "The base URL of the listening 'dendrite-client-api-server' process. E.g. 'http://localhost:4321'")
 	mediaAPIURL   = flag.String("media-api-server-url", "", "The base URL of the listening 'dendrite-media-api-server' process. E.g. 'http://localhost:7779'")
 	bindAddress   = flag.String("bind-address", ":8008", "The listening port for the proxy.")
+	certFile      = flag.String("tls-cert", "server.crt", "The PEM formatted X509 certificate to use for TLS")
+	keyFile       = flag.String("tls-key", "server.key", "The PEM private key to use for TLS")
 )
 
 func makeProxy(targetURL string) (*httputil.ReverseProxy, error) {
@@ -148,6 +150,9 @@ func main() {
 	fmt.Println("  /_matrix/media/v1        => ", *mediaAPIURL+"/api/_matrix/media/v1")
 	fmt.Println("  /*                       => ", *clientAPIURL+"/api/*")
 	fmt.Println("Listening on ", *bindAddress)
-	srv.ListenAndServe()
+	if *certFile != "" && *keyFile != "" {
-
+		panic(srv.ListenAndServeTLS(*certFile, *keyFile))
+	} else {
+		panic(srv.ListenAndServe())
+	}
 }

src/github.com/matrix-org/dendrite/cmd/dendrite-media-api-server/main.go

@@ -15,6 +15,7 @@
 package main
 
 import (
+	"flag"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -36,27 +37,29 @@ import (
 )
 
 var (
-	bindAddr   = os.Getenv("BIND_ADDRESS")
+	bindAddr   = flag.String("listen", "", "The port to listen on.")
 	dataSource = os.Getenv("DATABASE")
 	logDir     = os.Getenv("LOG_DIR")
 	serverName = os.Getenv("SERVER_NAME")
 	basePath   = os.Getenv("BASE_PATH")
 	// Note: if the MAX_FILE_SIZE_BYTES is set to 0, it will be unlimited
 	maxFileSizeBytesString = os.Getenv("MAX_FILE_SIZE_BYTES")
-	configPath             = os.Getenv("CONFIG_PATH")
+	configPath             = flag.String("config", "", "The path to the config file. For more information, see the config file in this repository.")
 )
 
 func main() {
 	common.SetupLogging(logDir)
 
+	flag.Parse()
+
 	log.WithFields(log.Fields{
-		"BIND_ADDRESS":        bindAddr,
+		"listen":              *bindAddr,
 		"DATABASE":            dataSource,
 		"LOG_DIR":             logDir,
 		"SERVER_NAME":         serverName,
 		"BASE_PATH":           basePath,
 		"MAX_FILE_SIZE_BYTES": maxFileSizeBytesString,
-		"CONFIG_PATH":         configPath,
+		"config":              *configPath,
 	}).Info("Loading configuration based on config file and environment variables")
 
 	cfg, err := configureServer()
@@ -64,15 +67,10 @@ func main() {
 		log.WithError(err).Fatal("Invalid configuration")
 	}
 
-	db, err := storage.Open(cfg.DataSource)
-	if err != nil {
-		log.WithError(err).Panic("Failed to open database")
-	}
-
 	log.WithFields(log.Fields{
-		"BIND_ADDRESS":           bindAddr,
+		"listen":                 *bindAddr,
 		"LOG_DIR":                logDir,
-		"CONFIG_PATH":            configPath,
+		"CONFIG_PATH":            *configPath,
 		"ServerName":             cfg.ServerName,
 		"AbsBasePath":            cfg.AbsBasePath,
 		"MaxFileSizeBytes":       *cfg.MaxFileSizeBytes,
@@ -82,13 +80,21 @@ func main() {
 		"ThumbnailSizes":         cfg.ThumbnailSizes,
 	}).Info("Starting mediaapi server with configuration")
 
+	db, err := storage.Open(cfg.DataSource)
+	if err != nil {
+		log.WithError(err).Panic("Failed to open database")
+	}
+
 	routing.Setup(http.DefaultServeMux, http.DefaultClient, cfg, db)
-	log.Fatal(http.ListenAndServe(bindAddr, nil))
+	log.Fatal(http.ListenAndServe(*bindAddr, nil))
 }
 
 // configureServer loads configuration from a yaml file and overrides with environment variables
 func configureServer() (*config.MediaAPI, error) {
-	cfg, err := loadConfig(configPath)
+	if *configPath == "" {
+		log.Fatal("--config must be supplied")
+	}
+	cfg, err := loadConfig(*configPath)
 	if err != nil {
 		log.WithError(err).Fatal("Invalid config file")
 	}
@@ -172,14 +178,14 @@ func applyOverrides(cfg *config.MediaAPI) {
 	if cfg.MaxThumbnailGenerators == 0 {
 		log.WithField(
 			"max_thumbnail_generators", cfg.MaxThumbnailGenerators,
-		).Info("Using default max_thumbnail_generators")
+		).Info("Using default max_thumbnail_generators value of 10")
 		cfg.MaxThumbnailGenerators = 10
 	}
 }
 
 func validateConfig(cfg *config.MediaAPI) error {
-	if bindAddr == "" {
+	if *bindAddr == "" {
-		return fmt.Errorf("no BIND_ADDRESS environment variable found")
+		log.Fatal("--listen must be supplied")
 	}
 
 	absBasePath, err := getAbsolutePath(cfg.BasePath)

src/github.com/matrix-org/dendrite/cmd/mediaapi-integration-tests/TESTS.md

@@ -0,0 +1,69 @@
+# Media API Tests
+
+## Implemented
+
+* functional
+    * upload
+        * normal case
+    * download
+        * local file
+            * existing
+            * non-existing
+        * remote file
+            * existing
+    * thumbnail
+        * original file formats
+            * JPEG
+        * local file
+            * existing
+        * remote file
+            * existing
+        * cache
+            * cold
+            * hot
+        * pre-generation according to configuration
+            * scale
+            * crop
+        * dynamic generation
+            * cold cache
+            * larger than original
+            * scale
+
+## TODO
+
+* functional
+    * upload
+        * file too large
+        * 0-byte file?
+        * invalid filename
+        * invalid content-type
+    * download
+        * invalid origin
+        * invalid media id
+    * thumbnail
+        * original file formats
+            * GIF
+            * PNG
+            * BMP
+            * SVG
+            * PDF
+            * TIFF
+            * WEBP
+        * local file
+            * non-existing
+        * remote file
+            * non-existing
+        * pre-generation according to configuration
+            * manual verification + hash check for regressions?
+        * dynamic generation
+            * hot cache
+            * limit on dimensions?
+            * 0x0
+            * crop
+* load
+    * 100 parallel requests
+        * same file
+        * different local files
+        * different remote files
+        * pre-generated thumbnails
+        * non-pre-generated thumbnails

src/github.com/matrix-org/dendrite/cmd/mediaapi-integration-tests/main.go

@@ -0,0 +1,272 @@
+// Copyright 2017 Vector Creations Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"os"
+	"os/exec"
+	"path"
+	"path/filepath"
+	"strconv"
+	"time"
+
+	"github.com/matrix-org/dendrite/common/test"
+)
+
+var (
+	// How long to wait for the server to write the expected output messages.
+	// This needs to be high enough to account for the time it takes to create
+	// the postgres database tables which can take a while on travis.
+	timeoutString = test.Defaulting(os.Getenv("TIMEOUT"), "10s")
+	// The name of maintenance database to connect to in order to create the test database.
+	postgresDatabase = test.Defaulting(os.Getenv("POSTGRES_DATABASE"), "postgres")
+	// The name of the test database to create.
+	testDatabaseName = test.Defaulting(os.Getenv("DATABASE_NAME"), "mediaapi_test")
+	// Postgres docker container name (for running psql). If not set, psql must be in PATH.
+	postgresContainerName = os.Getenv("POSTGRES_CONTAINER")
+	// Test image to be uploaded/downloaded
+	testJPEG = test.Defaulting(os.Getenv("TEST_JPEG_PATH"), "src/github.com/matrix-org/dendrite/cmd/mediaapi-integration-tests/totem.jpg")
+)
+
+var thumbnailPregenerationConfig = (`
+thumbnail_sizes:
+- width: 32
+  height: 32
+  method: crop
+- width: 96
+  height: 96
+  method: crop
+- width: 320
+  height: 240
+  method: scale
+- width: 640
+  height: 480
+  method: scale
+- width: 800
+  height: 600
+  method: scale
+`)
+
+const serverType = "media-api"
+
+var testDatabaseTemplate = "dbname=%s sslmode=disable binary_parameters=yes"
+
+var timeout time.Duration
+
+func startMediaAPI(suffix string, dynamicThumbnails bool) (*exec.Cmd, chan error, string, *exec.Cmd, chan error, string, string) {
+	dir, err := ioutil.TempDir("", serverType+"-server-test"+suffix)
+	if err != nil {
+		panic(err)
+	}
+
+	serverAddr := "localhost:177" + suffix + "9"
+	proxyAddr := "localhost:1800" + suffix
+
+	configFilename := serverType + "-server-test-config" + suffix + ".yaml"
+	configFileContents := makeConfig(proxyAddr, suffix, dir, dynamicThumbnails)
+
+	serverArgs := []string{
+		"--config", configFilename,
+		"--listen", serverAddr,
+	}
+
+	databases := []string{
+		testDatabaseName + suffix,
+	}
+
+	proxyCmd, proxyCmdChan := test.StartProxy(
+		proxyAddr,
+		"http://localhost:177"+suffix+"6",
+		"http://localhost:177"+suffix+"8",
+		"http://"+serverAddr,
+	)
+
+	cmd, cmdChan := test.StartServer(
+		serverType,
+		serverArgs,
+		suffix,
+		configFilename,
+		configFileContents,
+		postgresDatabase,
+		postgresContainerName,
+		databases,
+	)
+
+	fmt.Printf("==TESTSERVER== STARTED %v -> %v : %v\n", proxyAddr, serverAddr, dir)
+	return cmd, cmdChan, serverAddr, proxyCmd, proxyCmdChan, proxyAddr, dir
+}
+
+func makeConfig(serverAddr, suffix, basePath string, dynamicThumbnails bool) string {
+	return fmt.Sprintf(
+		`
+server_name: "%s"
+base_path: %s
+max_file_size_bytes: %s
+database: "%s"
+dynamic_thumbnails: %s
+%s`,
+		serverAddr,
+		basePath,
+		"10485760",
+		fmt.Sprintf(testDatabaseTemplate, testDatabaseName+suffix),
+		strconv.FormatBool(dynamicThumbnails),
+		thumbnailPregenerationConfig,
+	)
+}
+
+func cleanUpServer(cmd *exec.Cmd, dir string) {
+	cmd.Process.Kill() // ensure server is dead, only cleaning up so don't care about errors this returns.
+	if err := os.RemoveAll(dir); err != nil {
+		fmt.Printf("WARNING: Failed to remove temporary directory %v: %q\n", dir, err)
+	}
+}
+
+// Runs a battery of media API server tests
+// The tests will pause at various points in this list to conduct tests on the HTTP responses before continuing.
+func main() {
+	fmt.Println("==TESTING==", os.Args[0])
+
+	var err error
+	timeout, err = time.ParseDuration(timeoutString)
+	if err != nil {
+		fmt.Printf("ERROR: Invalid timeout string %v: %q\n", timeoutString, err)
+		return
+	}
+
+	// create server1 with only pre-generated thumbnails allowed
+	server1Cmd, server1CmdChan, _, server1ProxyCmd, _, server1ProxyAddr, server1Dir := startMediaAPI("1", false)
+	defer cleanUpServer(server1Cmd, server1Dir)
+	defer server1ProxyCmd.Process.Kill()
+	testDownload(server1ProxyAddr, server1ProxyAddr, "doesnotexist", "", 404, server1CmdChan)
+
+	// upload a JPEG file
+	testUpload(server1ProxyAddr, testJPEG, "image/jpeg", `{
+		"content_uri": "mxc://localhost:18001/1VuVy8u_hmDllD8BrcY0deM34Bl7SPJeY9J6BkMmpx0"
+	}`, 200, server1CmdChan)
+
+	// download that JPEG file
+	testDownload(server1ProxyAddr, "localhost:18001", "1VuVy8u_hmDllD8BrcY0deM34Bl7SPJeY9J6BkMmpx0", "", 200, server1CmdChan)
+
+	// thumbnail that JPEG file
+	testThumbnail(64, 64, "crop", server1ProxyAddr, "localhost:18001", "1VuVy8u_hmDllD8BrcY0deM34Bl7SPJeY9J6BkMmpx0", "", 200, server1CmdChan)
+
+	// create server2 with dynamic thumbnail generation
+	server2Cmd, server2CmdChan, _, server2ProxyCmd, _, server2ProxyAddr, server2Dir := startMediaAPI("2", true)
+	defer cleanUpServer(server2Cmd, server2Dir)
+	defer server2ProxyCmd.Process.Kill()
+	testDownload(server2ProxyAddr, server2ProxyAddr, "doesnotexist", "", 404, server2CmdChan)
+
+	// pre-generated thumbnail that JPEG file via server2
+	testThumbnail(800, 600, "scale", server2ProxyAddr, "localhost:18001", "1VuVy8u_hmDllD8BrcY0deM34Bl7SPJeY9J6BkMmpx0", "", 200, server2CmdChan)
+
+	// download that JPEG file via server2
+	testDownload(server2ProxyAddr, "localhost:18001", "1VuVy8u_hmDllD8BrcY0deM34Bl7SPJeY9J6BkMmpx0", "", 200, server2CmdChan)
+
+	// dynamic thumbnail that JPEG file via server2
+	testThumbnail(1920, 1080, "scale", server2ProxyAddr, "localhost:18001", "1VuVy8u_hmDllD8BrcY0deM34Bl7SPJeY9J6BkMmpx0", "", 200, server2CmdChan)
+
+	// thumbnail that JPEG file via server2
+	testThumbnail(10000, 10000, "scale", server2ProxyAddr, "localhost:18001", "1VuVy8u_hmDllD8BrcY0deM34Bl7SPJeY9J6BkMmpx0", "", 200, server2CmdChan)
+
+}
+
+func getMediaURI(scheme, host, endpoint, query string, components []string) string {
+	pathComponents := []string{host, "_matrix/media/v1", endpoint}
+	pathComponents = append(pathComponents, components...)
+	return scheme + path.Join(pathComponents...) + query
+}
+
+func testUpload(host, filePath, contentType, wantedBody string, wantedStatusCode int, serverCmdChan chan error) {
+	fmt.Printf("==TESTING== upload %v to %v\n", filePath, host)
+	file, err := os.Open(filePath)
+	defer file.Close()
+	if err != nil {
+		panic(err)
+	}
+	filename := filepath.Base(filePath)
+	stat, err := file.Stat()
+	if os.IsNotExist(err) {
+		panic(err)
+	}
+	fileSize := stat.Size()
+
+	req, err := http.NewRequest(
+		"POST",
+		getMediaURI("https://", host, "upload", "?filename="+filename, nil),
+		file,
+	)
+	if err != nil {
+		panic(err)
+	}
+	req.ContentLength = fileSize
+	req.Header.Set("Content-Type", contentType)
+
+	testReq := &test.Request{
+		Req:              req,
+		WantedStatusCode: wantedStatusCode,
+		WantedBody:       test.CanonicalJSONInput([]string{wantedBody})[0],
+	}
+	if err := testReq.Do(); err != nil {
+		panic(err)
+	}
+	fmt.Printf("==TESTING== upload %v to %v PASSED\n", filePath, host)
+}
+
+func testDownload(host, origin, mediaID, wantedBody string, wantedStatusCode int, serverCmdChan chan error) {
+	req, err := http.NewRequest(
+		"GET",
+		getMediaURI("https://", host, "download", "", []string{
+			origin,
+			mediaID,
+		}),
+		nil,
+	)
+	if err != nil {
+		panic(err)
+	}
+	testReq := &test.Request{
+		Req:              req,
+		WantedStatusCode: wantedStatusCode,
+		WantedBody:       test.CanonicalJSONInput([]string{wantedBody})[0],
+	}
+	testReq.Run(fmt.Sprintf("download mxc://%v/%v from %v", origin, mediaID, host), timeout, serverCmdChan)
+}
+
+func testThumbnail(width, height int, resizeMethod, host, origin, mediaID, wantedBody string, wantedStatusCode int, serverCmdChan chan error) {
+	query := fmt.Sprintf("?width=%v&height=%v", width, height)
+	if resizeMethod != "" {
+		query += "&method=" + resizeMethod
+	}
+	req, err := http.NewRequest(
+		"GET",
+		getMediaURI("https://", host, "thumbnail", query, []string{
+			origin,
+			mediaID,
+		}),
+		nil,
+	)
+	if err != nil {
+		panic(err)
+	}
+	testReq := &test.Request{
+		Req:              req,
+		WantedStatusCode: wantedStatusCode,
+		WantedBody:       test.CanonicalJSONInput([]string{wantedBody})[0],
+	}
+	testReq.Run(fmt.Sprintf("thumbnail mxc://%v/%v%v from %v", origin, mediaID, query, host), timeout, serverCmdChan)
+}

src/github.com/matrix-org/dendrite/cmd/syncserver-integration-tests/main.go

@@ -17,13 +17,10 @@ package main
 import (
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"os"
 	"os/exec"
 	"path/filepath"
-	"strings"
-	"sync"
 	"time"
 
 	"github.com/matrix-org/dendrite/common/test"
@@ -33,23 +30,25 @@ import (
 
 var (
 	// Path to where kafka is installed.
-	kafkaDir = defaulting(os.Getenv("KAFKA_DIR"), "kafka")
+	kafkaDir = test.Defaulting(os.Getenv("KAFKA_DIR"), "kafka")
 	// The URI the kafka zookeeper is listening on.
-	zookeeperURI = defaulting(os.Getenv("ZOOKEEPER_URI"), "localhost:2181")
+	zookeeperURI = test.Defaulting(os.Getenv("ZOOKEEPER_URI"), "localhost:2181")
 	// The URI the kafka server is listening on.
-	kafkaURI = defaulting(os.Getenv("KAFKA_URIS"), "localhost:9092")
+	kafkaURI = test.Defaulting(os.Getenv("KAFKA_URIS"), "localhost:9092")
 	// The address the syncserver should listen on.
-	syncserverAddr = defaulting(os.Getenv("SYNCSERVER_URI"), "localhost:9876")
+	syncserverAddr = test.Defaulting(os.Getenv("SYNCSERVER_URI"), "localhost:9876")
 	// How long to wait for the syncserver to write the expected output messages.
 	// This needs to be high enough to account for the time it takes to create
 	// the postgres database tables which can take a while on travis.
-	timeoutString = defaulting(os.Getenv("TIMEOUT"), "10s")
+	timeoutString = test.Defaulting(os.Getenv("TIMEOUT"), "10s")
 	// The name of maintenance database to connect to in order to create the test database.
-	postgresDatabase = defaulting(os.Getenv("POSTGRES_DATABASE"), "postgres")
+	postgresDatabase = test.Defaulting(os.Getenv("POSTGRES_DATABASE"), "postgres")
+	// Postgres docker container name (for running psql). If not set, psql must be in PATH.
+	postgresContainerName = os.Getenv("POSTGRES_CONTAINER")
 	// The name of the test database to create.
-	testDatabaseName = defaulting(os.Getenv("DATABASE_NAME"), "syncserver_test")
+	testDatabaseName = test.Defaulting(os.Getenv("DATABASE_NAME"), "syncserver_test")
 	// The postgres connection config for connecting to the test database.
-	testDatabase = defaulting(os.Getenv("DATABASE"), fmt.Sprintf("dbname=%s sslmode=disable binary_parameters=yes", testDatabaseName))
+	testDatabase = test.Defaulting(os.Getenv("DATABASE"), fmt.Sprintf("dbname=%s sslmode=disable binary_parameters=yes", testDatabaseName))
 )
 
 const inputTopic = "syncserverInput"
@@ -63,36 +62,12 @@ var exe = test.KafkaExecutor{
 	OutputWriter: os.Stderr,
 }
 
-var (
-	lastRequestMutex sync.Mutex
-	lastRequestErr   error
-)
-
-func setLastRequestError(err error) {
-	lastRequestMutex.Lock()
-	defer lastRequestMutex.Unlock()
-	lastRequestErr = err
-}
-
-func getLastRequestError() error {
-	lastRequestMutex.Lock()
-	defer lastRequestMutex.Unlock()
-	return lastRequestErr
-}
-
 var syncServerConfigFileContents = (`consumer_uris: ["` + kafkaURI + `"]
 roomserver_topic: "` + inputTopic + `"
 database: "` + testDatabase + `"
 server_name: "localhost"
 `)
 
-func defaulting(value, defaultValue string) string {
-	if value == "" {
-		value = defaultValue
-	}
-	return value
-}
-
 var timeout time.Duration
 var clientEventTestData []string
 
@@ -108,31 +83,6 @@ func init() {
 	}
 }
 
-// TODO: dupes roomserver integration tests. Factor out.
-func createDatabase(database string) error {
-	cmd := exec.Command("psql", postgresDatabase)
-	cmd.Stdin = strings.NewReader(
-		fmt.Sprintf("DROP DATABASE IF EXISTS %s; CREATE DATABASE %s;", database, database),
-	)
-	// Send stdout and stderr to our stderr so that we see error messages from
-	// the psql process
-	cmd.Stdout = os.Stderr
-	cmd.Stderr = os.Stderr
-	return cmd.Run()
-}
-
-// TODO: dupes roomserver integration tests. Factor out.
-func canonicalJSONInput(jsonData []string) []string {
-	for i := range jsonData {
-		jsonBytes, err := gomatrixserverlib.CanonicalJSON([]byte(jsonData[i]))
-		if err != nil {
-			panic(err)
-		}
-		jsonData[i] = string(jsonBytes)
-	}
-	return jsonData
-}
-
 func createTestUser(database, username, token string) error {
 	cmd := exec.Command(
 		filepath.Join(filepath.Dir(os.Args[0]), "create-account"),
@@ -172,66 +122,32 @@ func clientEventJSONForOutputRoomEvent(outputRoomEvent string) string {
 	return string(jsonBytes)
 }
 
-// doSyncRequest does a /sync request and returns an error if it fails or doesn't
-// return the wanted string.
-func doSyncRequest(syncServerURL, want string) error {
-	cli := &http.Client{
-		Timeout: 5 * time.Second,
-	}
-	res, err := cli.Get(syncServerURL)
-	if err != nil {
-		return err
-	}
-	if res.StatusCode != 200 {
-		return fmt.Errorf("/sync returned HTTP status %d", res.StatusCode)
-	}
-	defer res.Body.Close()
-	resBytes, err := ioutil.ReadAll(res.Body)
-	if err != nil {
-		return err
-	}
-	jsonBytes, err := gomatrixserverlib.CanonicalJSON(resBytes)
-	if err != nil {
-		return err
-	}
-	if string(jsonBytes) != want {
-		return fmt.Errorf("/sync returned wrong bytes. Expected:\n%s\n\nGot:\n%s", want, string(jsonBytes))
-	}
-	return nil
-}
-
-// syncRequestUntilSuccess blocks and performs the same /sync request over and over until
-// the response returns the wanted string, where it will close the given channel and return.
-// It will keep track of the last error in `lastRequestErr`.
-func syncRequestUntilSuccess(done chan error, userID, since, want string) {
-	for {
-		sinceQuery := ""
-		if since != "" {
-			sinceQuery = "&since=" + since
-		}
-		err := doSyncRequest(
-			// low value timeout so polling with an up-to-date token returns quickly
-			"http://"+syncserverAddr+"/api/_matrix/client/r0/sync?timeout=100&access_token="+userID+sinceQuery,
-			want,
-		)
-		if err != nil {
-			setLastRequestError(err)
-			time.Sleep(1 * time.Second) // don't tightloop
-			continue
-		}
-		close(done)
-		return
-	}
-}
-
 // startSyncServer creates the database and config file needed for the sync server to run and
 // then starts the sync server. The Cmd being executed is returned. A channel is also returned,
 // which will have any termination errors sent down it, followed immediately by the channel being closed.
 func startSyncServer() (*exec.Cmd, chan error) {
-	if err := createDatabase(testDatabaseName); err != nil {
+	const configFilename = "sync-api-server-config-test.yaml"
-		panic(err)
+
+	serverArgs := []string{
+		"--config", configFilename,
+		"--listen", syncserverAddr,
 	}
 
+	databases := []string{
+		testDatabaseName,
+	}
+
+	cmd, cmdChan := test.StartServer(
+		"sync-api",
+		serverArgs,
+		"",
+		configFilename,
+		syncServerConfigFileContents,
+		postgresDatabase,
+		postgresContainerName,
+		databases,
+	)
+
 	if err := createTestUser(testDatabase, "alice", "@alice:localhost"); err != nil {
 		panic(err)
 	}
@@ -242,29 +158,7 @@ func startSyncServer() (*exec.Cmd, chan error) {
 		panic(err)
 	}
 
-	const configFileName = "sync-api-server-config-test.yaml"
+	return cmd, cmdChan
-	err := ioutil.WriteFile(configFileName, []byte(syncServerConfigFileContents), 0644)
-	if err != nil {
-		panic(err)
-	}
-
-	cmd := exec.Command(
-		filepath.Join(filepath.Dir(os.Args[0]), "dendrite-sync-api-server"),
-		"--config", configFileName,
-		"--listen", syncserverAddr,
-	)
-	cmd.Stderr = os.Stderr
-	cmd.Stdout = os.Stderr
-
-	if err := cmd.Start(); err != nil {
-		panic("failed to start sync server: " + err.Error())
-	}
-	syncServerCmdChan := make(chan error, 1)
-	go func() {
-		syncServerCmdChan <- cmd.Wait()
-		close(syncServerCmdChan)
-	}()
-	return cmd, syncServerCmdChan
 }
 
 // prepareKafka creates the topics which will be written to by the tests.
@@ -277,49 +171,24 @@ func prepareKafka() {
 
 func testSyncServer(syncServerCmdChan chan error, userID, since, want string) {
 	fmt.Printf("==TESTING== testSyncServer(%s,%s)\n", userID, since)
-	done := make(chan error, 1)
+	sinceQuery := ""
-
+	if since != "" {
-	// We need to wait for the sync server to:
+		sinceQuery = "&since=" + since
-	// - have created the tables
-	// - be listening on the given port
-	// - have consumed the kafka logs
-	// before we begin hitting it with /sync requests. We don't get told when it has done
-	// all these things, so we just continually hit /sync until it returns the right bytes.
-	// We can't even wait for the first valid 200 OK response because it's possible to race
-	// with consuming the kafka logs (so the /sync response will be missing events and
-	// therefore fail the test).
-	go syncRequestUntilSuccess(done, userID, since, canonicalJSONInput([]string{want})[0])
-
-	// wait for one of:
-	// - the test to pass (done channel is closed)
-	// - the sync server to exit with an error (error sent on syncServerCmdChan)
-	// - our test timeout to expire
-	// We don't need to clean up since the main() function handles that in the event we panic
-	var testPassed bool
-
-	select {
-	case <-time.After(timeout):
-		if testPassed {
-			break
-		}
-		fmt.Printf("==TESTING== testSyncServer(%s,%s) TIMEOUT\n", userID, since)
-		if reqErr := getLastRequestError(); reqErr != nil {
-			fmt.Println("Last /sync request error:")
-			fmt.Println(reqErr)
-		}
-		panic("dendrite-sync-api-server timed out")
-	case err := <-syncServerCmdChan:
-		if err != nil {
-			fmt.Println("=============================================================================================")
-			fmt.Println("sync server failed to run. If failing with 'pq: password authentication failed for user' try:")
-			fmt.Println("    export PGHOST=/var/run/postgresql")
-			fmt.Println("=============================================================================================")
-			panic(err)
-		}
-	case <-done:
-		testPassed = true
-		fmt.Printf("==TESTING== testSyncServer(%s,%s) PASSED\n", userID, since)
 	}
+	req, err := http.NewRequest(
+		"GET",
+		"http://"+syncserverAddr+"/api/_matrix/client/r0/sync?timeout=100&access_token="+userID+sinceQuery,
+		nil,
+	)
+	if err != nil {
+		panic(err)
+	}
+	testReq := &test.Request{
+		Req:              req,
+		WantedStatusCode: 200,
+		WantedBody:       test.CanonicalJSONInput([]string{want})[0],
+	}
+	testReq.Run("sync-api", timeout, syncServerCmdChan)
 }
 
 func writeToRoomServerLog(indexes ...int) {
@@ -327,7 +196,7 @@ func writeToRoomServerLog(indexes ...int) {
 	for _, i := range indexes {
 		roomEvents = append(roomEvents, outputRoomEventTestData[i])
 	}
-	if err := exe.WriteToTopic(inputTopic, canonicalJSONInput(roomEvents)); err != nil {
+	if err := exe.WriteToTopic(inputTopic, test.CanonicalJSONInput(roomEvents)); err != nil {
 		panic(err)
 	}
 }

src/github.com/matrix-org/dendrite/common/test/client.go

@@ -0,0 +1,163 @@
+// Copyright 2017 Vector Creations Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package test
+
+import (
+	"crypto/tls"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"net/http"
+	"sync"
+	"time"
+
+	"github.com/matrix-org/gomatrixserverlib"
+)
+
+// Request contains the information necessary to issue a request and test its result
+type Request struct {
+	Req              *http.Request
+	WantedBody       string
+	WantedStatusCode int
+	LastErr          *LastRequestErr
+}
+
+// LastRequestErr is a synchronized error wrapper
+// Useful for obtaining the last error from a set of requests
+type LastRequestErr struct {
+	sync.Mutex
+	Err error
+}
+
+// Set sets the error
+func (r *LastRequestErr) Set(err error) {
+	r.Lock()
+	defer r.Unlock()
+	r.Err = err
+}
+
+// Get gets the error
+func (r *LastRequestErr) Get() error {
+	r.Lock()
+	defer r.Unlock()
+	return r.Err
+}
+
+// CanonicalJSONInput canonicalises a slice of JSON strings
+// Useful for test input
+func CanonicalJSONInput(jsonData []string) []string {
+	for i := range jsonData {
+		jsonBytes, err := gomatrixserverlib.CanonicalJSON([]byte(jsonData[i]))
+		if err != nil && err != io.EOF {
+			panic(err)
+		}
+		jsonData[i] = string(jsonBytes)
+	}
+	return jsonData
+}
+
+// Do issues a request and checks the status code and body of the response
+func (r *Request) Do() error {
+	client := &http.Client{
+		Timeout: 5 * time.Second,
+		Transport: &http.Transport{
+			TLSClientConfig: &tls.Config{
+				InsecureSkipVerify: true,
+			},
+		},
+	}
+	res, err := client.Do(r.Req)
+	if err != nil {
+		return err
+	}
+	defer res.Body.Close()
+
+	if res.StatusCode != r.WantedStatusCode {
+		return fmt.Errorf("incorrect status code. Expected: %d  Got: %d", r.WantedStatusCode, res.StatusCode)
+	}
+
+	if r.WantedBody != "" {
+		resBytes, err := ioutil.ReadAll(res.Body)
+		if err != nil {
+			return err
+		}
+		jsonBytes, err := gomatrixserverlib.CanonicalJSON(resBytes)
+		if err != nil {
+			return err
+		}
+		if string(jsonBytes) != r.WantedBody {
+			return fmt.Errorf("returned wrong bytes. Expected:\n%s\n\nGot:\n%s", r.WantedBody, string(jsonBytes))
+		}
+	}
+
+	return nil
+}
+
+// DoUntilSuccess blocks and repeats the same request until the response returns the desired status code and body.
+// It then closes the given channel and returns.
+func (r *Request) DoUntilSuccess(done chan error) {
+	r.LastErr = &LastRequestErr{}
+	for {
+		if err := r.Do(); err != nil {
+			r.LastErr.Set(err)
+			time.Sleep(1 * time.Second) // don't tightloop
+			continue
+		}
+		close(done)
+		return
+	}
+}
+
+// Run repeatedly issues a request until success, error or a timeout is reached
+func (r *Request) Run(label string, timeout time.Duration, serverCmdChan chan error) {
+	fmt.Printf("==TESTING== %v (timeout: %v)\n", label, timeout)
+	done := make(chan error, 1)
+
+	// We need to wait for the server to:
+	// - have connected to the database
+	// - have created the tables
+	// - be listening on the given port
+	go r.DoUntilSuccess(done)
+
+	// wait for one of:
+	// - the test to pass (done channel is closed)
+	// - the server to exit with an error (error sent on serverCmdChan)
+	// - our test timeout to expire
+	// We don't need to clean up since the main() function handles that in the event we panic
+	var testPassed bool
+	select {
+	case <-time.After(timeout):
+		if testPassed {
+			break
+		}
+		fmt.Printf("==TESTING== %v TIMEOUT\n", label)
+		if reqErr := r.LastErr.Get(); reqErr != nil {
+			fmt.Println("Last /sync request error:")
+			fmt.Println(reqErr)
+		}
+		panic(fmt.Sprintf("%v server timed out", label))
+	case err := <-serverCmdChan:
+		if err != nil {
+			fmt.Println("=============================================================================================")
+			fmt.Printf("%v server failed to run. If failing with 'pq: password authentication failed for user' try:", label)
+			fmt.Println("    export PGHOST=/var/run/postgresql")
+			fmt.Println("=============================================================================================")
+			panic(err)
+		}
+	case <-done:
+		testPassed = true
+		fmt.Printf("==TESTING== %v PASSED\n", label)
+	}
+}

src/github.com/matrix-org/dendrite/common/test/server.go

@@ -0,0 +1,116 @@
+// Copyright 2017 Vector Creations Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package test
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+)
+
+// Defaulting allows assignment of string variables with a fallback default value
+// Useful for use with os.Getenv() for example
+func Defaulting(value, defaultValue string) string {
+	if value == "" {
+		value = defaultValue
+	}
+	return value
+}
+
+// CreateDatabase creates a new database, dropping it first if it exists
+func CreateDatabase(command string, args []string, database string) error {
+	cmd := exec.Command(command, args...)
+	cmd.Stdin = strings.NewReader(
+		fmt.Sprintf("DROP DATABASE IF EXISTS %s; CREATE DATABASE %s;", database, database),
+	)
+	// Send stdout and stderr to our stderr so that we see error messages from
+	// the psql process
+	cmd.Stdout = os.Stderr
+	cmd.Stderr = os.Stderr
+	return cmd.Run()
+}
+
+// CreateBackgroundCommand creates an executable command
+// The Cmd being executed is returned. A channel is also returned,
+// which will have any termination errors sent down it, followed immediately by the channel being closed.
+func CreateBackgroundCommand(command string, args []string) (*exec.Cmd, chan error) {
+	cmd := exec.Command(command, args...)
+	cmd.Stderr = os.Stderr
+	cmd.Stdout = os.Stderr
+
+	if err := cmd.Start(); err != nil {
+		panic("failed to start server: " + err.Error())
+	}
+	cmdChan := make(chan error, 1)
+	go func() {
+		cmdChan <- cmd.Wait()
+		close(cmdChan)
+	}()
+	return cmd, cmdChan
+}
+
+// StartServer creates the database and config file needed for the server to run and
+// then starts the server. The Cmd being executed is returned. A channel is also returned,
+// which will have any termination errors sent down it, followed immediately by the channel being closed.
+// If postgresContainerName is not an empty string, psql will be run from inside that container. If it is
+// an empty string, psql will be assumed to be in PATH.
+func StartServer(serverType string, serverArgs []string, suffix, configFilename, configFileContents, postgresDatabase, postgresContainerName string, databases []string) (*exec.Cmd, chan error) {
+	if len(databases) > 0 {
+		var dbCmd string
+		var dbArgs []string
+		if postgresContainerName == "" {
+			dbCmd = "psql"
+			dbArgs = []string{postgresDatabase}
+		} else {
+			dbCmd = "docker"
+			dbArgs = []string{
+				"exec", "-i", postgresContainerName, "psql", "-U", "postgres", postgresDatabase,
+			}
+		}
+		for _, database := range databases {
+			if err := CreateDatabase(dbCmd, dbArgs, database); err != nil {
+				panic(err)
+			}
+		}
+	}
+
+	if configFilename != "" {
+		if err := ioutil.WriteFile(configFilename, []byte(configFileContents), 0644); err != nil {
+			panic(err)
+		}
+	}
+
+	return CreateBackgroundCommand(
+		filepath.Join(filepath.Dir(os.Args[0]), "dendrite-"+serverType+"-server"),
+		serverArgs,
+	)
+}
+
+// StartProxy creates a reverse proxy
+func StartProxy(bindAddr, syncAddr, clientAddr, mediaAddr string) (*exec.Cmd, chan error) {
+	proxyArgs := []string{
+		"--bind-address", bindAddr,
+		"--sync-api-server-url", syncAddr,
+		"--client-api-server-url", clientAddr,
+		"--media-api-server-url", mediaAddr,
+	}
+	return CreateBackgroundCommand(
+		filepath.Join(filepath.Dir(os.Args[0]), "client-api-proxy"),
+		proxyArgs,
+	)
+}

src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer.go

@@ -17,10 +17,12 @@ package thumbnailer
 import (
 	"fmt"
 	"math"
+	"os"
 	"path/filepath"
 	"sync"
 
 	log "github.com/Sirupsen/logrus"
+	"github.com/matrix-org/dendrite/mediaapi/storage"
 	"github.com/matrix-org/dendrite/mediaapi/types"
 )
 
@@ -131,6 +133,24 @@ func broadcastGeneration(dst types.Path, activeThumbnailGeneration *types.Active
 	delete(activeThumbnailGeneration.PathToResult, string(dst))
 }
 
+func isThumbnailExists(dst types.Path, config types.ThumbnailSize, mediaMetadata *types.MediaMetadata, db *storage.Database, logger *log.Entry) (bool, error) {
+	thumbnailMetadata, err := db.GetThumbnail(mediaMetadata.MediaID, mediaMetadata.Origin, config.Width, config.Height, config.ResizeMethod)
+	if err != nil {
+		logger.Error("Failed to query database for thumbnail.")
+		return false, err
+	}
+	if thumbnailMetadata != nil {
+		return true, nil
+	}
+	// Note: The double-negative is intentional as os.IsExist(err) != !os.IsNotExist(err).
+	// The functions are error checkers to be used in different cases.
+	if _, err = os.Stat(string(dst)); !os.IsNotExist(err) {
+		// Thumbnail exists
+		return true, nil
+	}
+	return false, nil
+}
+
 // init with worst values
 func newThumbnailFitness() thumbnailFitness {
 	return thumbnailFitness{

src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer_bimg.go

@@ -17,7 +17,6 @@
 package thumbnailer
 
 import (
-	"fmt"
 	"os"
 	"time"
 
@@ -34,9 +33,10 @@ func GenerateThumbnails(src types.Path, configs []types.ThumbnailSize, mediaMeta
 		logger.WithError(err).WithField("src", src).Error("Failed to read src file")
 		return false, err
 	}
+	img := bimg.NewImage(buffer)
 	for _, config := range configs {
 		// Note: createThumbnail does locking based on activeThumbnailGeneration
-		busy, err = createThumbnail(src, buffer, config, mediaMetadata, activeThumbnailGeneration, maxThumbnailGenerators, db, logger)
+		busy, err = createThumbnail(src, img, config, mediaMetadata, activeThumbnailGeneration, maxThumbnailGenerators, db, logger)
 		if err != nil {
 			logger.WithError(err).WithField("src", src).Error("Failed to generate thumbnails")
 			return false, err
@@ -57,8 +57,9 @@ func GenerateThumbnail(src types.Path, config types.ThumbnailSize, mediaMetadata
 		}).Error("Failed to read src file")
 		return false, err
 	}
+	img := bimg.NewImage(buffer)
 	// Note: createThumbnail does locking based on activeThumbnailGeneration
-	busy, err = createThumbnail(src, buffer, config, mediaMetadata, activeThumbnailGeneration, maxThumbnailGenerators, db, logger)
+	busy, err = createThumbnail(src, img, config, mediaMetadata, activeThumbnailGeneration, maxThumbnailGenerators, db, logger)
 	if err != nil {
 		logger.WithError(err).WithFields(log.Fields{
 			"src": src,
@@ -73,13 +74,18 @@ func GenerateThumbnail(src types.Path, config types.ThumbnailSize, mediaMetadata
 
 // createThumbnail checks if the thumbnail exists, and if not, generates it
 // Thumbnail generation is only done once for each non-existing thumbnail.
-func createThumbnail(src types.Path, buffer []byte, config types.ThumbnailSize, mediaMetadata *types.MediaMetadata, activeThumbnailGeneration *types.ActiveThumbnailGeneration, maxThumbnailGenerators int, db *storage.Database, logger *log.Entry) (busy bool, errorReturn error) {
+func createThumbnail(src types.Path, img *bimg.Image, config types.ThumbnailSize, mediaMetadata *types.MediaMetadata, activeThumbnailGeneration *types.ActiveThumbnailGeneration, maxThumbnailGenerators int, db *storage.Database, logger *log.Entry) (busy bool, errorReturn error) {
 	logger = logger.WithFields(log.Fields{
 		"Width":        config.Width,
 		"Height":       config.Height,
 		"ResizeMethod": config.ResizeMethod,
 	})
 
+	// Check if request is larger than original
+	if isLargerThanOriginal(config, img) {
+		return false, nil
+	}
+
 	dst := GetThumbnailPath(src, config)
 
 	// Note: getActiveThumbnailGeneration uses mutexes and conditions from activeThumbnailGeneration
@@ -104,30 +110,13 @@ func createThumbnail(src types.Path, buffer []byte, config types.ThumbnailSize,
 		}()
 	}
 
-	// Check if the thumbnail exists.
+	exists, err := isThumbnailExists(dst, config, mediaMetadata, db, logger)
-	thumbnailMetadata, err := db.GetThumbnail(mediaMetadata.MediaID, mediaMetadata.Origin, config.Width, config.Height, config.ResizeMethod)
+	if err != nil || exists {
-	if err != nil {
-		logger.Error("Failed to query database for thumbnail.")
 		return false, err
 	}
-	if thumbnailMetadata != nil {
-		return false, nil
-	}
-	// Note: The double-negative is intentional as os.IsExist(err) != !os.IsNotExist(err).
-	// The functions are error checkers to be used in different cases.
-	if _, err = os.Stat(string(dst)); !os.IsNotExist(err) {
-		// Thumbnail exists
-		return false, nil
-	}
-
-	if isActive == false {
-		// Note: This should not happen, but we check just in case.
-		logger.Error("Failed to stat file but this is not the active thumbnail generator. This should not happen.")
-		return false, fmt.Errorf("Not active thumbnail generator. Stat error: %q", err)
-	}
 
 	start := time.Now()
-	width, height, err := resize(dst, buffer, config.Width, config.Height, config.ResizeMethod == "crop", logger)
+	width, height, err := resize(dst, img, config.Width, config.Height, config.ResizeMethod == "crop", logger)
 	if err != nil {
 		return false, err
 	}
@@ -142,7 +131,7 @@ func createThumbnail(src types.Path, buffer []byte, config types.ThumbnailSize,
 		return false, err
 	}
 
-	thumbnailMetadata = &types.ThumbnailMetadata{
+	thumbnailMetadata := &types.ThumbnailMetadata{
 		MediaMetadata: &types.MediaMetadata{
 			MediaID: mediaMetadata.MediaID,
 			Origin:  mediaMetadata.Origin,
@@ -151,8 +140,8 @@ func createThumbnail(src types.Path, buffer []byte, config types.ThumbnailSize,
 			FileSizeBytes: types.FileSizeBytes(stat.Size()),
 		},
 		ThumbnailSize: types.ThumbnailSize{
-			Width:        width,
+			Width:        config.Width,
-			Height:       height,
+			Height:       config.Height,
 			ResizeMethod: config.ResizeMethod,
 		},
 	}
@@ -169,12 +158,18 @@ func createThumbnail(src types.Path, buffer []byte, config types.ThumbnailSize,
 	return false, nil
 }
 
+func isLargerThanOriginal(config types.ThumbnailSize, img *bimg.Image) bool {
+	imgSize, err := img.Size()
+	if err == nil && config.Width >= imgSize.Width && config.Height >= imgSize.Height {
+		return true
+	}
+	return false
+}
+
 // resize scales an image to fit within the provided width and height
 // If the source aspect ratio is different to the target dimensions, one edge will be smaller than requested
 // If crop is set to true, the image will be scaled to fill the width and height with any excess being cropped off
-func resize(dst types.Path, buffer []byte, w, h int, crop bool, logger *log.Entry) (int, int, error) {
+func resize(dst types.Path, inImage *bimg.Image, w, h int, crop bool, logger *log.Entry) (int, int, error) {
-	inImage := bimg.NewImage(buffer)
-
 	inSize, err := inImage.Size()
 	if err != nil {
 		return -1, -1, err

src/github.com/matrix-org/dendrite/mediaapi/thumbnailer/thumbnailer_nfnt.go

@@ -17,7 +17,6 @@
 package thumbnailer
 
 import (
-	"fmt"
 	"image"
 	"image/draw"
 	// Imported for gif codec
@@ -114,6 +113,11 @@ func createThumbnail(src types.Path, img image.Image, config types.ThumbnailSize
 		"ResizeMethod": config.ResizeMethod,
 	})
 
+	// Check if request is larger than original
+	if config.Width >= img.Bounds().Dx() && config.Height >= img.Bounds().Dy() {
+		return false, nil
+	}
+
 	dst := GetThumbnailPath(src, config)
 
 	// Note: getActiveThumbnailGeneration uses mutexes and conditions from activeThumbnailGeneration
@@ -138,27 +142,10 @@ func createThumbnail(src types.Path, img image.Image, config types.ThumbnailSize
 		}()
 	}
 
-	// Check if the thumbnail exists.
+	exists, err := isThumbnailExists(dst, config, mediaMetadata, db, logger)
-	thumbnailMetadata, err := db.GetThumbnail(mediaMetadata.MediaID, mediaMetadata.Origin, config.Width, config.Height, config.ResizeMethod)
+	if err != nil || exists {
-	if err != nil {
-		logger.Error("Failed to query database for thumbnail.")
 		return false, err
 	}
-	if thumbnailMetadata != nil {
-		return false, nil
-	}
-	// Note: The double-negative is intentional as os.IsExist(err) != !os.IsNotExist(err).
-	// The functions are error checkers to be used in different cases.
-	if _, err = os.Stat(string(dst)); !os.IsNotExist(err) {
-		// Thumbnail exists
-		return false, nil
-	}
-
-	if isActive == false {
-		// Note: This should not happen, but we check just in case.
-		logger.Error("Failed to stat file but this is not the active thumbnail generator. This should not happen.")
-		return false, fmt.Errorf("Not active thumbnail generator. Stat error: %q", err)
-	}
 
 	start := time.Now()
 	width, height, err := adjustSize(dst, img, config.Width, config.Height, config.ResizeMethod == "crop", logger)
@@ -176,7 +163,7 @@ func createThumbnail(src types.Path, img image.Image, config types.ThumbnailSize
 		return false, err
 	}
 
-	thumbnailMetadata = &types.ThumbnailMetadata{
+	thumbnailMetadata := &types.ThumbnailMetadata{
 		MediaMetadata: &types.MediaMetadata{
 			MediaID: mediaMetadata.MediaID,
 			Origin:  mediaMetadata.Origin,
@@ -185,8 +172,8 @@ func createThumbnail(src types.Path, img image.Image, config types.ThumbnailSize
 			FileSizeBytes: types.FileSizeBytes(stat.Size()),
 		},
 		ThumbnailSize: types.ThumbnailSize{
-			Width:        width,
+			Width:        config.Width,
-			Height:       height,
+			Height:       config.Height,
 			ResizeMethod: config.ResizeMethod,
 		},
 	}

travis-test.sh

@@ -8,6 +8,9 @@ gb build github.com/matrix-org/dendrite/cmd/roomserver-integration-tests
 gb build github.com/matrix-org/dendrite/cmd/dendrite-sync-api-server
 gb build github.com/matrix-org/dendrite/cmd/syncserver-integration-tests
 gb build github.com/matrix-org/dendrite/cmd/create-account
+gb build github.com/matrix-org/dendrite/cmd/dendrite-media-api-server
+gb build github.com/matrix-org/dendrite/cmd/mediaapi-integration-tests
+gb build github.com/matrix-org/dendrite/cmd/client-api-proxy
 
 # Run the pre commit hooks
 ./hooks/pre-commit
@@ -15,3 +18,4 @@ gb build github.com/matrix-org/dendrite/cmd/create-account
 # Run the integration tests
 bin/roomserver-integration-tests
 bin/syncserver-integration-tests
+bin/mediaapi-integration-tests