From f3aaa9a95cc2b61f1f255472d7014d08faa66561 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 5 Dec 2017 17:46:33 +0100 Subject: Fix client search --- server/controllers/api/videos/index.ts | 22 +++---- server/initializers/constants.ts | 6 -- server/middlewares/index.ts | 1 - server/middlewares/search.ts | 14 ---- server/middlewares/validators/videos.ts | 5 +- server/models/video/video-interface.ts | 1 - server/models/video/video.ts | 47 ++++++-------- server/tests/api/single-server.ts | 112 +++++++++++++++++--------------- server/tests/utils/videos.ts | 20 +++--- 9 files changed, 98 insertions(+), 130 deletions(-) delete mode 100644 server/middlewares/search.ts (limited to 'server') diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts index e2798830e..2b70d535e 100644 --- a/server/controllers/api/videos/index.ts +++ b/server/controllers/api/videos/index.ts @@ -26,7 +26,6 @@ import { authenticate, paginationValidator, setPagination, - setVideosSearch, setVideosSort, videosAddValidator, videosGetValidator, @@ -84,6 +83,14 @@ videosRouter.get('/', setPagination, asyncMiddleware(listVideos) ) +videosRouter.get('/search', + videosSearchValidator, + paginationValidator, + videosSortValidator, + setVideosSort, + setPagination, + asyncMiddleware(searchVideos) +) videosRouter.put('/:id', authenticate, asyncMiddleware(videosUpdateValidator), @@ -115,16 +122,6 @@ videosRouter.delete('/:id', asyncMiddleware(removeVideoRetryWrapper) ) -videosRouter.get('/search/:value', - videosSearchValidator, - paginationValidator, - videosSortValidator, - setVideosSort, - setPagination, - setVideosSearch, - asyncMiddleware(searchVideos) -) - // --------------------------------------------------------------------------- export { @@ -378,8 +375,7 @@ async function removeVideo (req: express.Request, res: express.Response) { async function searchVideos (req: express.Request, res: express.Response, next: express.NextFunction) { const resultList = await db.Video.searchAndPopulateAccountAndServerAndTags( - req.params.value, - req.query.field, + req.query.search, req.query.start, req.query.count, req.query.sort diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index 144a4edbf..3e083fd92 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -24,11 +24,6 @@ const API_VERSION = 'v1' // Number of results by default for the pagination const PAGINATION_COUNT_DEFAULT = 15 -// Sortable columns per schema -const SEARCHABLE_COLUMNS = { - VIDEOS: [ 'name', 'magnetUri', 'host', 'account', 'tags' ] -} - // Sortable columns per schema const SORTABLE_COLUMNS = { USERS: [ 'id', 'username', 'createdAt' ], @@ -361,7 +356,6 @@ export { REMOTE_SCHEME, FOLLOW_STATES, AVATARS_DIR, - SEARCHABLE_COLUMNS, SERVER_ACCOUNT_NAME, PRIVATE_RSA_KEY_SIZE, SORTABLE_COLUMNS, diff --git a/server/middlewares/index.ts b/server/middlewares/index.ts index aafcad2d9..0cef26953 100644 --- a/server/middlewares/index.ts +++ b/server/middlewares/index.ts @@ -4,6 +4,5 @@ export * from './async' export * from './oauth' export * from './pagination' export * from './servers' -export * from './search' export * from './sort' export * from './user-right' diff --git a/server/middlewares/search.ts b/server/middlewares/search.ts deleted file mode 100644 index 6fe83d25b..000000000 --- a/server/middlewares/search.ts +++ /dev/null @@ -1,14 +0,0 @@ -import 'express-validator' -import * as express from 'express' - -function setVideosSearch (req: express.Request, res: express.Response, next: express.NextFunction) { - if (!req.query.field) req.query.field = 'name' - - return next() -} - -// --------------------------------------------------------------------------- - -export { - setVideosSearch -} diff --git a/server/middlewares/validators/videos.ts b/server/middlewares/validators/videos.ts index f21680aa0..ee2ac50c8 100644 --- a/server/middlewares/validators/videos.ts +++ b/server/middlewares/validators/videos.ts @@ -18,7 +18,7 @@ import { } from '../../helpers/custom-validators/videos' import { getDurationFromVideoFile } from '../../helpers/ffmpeg-utils' import { logger } from '../../helpers/logger' -import { CONSTRAINTS_FIELDS, SEARCHABLE_COLUMNS } from '../../initializers' +import { CONSTRAINTS_FIELDS } from '../../initializers' import { database as db } from '../../initializers/database' import { UserInstance } from '../../models/account/user-interface' import { VideoInstance } from '../../models/video/video-interface' @@ -172,8 +172,7 @@ const videosRemoveValidator = [ ] const videosSearchValidator = [ - param('value').not().isEmpty().withMessage('Should have a valid search'), - query('field').optional().isIn(SEARCHABLE_COLUMNS.VIDEOS).withMessage('Should have correct searchable column'), + query('search').not().isEmpty().withMessage('Should have a valid search'), (req: express.Request, res: express.Response, next: express.NextFunction) => { logger.debug('Checking videosSearch parameters', { parameters: req.params }) diff --git a/server/models/video/video-interface.ts b/server/models/video/video-interface.ts index be140de86..2a63350af 100644 --- a/server/models/video/video-interface.ts +++ b/server/models/video/video-interface.ts @@ -50,7 +50,6 @@ export namespace VideoMethods { export type ListUserVideosForApi = (userId: number, start: number, count: number, sort: string) => Bluebird< ResultList > export type SearchAndPopulateAccountAndServerAndTags = ( value: string, - field: string, start: number, count: number, sort: string diff --git a/server/models/video/video.ts b/server/models/video/video.ts index f3469c1de..4dce8e2fc 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -1070,7 +1070,7 @@ loadByUUIDAndPopulateAccountAndServerAndTags = function (uuid: string) { return Video.findOne(options) } -searchAndPopulateAccountAndServerAndTags = function (value: string, field: string, start: number, count: number, sort: string) { +searchAndPopulateAccountAndServerAndTags = function (value: string, start: number, count: number, sort: string) { const serverInclude: Sequelize.IncludeOptions = { model: Video['sequelize'].models.Server, required: false @@ -1099,33 +1099,24 @@ searchAndPopulateAccountAndServerAndTags = function (value: string, field: strin order: [ getSort(sort), [ Video['sequelize'].models.Tag, 'name', 'ASC' ] ] } - if (field === 'tags') { - const escapedValue = Video['sequelize'].escape('%' + value + '%') - query.where['id'][Sequelize.Op.in] = Video['sequelize'].literal( - `(SELECT "VideoTags"."videoId" - FROM "Tags" - INNER JOIN "VideoTags" ON "Tags"."id" = "VideoTags"."tagId" - WHERE name ILIKE ${escapedValue} - )` - ) - } else if (field === 'host') { - // FIXME: Include our server? (not stored in the database) - serverInclude.where = { - host: { - [Sequelize.Op.iLike]: '%' + value + '%' - } - } - serverInclude.required = true - } else if (field === 'account') { - accountInclude.where = { - name: { - [Sequelize.Op.iLike]: '%' + value + '%' - } - } - } else { - query.where[field] = { - [Sequelize.Op.iLike]: '%' + value + '%' - } + // TODO: search on tags too + // const escapedValue = Video['sequelize'].escape('%' + value + '%') + // query.where['id'][Sequelize.Op.in] = Video['sequelize'].literal( + // `(SELECT "VideoTags"."videoId" + // FROM "Tags" + // INNER JOIN "VideoTags" ON "Tags"."id" = "VideoTags"."tagId" + // WHERE name ILIKE ${escapedValue} + // )` + // ) + + // TODO: search on account too + // accountInclude.where = { + // name: { + // [Sequelize.Op.iLike]: '%' + value + '%' + // } + // } + query.where['name'] = { + [Sequelize.Op.iLike]: '%' + value + '%' } query.include = [ diff --git a/server/tests/api/single-server.ts b/server/tests/api/single-server.ts index 041d13225..fe192d391 100644 --- a/server/tests/api/single-server.ts +++ b/server/tests/api/single-server.ts @@ -225,7 +225,7 @@ describe('Test a single server', function () { expect(video.views).to.equal(3) }) - it('Should search the video by name by default', async function () { + it('Should search the video by name', async function () { const res = await searchVideo(server.url, 'my') expect(res.body.total).to.equal(1) @@ -279,35 +279,36 @@ describe('Test a single server', function () { // }) // }) - it('Should search the video by tag', async function () { - const res = await searchVideo(server.url, 'tag1', 'tags') - - expect(res.body.total).to.equal(1) - expect(res.body.data).to.be.an('array') - expect(res.body.data.length).to.equal(1) - - const video = res.body.data[0] - expect(video.name).to.equal('my super name') - expect(video.category).to.equal(2) - expect(video.categoryLabel).to.equal('Films') - expect(video.licence).to.equal(6) - expect(video.licenceLabel).to.equal('Attribution - Non Commercial - No Derivatives') - expect(video.language).to.equal(3) - expect(video.languageLabel).to.equal('Mandarin') - expect(video.nsfw).to.be.ok - expect(video.description).to.equal('my super description') - expect(video.serverHost).to.equal('localhost:9001') - expect(video.account).to.equal('root') - expect(video.isLocal).to.be.true - expect(video.tags).to.deep.equal([ 'tag1', 'tag2', 'tag3' ]) - expect(dateIsValid(video.createdAt)).to.be.true - expect(dateIsValid(video.updatedAt)).to.be.true - - const test = await testVideoImage(server.url, 'video_short.webm', video.thumbnailPath) - expect(test).to.equal(true) - }) + // Not implemented yet + // it('Should search the video by tag', async function () { + // const res = await searchVideo(server.url, 'tag1') + // + // expect(res.body.total).to.equal(1) + // expect(res.body.data).to.be.an('array') + // expect(res.body.data.length).to.equal(1) + // + // const video = res.body.data[0] + // expect(video.name).to.equal('my super name') + // expect(video.category).to.equal(2) + // expect(video.categoryLabel).to.equal('Films') + // expect(video.licence).to.equal(6) + // expect(video.licenceLabel).to.equal('Attribution - Non Commercial - No Derivatives') + // expect(video.language).to.equal(3) + // expect(video.languageLabel).to.equal('Mandarin') + // expect(video.nsfw).to.be.ok + // expect(video.description).to.equal('my super description') + // expect(video.serverHost).to.equal('localhost:9001') + // expect(video.account).to.equal('root') + // expect(video.isLocal).to.be.true + // expect(video.tags).to.deep.equal([ 'tag1', 'tag2', 'tag3' ]) + // expect(dateIsValid(video.createdAt)).to.be.true + // expect(dateIsValid(video.updatedAt)).to.be.true + // + // const test = await testVideoImage(server.url, 'video_short.webm', video.thumbnailPath) + // expect(test).to.equal(true) + // }) - it('Should not find a search by name by default', async function () { + it('Should not find a search by name', async function () { const res = await searchVideo(server.url, 'hello') expect(res.body.total).to.equal(0) @@ -315,21 +316,23 @@ describe('Test a single server', function () { expect(res.body.data.length).to.equal(0) }) - it('Should not find a search by author', async function () { - const res = await searchVideo(server.url, 'hello', 'account') - - expect(res.body.total).to.equal(0) - expect(res.body.data).to.be.an('array') - expect(res.body.data.length).to.equal(0) - }) - - it('Should not find a search by tag', async function () { - const res = await searchVideo(server.url, 'hello', 'tags') - - expect(res.body.total).to.equal(0) - expect(res.body.data).to.be.an('array') - expect(res.body.data.length).to.equal(0) - }) + // Not implemented yet + // it('Should not find a search by author', async function () { + // const res = await searchVideo(server.url, 'hello') + // + // expect(res.body.total).to.equal(0) + // expect(res.body.data).to.be.an('array') + // expect(res.body.data.length).to.equal(0) + // }) + // + // Not implemented yet + // it('Should not find a search by tag', async function () { + // const res = await searchVideo(server.url, 'hello') + // + // expect(res.body.total).to.equal(0) + // expect(res.body.data).to.be.an('array') + // expect(res.body.data.length).to.equal(0) + // }) it('Should remove the video', async function () { await removeVideo(server.url, server.accessToken, videoId) @@ -443,7 +446,7 @@ describe('Test a single server', function () { }) it('Should search the first video', async function () { - const res = await searchVideoWithPagination(server.url, 'webm', 'name', 0, 1, 'name') + const res = await searchVideoWithPagination(server.url, 'webm', 0, 1, 'name') const videos = res.body.data expect(res.body.total).to.equal(4) @@ -452,7 +455,7 @@ describe('Test a single server', function () { }) it('Should search the last two videos', async function () { - const res = await searchVideoWithPagination(server.url, 'webm', 'name', 2, 2, 'name') + const res = await searchVideoWithPagination(server.url, 'webm', 2, 2, 'name') const videos = res.body.data expect(res.body.total).to.equal(4) @@ -462,20 +465,21 @@ describe('Test a single server', function () { }) it('Should search all the webm videos', async function () { - const res = await searchVideoWithPagination(server.url, 'webm', 'name', 0, 15) + const res = await searchVideoWithPagination(server.url, 'webm', 0, 15) const videos = res.body.data expect(res.body.total).to.equal(4) expect(videos.length).to.equal(4) }) - it('Should search all the root author videos', async function () { - const res = await searchVideoWithPagination(server.url, 'root', 'account', 0, 15) - - const videos = res.body.data - expect(res.body.total).to.equal(6) - expect(videos.length).to.equal(6) - }) + // Not implemented yet + // it('Should search all the root author videos', async function () { + // const res = await searchVideoWithPagination(server.url, 'root', 0, 15) + // + // const videos = res.body.data + // expect(res.body.total).to.equal(6) + // expect(videos.length).to.equal(6) + // }) // Not implemented yet // it('Should search all the 9001 port videos', async function () { diff --git a/server/tests/utils/videos.ts b/server/tests/utils/videos.ts index 73a9f1a0a..ff7da9bb2 100644 --- a/server/tests/utils/videos.ts +++ b/server/tests/utils/videos.ts @@ -145,26 +145,25 @@ function removeVideo (url: string, token: string, id: number, expectedStatus = 2 .expect(expectedStatus) } -function searchVideo (url: string, search: string, field?: string) { +function searchVideo (url: string, search: string) { const path = '/api/v1/videos' const req = request(url) - .get(path + '/search/' + search) - .set('Accept', 'application/json') - - if (field) req.query({ field }) + .get(path + '/search') + .query({ search }) + .set('Accept', 'application/json') return req.expect(200) - .expect('Content-Type', /json/) + .expect('Content-Type', /json/) } -function searchVideoWithPagination (url: string, search: string, field: string, start: number, count: number, sort?: string) { +function searchVideoWithPagination (url: string, search: string, start: number, count: number, sort?: string) { const path = '/api/v1/videos' const req = request(url) - .get(path + '/search/' + search) + .get(path + '/search') .query({ start }) + .query({ search }) .query({ count }) - .query({ field }) if (sort) req.query({ sort }) @@ -177,7 +176,8 @@ function searchVideoWithSort (url: string, search: string, sort: string) { const path = '/api/v1/videos' return request(url) - .get(path + '/search/' + search) + .get(path + '/search') + .query({ search }) .query({ sort }) .set('Accept', 'application/json') .expect(200) -- cgit v1.2.3