refactor texture uploading

app/Http/Controllers/SkinlibController.php

@@ -7,33 +7,18 @@ use App\Models\Texture;
 use App\Models\User;
 use Auth;
 use Blessing\Filter;
+use Blessing\Rejection;
+use Illuminate\Contracts\Events\Dispatcher;
 use Illuminate\Database\Eloquent\Builder;
-use Illuminate\Http\JsonResponse;
+use Illuminate\Filesystem\FilesystemAdapter;
 use Illuminate\Http\Request;
-use Option;
+use Illuminate\Support\Facades\Log;
+use Illuminate\Validation\Rule;
 use Parsedown;
 use Storage;
 
 class SkinlibController extends Controller
 {
-    /**
-     * Map error code of file uploading to human-readable text.
-     *
-     * @see http://php.net/manual/en/features.file-upload.errors.php
-     *
-     * @var array
-     */
-    public static $phpFileUploadErrors = [
-        0 => 'There is no error, the file uploaded with success',
-        1 => 'The uploaded file exceeds the upload_max_filesize directive in php.ini',
-        2 => 'The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form',
-        3 => 'The uploaded file was only partially uploaded',
-        4 => 'No file was uploaded',
-        6 => 'Missing a temporary folder',
-        7 => 'Failed to write file to disk.',
-        8 => 'A PHP extension stopped the file upload.',
-    ];
-
     public function library(Request $request)
     {
         $user = Auth::user();
@@ -180,61 +165,127 @@ class SkinlibController extends Controller
             ]);
     }
 
