Skip to content

Keys changes v2 #340

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 11, 2022
Merged

Keys changes v2 #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a76079
Add limit and offset query params to client.keys method
brunoocasali Jul 7, 2022
e12ea73
Rename variable to be explicity about key and uid
brunoocasali Jul 7, 2022
e52325a
Add coverage to missing spots in the keys resource
brunoocasali Jul 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions lib/meilisearch/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,14 @@ def fetch_raw_index(index_uid)

### KEYS

def keys
http_get '/keys'
def keys(limit: nil, offset: nil)
body = { limit: limit, offset: offset }.compact

http_get '/keys', body
end

def key(key_uid)
http_get "/keys/#{key_uid}"
def key(uid_or_key)
http_get "/keys/#{uid_or_key}"
end

def create_key(key_options)
Expand All @@ -66,15 +68,15 @@ def create_key(key_options)
http_post '/keys', body
end

def update_key(key_uid, key_options)
def update_key(uid_or_key, key_options)
body = Utils.transform_attributes(key_options)
body = body.slice('description', 'name')

http_patch "/keys/#{key_uid}", body
http_patch "/keys/#{uid_or_key}", body
end

def delete_key(key_uid)
http_delete "/keys/#{key_uid}"
def delete_key(uid_or_key)
http_delete "/keys/#{uid_or_key}"
end

### HEALTH
Expand Down
50 changes: 39 additions & 11 deletions spec/meilisearch/client/keys_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

RSpec.describe 'MeiliSearch::Client - Keys' do
context 'When managing keys' do
let(:uuid_v4) { 'c483e150-cff1-4a45-ac26-bb8eb8e01d36' }
let(:delete_docs_key_options) do
{
description: 'A new key to delete docs',
Expand Down Expand Up @@ -30,18 +31,20 @@
expect(new_key['description']).to eq('A new key to add docs')
end

it 'creates a key using snake_case' do
new_key = client.create_key(add_docs_key_options)
it 'creates a key with setting uid' do
new_key = client.create_key(add_docs_key_options.merge(uid: uuid_v4))

expect(new_key['expiresAt']).to be_nil
expect(new_key['name']).to be_nil
expect(new_key['uid']).to eq(uuid_v4)
expect(new_key['key']).to be_a(String)
expect(new_key['createdAt']).to be_a(String)
expect(new_key['updatedAt']).to be_a(String)
expect(new_key['indexes']).to eq(['*'])
expect(new_key['description']).to eq('A new key to add docs')
end

it 'gets a key' do
it 'gets a key with their key data' do
new_key = client.create_key(delete_docs_key_options)

expect(client.key(new_key['key'])['description']).to eq('A new key to delete docs')
Expand All @@ -56,7 +59,32 @@
expect(key['description']).to eq('A new key to delete docs')
end

it 'updates a key' do
it 'retrieves a list of keys' do
new_key = client.create_key(add_docs_key_options)

list = client.keys

expect(list.keys).to contain_exactly('limit', 'offset', 'results', 'total')
expect(list['results']).to eq([new_key])
expect(list['total']).to eq(1)
end

it 'paginates keys list with limit/offset' do
client.create_key(add_docs_key_options)

expect(client.keys(limit: 0, offset: 20)['results']).to be_empty
expect(client.keys(limit: 5, offset: 199)['results']).to be_empty
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I get the point of this test. Why do you set a limit of 5 but expect it to be empty?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The answer is more complex than what I'm going to say to you, but what I can say is:

If we set a "random" limit/offset combination that will probably be out of bounds and make a request Meilisearch should return an empty array as the result.

I know it sounds counter-intuitive, but in the future our idea is to remove all of these tests and replace them by mocks like this:

    it 'sends limit/offset in the request' do
       expect(a_request(:get, "www.meilisearch.com").with(query: {"limit" => 5, "offset" => 10})).
  to have_been_made
  
       client.keys(limit: 5, offset: 10)
    end

Because if you agree with me, it will be enough to guarantee the contract between the API (Meili), and the caller (SDK).

end

it 'gets a key with their uid' do
new_key = client.create_key(delete_docs_key_options.merge(uid: uuid_v4))

key = client.key(uuid_v4)

expect(key).to eq(new_key)
end

it 'updates a key with their key data' do
new_key = client.create_key(delete_docs_key_options)
new_updated_key = client.update_key(new_key['key'], indexes: ['coco'], description: 'no coco')

Expand All @@ -66,20 +94,20 @@
expect(new_updated_key['indexes']).to eq(['*'])
end

it 'updates a key using snake_case' do
new_key = client.create_key(delete_docs_key_options)
new_updated_key = client.update_key(new_key['key'], indexes: ['coco'])
it 'updates a key with their uid data' do
new_key = client.create_key(delete_docs_key_options.merge(uid: uuid_v4))
new_updated_key = client.update_key(uuid_v4, name: 'coco')

expect(new_updated_key['key']).to eq(new_key['key'])
expect(new_updated_key['description']).to eq(new_key['description'])
expect(new_updated_key['indexes']).to eq(['*'])
expect(new_updated_key['name']).to eq('coco')
end

it 'deletes a key' do
new_key = client.create_key(add_docs_key_options)
client.delete_key(new_key['key'])

expect(client.keys.filter { |k| k['key'] == new_key['key'] }).to be_empty
expect {
client.key(new_key['key'])
}.to raise_error(MeiliSearch::ApiError)
end
end
end