-
Notifications
You must be signed in to change notification settings - Fork 65
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
perf: improve perf inside search #2663
Closed
Closed
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
3a27c1b
feat(deps): Upgrade cozy-bar to be able to call onSelect function
trollepierre f1e5acf
feat(deps): Upgrade cozy-client to be able to findAll in FileCollection
trollepierre 1133fb9
feat(search): improve search by querying fewer data
trollepierre 8d4abc3
chore(bundlemon): Upgrade maxSize of qualification migration
trollepierre 298411d
fix(search): use find method where args are for find
trollepierre ba47d2e
fix(search): note needs metadata to be considered as note
trollepierre 4dcbc83
fix(search): notes function can not be postMessage as a function
trollepierre db97874
fix(search): isNote should not check all metadata, missing while fetc…
trollepierre 1cd1efc
fixup: with cozy-bar upgrade
trollepierre File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,9 @@ | |
|
||
## ✨ Features | ||
|
||
* Upgrade [email protected] to be able to findAll in FileCollection | ||
* Upgrade [email protected] to be able to call onSelect function | ||
* Improve speed of search suggestion by querying fewer data | ||
* Update cozy-stack-client and cozy-pouch-link to sync with cozy-client version | ||
* Update cozy-ui | ||
- Modify Viewers to handle [68.0.0 BC](https://github.com/cozy/cozy-ui/releases/tag/v68.0.0) | ||
|
@@ -12,7 +15,7 @@ | |
|
||
* Improve cozy-bar implementation to fix UI bugs in Amirale | ||
* Fix navigation through mobile Flagship on Note creation and opening | ||
* Remove unused contacts permissions on Photos | ||
* Remove unused contacts permissions on Photos | ||
|
||
## 🔧 Tech | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
112 changes: 112 additions & 0 deletions
112
src/drive/web/modules/services/components/SuggestionProvider.spec.jsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
import React from 'react' | ||
import { render, waitFor } from '@testing-library/react' | ||
import SuggestionProvider from './SuggestionProvider' | ||
import { dummyFile, dummyNote } from 'test/dummies/dummyFile' | ||
|
||
const parentFolder = dummyFile({ _id: 'id-file' }) | ||
const folder = dummyFile({ dir_id: 'id-file' }) | ||
const note = dummyNote({ | ||
dir_id: 'id-file', | ||
name: 'name.cozy-note' | ||
}) | ||
const mockFindAll = jest.fn().mockReturnValue([parentFolder, folder, note]) | ||
const mockClient = { | ||
collection: jest.fn().mockReturnValue({ findAll: mockFindAll }) | ||
} | ||
const mockIntentAttributesClient = 'intent-attributes-client' | ||
|
||
jest.mock('cozy-client', () => ({ | ||
...jest.requireActual('cozy-client'), | ||
withClient: Component => () => { | ||
const intent = { | ||
_id: 'id_intent', | ||
attributes: { client: mockIntentAttributesClient } | ||
} | ||
return <Component client={mockClient} intent={intent}></Component> | ||
} | ||
})) | ||
jest.mock('./iconContext', () => ({ getIconUrl: () => 'iconUrl' })) | ||
|
||
describe('SuggestionProvider', () => { | ||
let events = {} | ||
let event | ||
|
||
beforeEach(() => { | ||
window.addEventListener = jest.fn((event, callback) => { | ||
events[event] = callback | ||
}) | ||
window.parent.postMessage = jest.fn() | ||
event = { | ||
origin: mockIntentAttributesClient, | ||
data: { query: 'name', id: 'id' } | ||
} | ||
}) | ||
|
||
it('should query all files to display fuzzy suggestion', () => { | ||
// Given | ||
render(<SuggestionProvider />) | ||
|
||
// When | ||
events.message(event) | ||
|
||
// Then | ||
expect(mockClient.collection).toHaveBeenCalledWith('io.cozy.files') | ||
expect(mockFindAll).toHaveBeenCalledWith(null, { | ||
fields: [ | ||
'_id', | ||
'trashed', | ||
'dir_id', | ||
'name', | ||
'path', | ||
'type', | ||
'mime', | ||
'metadata.title', | ||
'metadata.version' | ||
], | ||
indexedFields: ['_id'], | ||
limit: 1000, | ||
partialFilter: { | ||
_id: { $ne: 'io.cozy.files.trash-dir' }, | ||
path: { $or: [{ $exists: false }, { $regex: '^(?!/.cozy_trash)' }] }, | ||
trashed: { $or: [{ $exists: false }, { $eq: false }] } | ||
} | ||
}) | ||
}) | ||
|
||
it('should provide onSelect with open url when file is not a note + and function when it is a note', async () => { | ||
// Given | ||
render(<SuggestionProvider />) | ||
|
||
// When | ||
events.message(event) | ||
|
||
// Then | ||
await waitFor(() => { | ||
expect(window.parent.postMessage).toHaveBeenCalledWith( | ||
{ | ||
id: 'id', | ||
suggestions: [ | ||
{ | ||
icon: 'iconUrl', | ||
id: 'id-file', | ||
onSelect: 'open:http://localhost/#/folder/id-file', | ||
subtitle: '/path', | ||
term: 'name', | ||
title: 'name' | ||
}, | ||
{ | ||
icon: 'iconUrl', | ||
id: 'id-file', | ||
onSelect: 'id_note:id-file', | ||
subtitle: '/path', | ||
term: 'name.cozy-note', | ||
title: 'name.cozy-note' | ||
} | ||
], | ||
type: 'intent-id_intent:data' | ||
}, | ||
'intent-attributes-client' | ||
) | ||
}) | ||
}) | ||
}) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we can benefits from http2 multiplexing, I'm wondering if we should not make two requests:
We'll have the almost the same number of round trips (1 or 2) but we'll have a few of them parallelized. @paultranvan what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that, most of the time, the number of files will be order of magnitudes higher than the number of folders. So, the folder query might end up sooner than the files one, which is good, as we want to display the folders first. The downside is that we have now 2 indexes to compute instead of one, which might slow down the search when we make a query with non-indexed docs.
I think what you propose is still relevant either way, as it should allow to get partial (i.e. folders) results faster and I think faster user response is more important than the overall performance.
That being said, I also think paginated mango queries is sub-optimal for this use-case :/ I look forward for view-base approach and/or pouchdb exploration