-    public function handleUpload(Request $request)
+    public function handleUpload(
-    {
+        Request $request,
-        $user = Auth::user();
+        Filter $filter,
-
+        Dispatcher $dispatcher
-        if (($response = $this->checkUpload($request)) instanceof JsonResponse) {
+    ) {
-            return $response;
-        }
-
         $file = $request->file('file');
-        $responses = event(new \App\Events\HashingFile($file));
+        if ($file && !$file->isValid()) {
-        if (isset($responses[0]) && is_string($responses[0])) {
+            Log::error($file->getErrorMessage());
-            return $responses[0];  // @codeCoverageIgnore
         }
 
-        $t = new Texture();
+        $data = $request->validate([
-        $t->name = $request->input('name');
+            'name' => [
-        $t->type = $request->input('type');
+                'required',
-        $t->hash = hash_file('sha256', $file);
+                option('texture_name_regexp') ? 'regex:'.option('texture_name_regexp') : 'string',
-        $t->size = ceil($request->file('file')->getSize() / 1024);
+            ],
-        $t->public = $request->input('public') == 'true';
+            'file' => 'required|mimes:png|max:'.option('max_upload_file_size'),
-        $t->uploader = $user->uid;
+            'type' => ['required', Rule::in(['steve', 'alex', 'cape'])],
+            'public' => 'required|boolean',
+        ]);
 
-        $cost = $t->size * ($t->public ? Option::get('score_per_storage') : Option::get('private_score_per_storage'));
+        $file = $filter->apply('uploaded_texture_file', $file);
-        $cost += option('score_per_closet_item');
-        $cost -= option('score_award_per_texture', 0);
 
-        if ($user->score < $cost) {
+        $name = $data['name'];
-            return json(trans('skinlib.upload.lack-score'), 7);
+        $name = $filter->apply('uploaded_texture_name', $name, [$file]);
+
+        $can = $filter->apply('can_upload_texture', true, [$file, $name]);
+        if ($can instanceof Rejection) {
+            return json($can->getReason(), 1);
         }
 
-        $repeated = Texture::where('hash', $t->hash)->where('public', true)->first();
+        $type = $data['type'];
-        if ($repeated) {
+        $size = getimagesize($file);
+        $ratio = $size[0] / $size[1];
+        if ($type == 'steve' || $type == 'alex') {
+            if ($ratio != 2 && $ratio != 1) {
+                $message = trans('skinlib.upload.invalid-size', [
+                    'type' => trans('general.skin'),
+                    'width' => $size[0],
+                    'height' => $size[1],
+                ]);
+
+                return json($message, 1);
+            }
+            if ($size[0] % 64 != 0 || $size[1] % 32 != 0) {
+                $message = trans('skinlib.upload.invalid-hd-skin', [
+                    'type' => trans('general.skin'),
+                    'width' => $size[0],
+                    'height' => $size[1],
+                ]);
+
+                return json($message, 1);
+            }
+        } elseif ($type == 'cape') {
+            if ($ratio != 2) {
+                $message = trans('skinlib.upload.invalid-size', [
+                    'type' => trans('general.cape'),
+                    'width' => $size[0],
+                    'height' => $size[1],
+                ]);
+
+                return json($message, 1);
+            }
+        }
+
+        $hash = hash_file('sha256', $file);
+        $hash = $filter->apply('uploaded_texture_hash', $hash, [$file]);
+
+        $duplicated = Texture::where('hash', $hash)->where('public', true)->first();
+        if ($duplicated) {
             // if the texture already uploaded was set to private,
             // then allow to re-upload it.
-            return json(trans('skinlib.upload.repeated'), 2, ['tid' => $repeated->tid]);
+            return json(trans('skinlib.upload.repeated'), 2, ['tid' => $duplicated->tid]);
         }
 
-        if (Storage::disk('textures')->missing($t->hash)) {
+        /** @var User */
-            Storage::disk('textures')->put($t->hash, file_get_contents($request->file('file')));
+        $user = Auth::user();
+
+        $size = ceil($file->getSize() / 1024);
+        $isPublic = is_string($data['public'])
+            ? $data['public'] === '1'
+            : $data['public'];
+        $cost = $size * (
+            $isPublic
+            ? option('score_per_storage')
+            : option('private_score_per_storage')
+        );
+        $cost += option('score_per_closet_item');
+        $cost -= option('score_award_per_texture', 0);
+        if ($user->score < $cost) {
+            return json(trans('skinlib.upload.lack-score'), 1);
         }
 
-        $t->likes++;
+        $dispatcher->dispatch('texture.uploading', [$file, $name, $hash]);
-        $t->save();
+
+        $texture = new Texture();
+        $texture->name = $name;
+        $texture->type = $type;
+        $texture->hash = $hash;
+        $texture->size = $size;
+        $texture->public = $isPublic;
+        $texture->uploader = $user->uid;
+        $texture->likes = 1;
+        $texture->save();
+
+        /** @var FilesystemAdapter */
+        $disk = Storage::disk('textures');
+        if ($disk->missing($hash)) {
+            $disk->putFile($hash, $file);
+        }
 
         $user->score -= $cost;
-        $user->closet()->attach($t->tid, ['item_name' => $t->name]);
+        $user->closet()->attach($texture->tid, ['item_name' => $name]);
         $user->save();
 
-        return json(trans('skinlib.upload.success', ['name' => $request->input('name')]), 0, [
+        $dispatcher->dispatch('texture.uploaded', [$texture, $file]);
-            'tid' => $t->tid,
+
+        return json(trans('skinlib.upload.success', ['name' => $name]), 0, [
+            'tid' => $texture->tid,
         ]);
     }
 
-    // @codeCoverageIgnore
-
     public function delete(Request $request)
     {
         $texture = Texture::find($request->tid);
@@ -355,48 +406,4 @@ class SkinlibController extends Controller
 
         return json(trans('skinlib.model.success', ['model' => $data['model']]), 0);
     }
-
-    protected function checkUpload(Request $request)
-    {
-        if ($file = $request->files->get('file')) {
-            if ($file->getError() !== UPLOAD_ERR_OK) {
-                return json(static::$phpFileUploadErrors[$file->getError()], $file->getError());
-            }
-        }
-
-        $request->validate([
-            'name' => [
-                'required',
-                option('texture_name_regexp') ? 'regex:'.option('texture_name_regexp') : 'string',
-            ],
-            'file' => 'required|max:'.option('max_upload_file_size'),
-            'public' => 'required',
-        ]);
-
-        $mime = $request->file('file')->getMimeType();
-        if ($mime != 'image/png' && $mime != 'image/x-png') {
-            return json(trans('skinlib.upload.type-error'), 1);
-        }
-
-        $type = $request->input('type');
-        $size = getimagesize($request->file('file'));
-        $ratio = $size[0] / $size[1];
-
-        if ($type == 'steve' || $type == 'alex') {
-            if ($ratio != 2 && $ratio != 1) {
-                return json(trans('skinlib.upload.invalid-size', ['type' => trans('general.skin'), 'width' => $size[0], 'height' => $size[1]]), 1);
-            }
-            if ($size[0] % 64 != 0 || $size[1] % 32 != 0) {
-                return json(trans('skinlib.upload.invalid-hd-skin', ['type' => trans('general.skin'), 'width' => $size[0], 'height' => $size[1]]), 1);
-            }
-        } elseif ($type == 'cape') {
-            if ($ratio != 2) {
-                return json(trans('skinlib.upload.invalid-size', ['type' => trans('general.cape'), 'width' => $size[0], 'height' => $size[1]]), 1);
-            }
-        } else {
-            return json(trans('general.illegal-parameters'), 1);
-        }
-    }
-
-    // @codeCoverageIgnore
 }

resources/assets/src/views/skinlib/Upload.tsx

@@ -82,7 +82,7 @@ const Upload: React.FC = () => {
     formData.append('name', name)
     formData.append('type', type)
     formData.append('file', file, file.name)
-    formData.append('public', (!isPrivate).toString())
+    formData.append('public', isPrivate ? '0' : '1')
 
     setIsUploading(true)
     const { code, message, data: { tid } = { tid: 0 } } = await fetch.post<

resources/assets/tests/views/skinlib/Upload.test.tsx

@@ -223,21 +223,24 @@ describe('upload texture', () => {
     expect(formData.get('name')).toBe('t')
     expect(formData.get('type')).toBe('steve')
     expect(formData.get('file')).toStrictEqual(file)
-    expect(formData.get('public')).toBe('true')
+    expect(formData.get('public')).toBe('1')
   })
 
   it('uploaded successfully', async () => {
     fetch.post.mockResolvedValue({ code: 0, message: 'ok', tid: 1 })
 
-    const { getByText, getByTitle } = render(<Upload />)
+    const { getByText, getByTitle, getByLabelText } = render(<Upload />)
 
     const file = new File([], 't.png', { type: 'image/png' })
     fireEvent.change(getByTitle(t('skinlib.upload.select-file')), {
       target: { files: [file] },
     })
+    fireEvent.click(getByLabelText(t('skinlib.upload.set-as-private')))
     fireEvent.click(getByText(t('skinlib.upload.button')))
 
     await waitFor(() => expect(fetch.post).toBeCalled())
+    const formData: FormData = fetch.post.mock.calls[0][1]
+    expect(formData.get('public')).toBe('0')
   })
 
   it('duplicated texture detected', async () => {

tests/HttpTest/ControllersTest/SkinlibControllerTest.php

@@ -5,10 +5,12 @@ namespace Tests;
 use App\Models\Player;
 use App\Models\Texture;
 use App\Models\User;
-use Blessing\Filter;
+use Blessing\Rejection;
 use Carbon\Carbon;
+use Illuminate\Filesystem\FilesystemAdapter;
 use Illuminate\Foundation\Testing\DatabaseTransactions;
 use Illuminate\Http\UploadedFile;
+use Illuminate\Support\Facades\Event;
 use Illuminate\Support\Facades\Storage;
 
 class SkinlibControllerTest extends TestCase
@@ -220,9 +222,21 @@ class SkinlibControllerTest extends TestCase
 
     public function testHandleUpload()
     {
-        Storage::fake('textures');
+        Event::fake();
+        /** @var FilesystemAdapter */
+        $disk = Storage::fake('textures');
+        $filter = Fakes\Filter::fake();
+        $user = factory(User::class)->create();
 
-        // Some error occurred when uploading file
+        // without file
+        $this->actingAs($user)
+            ->postJson('/skinlib/upload', [
+                'name' => 'name',
+                'type' => 'steve',
+                'public' => true,
+            ])->assertJsonValidationErrors('file');
+
+        // some error occurred when uploading file
         $file = UploadedFile::fake()->image('test.png');
         $upload = new UploadedFile(
             $file->path(),
@@ -231,31 +245,29 @@ class SkinlibControllerTest extends TestCase
             UPLOAD_ERR_NO_TMP_DIR,
             true
         );
-        $this->actingAs(factory(User::class)->create())
-            ->postJson(
-                '/skinlib/upload',
-                ['file' => $upload]
-            )->assertJson([
-                'code' => UPLOAD_ERR_NO_TMP_DIR,
-                'message' => \App\Http\Controllers\SkinlibController::$phpFileUploadErrors[UPLOAD_ERR_NO_TMP_DIR],
-            ]);
-
-        // Without `name` field
-        $this->postJson('/skinlib/upload')->assertJsonValidationErrors('name');
-
-        // Specified regular expression for texture name
-        option(['texture_name_regexp' => '/\\d+/']);
         $this->postJson('/skinlib/upload', [
-            'name' => 'abc',
+            'name' => 'name',
-        ])->assertJsonValidationErrors('name');
+            'file' => $upload,
-        option(['texture_name_regexp' => null]);
+            'type' => 'steve',
-
+            'public' => true,
-        // Without file
-        $this->postJson('/skinlib/upload', [
-            'name' => 'texture',
         ])->assertJsonValidationErrors('file');
 
-        // Too large file
+        // without `name` field
+        $this->postJson('/skinlib/upload')->assertJsonValidationErrors('name');
+
+        // specified regular expression for texture name
+        option(['texture_name_regexp' => '/\\d+/']);
+        $this->postJson('/skinlib/upload', ['name' => 'abc'])
+            ->assertJsonValidationErrors('name');
+        option(['texture_name_regexp' => null]);
+
+        // not a PNG file
+        $this->postJson('/skinlib/upload', [
+            'name' => 'texture',
+            'file' => UploadedFile::fake()->create('fake', 5),
+        ])->assertJsonValidationErrors('file');
+
+        // too large file
         option(['max_upload_file_size' => 2]);
         $upload = UploadedFile::fake()->create('large.png', 5);
         $this->postJson('/skinlib/upload', [
@@ -264,113 +276,80 @@ class SkinlibControllerTest extends TestCase
         ])->assertJsonValidationErrors('file');
         option(['max_upload_file_size' => 1024]);
 
-        // Without `public` field
+        // no texture type is specified
         $this->postJson('/skinlib/upload', [
             'name' => 'texture',
-            'file' => 'content',    // Though it is not a file, it is OK
+            'file' => $file,
+        ])->assertJsonValidationErrors('type');
+
+        // invalid texture type
+        $this->postJson('/skinlib/upload', [
+            'name' => 'texture',
+            'file' => $file,
+            'type' => 'nope',
+        ])->assertJsonValidationErrors('type');
+
+        // without `public` field
+        $this->postJson('/skinlib/upload', [
+            'name' => 'texture',
+            'file' => $file,
+            'type' => 'steve',
         ])->assertJsonValidationErrors('public');
 
-        // Not a PNG image
+        // invalid skin size
-        $this->postJson(
+        $this->postJson('/skinlib/upload', [
-            '/skinlib/upload',
+            'name' => 'texture',
-            [
+            'public' => true,
-                'name' => 'texture',
+            'type' => 'steve',
-                'public' => 'true',
+            'file' => UploadedFile::fake()->image('texture.png', 64, 30),
-                'file' => UploadedFile::fake()->create('fake', 5),
+        ])->assertJson([
-            ]
-        )->assertJson([
             'code' => 1,
-            'message' => trans('skinlib.upload.type-error'),
+            'message' => trans('skinlib.upload.invalid-size', [
+                'type' => trans('general.skin'),
+                'width' => 64,
+                'height' => 30,
+            ]),
         ]);
-
+        $this->postJson('/skinlib/upload', [
-        // No texture type is specified
+            'name' => 'texture',
-        $this->postJson(
+            'public' => true,
-            '/skinlib/upload',
+            'type' => 'alex',
-            [
+            'file' => UploadedFile::fake()->image('texture.png', 100, 50),
-                'name' => 'texture',
+        ])->assertJson([
-                'public' => 'true',
-                'file' => UploadedFile::fake()->image('texture.png', 64, 32),
-            ]
-        )->assertJson([
             'code' => 1,
-            'message' => trans('general.illegal-parameters'),
+            'message' => trans('skinlib.upload.invalid-hd-skin', [
+                'type' => trans('general.skin'),
+                'width' => 100,
+                'height' => 50,
+            ]),
         ]);
-
+        $this->postJson('/skinlib/upload', [
-        // Invalid skin size
+            'name' => 'texture',
-        $this->postJson(
+            'public' => true,
-            '/skinlib/upload',
+            'type' => 'cape',
-            [
+            'file' => UploadedFile::fake()->image('texture.png', 64, 30),
-                'name' => 'texture',
+        ])->assertJson([
-                'public' => 'true',
-                'type' => 'steve',
-                'file' => UploadedFile::fake()->image('texture.png', 64, 30),
-            ]
-        )->assertJson([
             'code' => 1,
-            'message' => trans(
+            'message' => trans('skinlib.upload.invalid-size', [
-                'skinlib.upload.invalid-size',
+                'type' => trans('general.cape'),
-                [
+                'width' => 64,
-                    'type' => trans('general.skin'),
+                'height' => 30,
-                    'width' => 64,
+            ]),
-                    'height' => 30,
-                ]
-            ),
-        ]);
-        $this->postJson(
-            '/skinlib/upload',
-            [
-                'name' => 'texture',
-                'public' => 'true',
-                'type' => 'alex',
-                'file' => UploadedFile::fake()->image('texture.png', 100, 50),
-            ]
-        )->assertJson([
-            'code' => 1,
-            'message' => trans(
-                'skinlib.upload.invalid-hd-skin',
-                [
-                    'type' => trans('general.skin'),
-                    'width' => 100,
-                    'height' => 50,
-                ]
-            ),
-        ]);
-        $this->postJson(
-            '/skinlib/upload',
-            [
-                'name' => 'texture',
-                'public' => 'true',
-                'type' => 'cape',
-                'file' => UploadedFile::fake()->image('texture.png', 64, 30),
-            ]
-        )->assertJson([
-            'code' => 1,
-            'message' => trans(
-                'skinlib.upload.invalid-size',
-                [
-                    'type' => trans('general.cape'),
-                    'width' => 64,
-                    'height' => 30,
-                ]
-            ),
         ]);
 
         $upload = UploadedFile::fake()->image('texture.png', 64, 32);
 
-        // Score is not enough
+        // score is not enough
         $user = factory(User::class)->create(['score' => 0]);
         $this->actingAs($user)
-            ->postJson(
+            ->postJson('/skinlib/upload', [
-                '/skinlib/upload',
+                'name' => 'texture',
-                [
+                'public' => true,
-                    'name' => 'texture',
+                'type' => 'steve',
-                    'public' => 'true',
+                'file' => $upload,
-                    'type' => 'steve',
+            ])
-                    'file' => $upload,
-                ]
-            )
             ->assertJson([
-                'code' => 7,
+                'code' => 1,
                 'message' => trans('skinlib.upload.lack-score'),
             ]);
 
@@ -381,60 +360,104 @@ class SkinlibControllerTest extends TestCase
             '/skinlib/upload',
             [
                 'name' => 'texture',
-                'public' => 'false',  // Private texture cost more scores
+                'public' => false,
                 'type' => 'steve',
                 'file' => $upload,
             ]
         )->assertJson([
-            'code' => 7,
+            'code' => 1,
             'message' => trans('skinlib.upload.lack-score'),
         ]);
 
-        // Success
+        // success
         option(['score_award_per_texture' => 2]);
-        $response = $this->postJson(
+        $this->postJson('/skinlib/upload', [
-            '/skinlib/upload',
+            'name' => 'texture',
-            [
+            'public' => true,
-                'name' => 'texture',
+            'type' => 'steve',
-                'public' => 'true',    // Public texture
+            'file' => $upload,
-                'type' => 'steve',
+        ])->assertJson([
-                'file' => $upload,
-            ]
-        );
-        $t = Texture::where('name', 'texture')->first();
-        $response->assertJson([
             'code' => 0,
             'message' => trans('skinlib.upload.success', ['name' => 'texture']),
-            'data' => ['tid' => $t->tid],
         ]);
-        Storage::disk('textures')->assertExists($t->hash);
+        $texture = Texture::where('name', 'texture')->first();
-        $user = User::find($user->uid);
+        $disk->assertExists($texture->hash);
+        $user->refresh();
         $this->assertEquals(2, $user->score);
-        $this->assertEquals('texture', $t->name);
+        $this->assertEquals('texture', $texture->name);
-        $this->assertEquals('steve', $t->type);
+        $this->assertEquals('steve', $texture->type);
-        $this->assertEquals(1, $t->likes);
+        $this->assertEquals(1, $texture->likes);
-        $this->assertEquals(1, $t->size);
+        $this->assertEquals(1, $texture->size);
-        $this->assertEquals($user->uid, $t->uploader);
+        $this->assertEquals($user->uid, $texture->uploader);
-        $this->assertTrue($t->public);
+        $this->assertTrue($texture->public);
+        $filter->assertApplied('uploaded_texture_file', function ($file) {
+            $this->assertInstanceOf(UploadedFile::class, $file);
 
-        // Upload a duplicated texture
+            return true;
+        });
+        $filter->assertApplied('uploaded_texture_name', function ($name) {
+            $this->assertEquals('texture', $name);
+
+            return true;
+        });
+        $filter->assertApplied(
+            'uploaded_texture_hash',
+            function ($hash, $file) use ($texture) {
+                $this->assertEquals($texture->hash, $hash);
+                $this->assertInstanceOf(UploadedFile::class, $file);
+
+                return true;
+            }
+        );
+        Event::assertDispatched(
+            'texture.uploading',
+            function ($eventName, $payload) use ($texture) {
+                $this->assertInstanceOf(UploadedFile::class, $payload[0]);
+                $this->assertEquals($texture->name, $payload[1]);
+                $this->assertEquals($texture->hash, $payload[2]);
+
+                return true;
+            }
+        );
+        Event::assertDispatched(
+            'texture.uploaded',
+            function ($eventName, $payload) use ($texture) {
+                $this->assertTrue($texture->is($payload[0]));
+                $this->assertInstanceOf(UploadedFile::class, $payload[1]);
+
+                return true;
+            }
+        );
+
+        // upload a duplicated texture
         $user = factory(User::class)->create();
         $this->actingAs($user)
-            ->postJson(
+            ->postJson('/skinlib/upload', [
-                '/skinlib/upload',
+                'name' => 'texture',
-                [
+                'public' => true,
-                    'name' => 'texture',
+                'type' => 'steve',
-                    'public' => 'true',
+                'file' => $upload,
-                    'type' => 'steve',
+            ])->assertJson([
-                    'file' => $upload,
-                ]
-            )->assertJson([
                 'code' => 2,
                 'message' => trans('skinlib.upload.repeated'),
-                'data' => ['tid' => $t->tid],
+                'data' => ['tid' => $texture->tid],
             ]);
 
-        unlink(storage_path('framework/testing/disks/textures/'.$t->hash));
+        // rejected
+        $filter->add('can_upload_texture', function ($can, $file, $name) {
+            $this->assertInstanceOf(UploadedFile::class, $file);
+            $this->assertEquals('texture', $name);
+
+            return new Rejection('rejected');
+        });
+        $this->postJson('/skinlib/upload', [
+                'name' => 'texture',
+                'public' => true,
+                'type' => 'steve',
+                'file' => $upload,
+            ])->assertJson(['code' => 1, 'message' => 'rejected']);
+
+        $disk->delete($texture->hash);
     }
 
     public function testDelete()