Skip to content

Upgrade Indexes-related methods to the v0.25.0 #120

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
brunoocasali merged 1 commit into from
Jan 17, 2022
Merged

Upgrade Indexes-related methods to the v0.25.0 #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion example/lib/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ void main() async {
var client = MeiliSearchClient('http://127.0.0.1:7700', 'masterKey');

// An index where books are stored.
var index = await client.createIndex('books');
await client.createIndex('books');
var index = await client.getIndex('books');

var documents = [
{'book_id': 123, 'title': 'Pride and Prejudice'},
Expand Down
8 changes: 5 additions & 3 deletions lib/src/client.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'package:meilisearch/src/pending_update.dart';

import 'http_request.dart';
import 'index.dart';
import 'client_impl.dart';
Expand Down Expand Up @@ -30,13 +32,13 @@ abstract class MeiliSearchClient {

/// Create a new index by given [uid] and optional [primaryKey] parameter.
/// Throws an error if index is already exists.
Future<MeiliSearchIndex> createIndex(String uid, {String primaryKey});
Future<PendingUpdate> createIndex(String uid, {String primaryKey});

/// Delete the index by matching [uid].
Future<void> deleteIndex(String uid);
Future<PendingUpdate> deleteIndex(String uid);

/// Update the primary Key of the index by matching [uid].
Future<void> updateIndex(String uid, String primaryKey);
Future<PendingUpdate> updateIndex(String uid, String primaryKey);

/// Return health of the MeiliSearch server.
/// Throws an error if containing details if MeiliSearch can't process your request.
Expand Down
29 changes: 19 additions & 10 deletions lib/src/client_impl.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import 'package:dio/dio.dart';
import 'package:meilisearch/src/client_task_impl.dart';
import 'package:meilisearch/src/pending_update.dart';

import 'http_request.dart';
import 'http_request_impl.dart';

Expand Down Expand Up @@ -26,19 +30,22 @@ class MeiliSearchClientImpl implements MeiliSearchClient {
return new MeiliSearchIndexImpl(this, uid);
}

Future<PendingUpdate> _update(Future<Response> future) async {
final response = await future;

return ClientTaskImpl.fromMap(this, response.data);
}

@override
Future<MeiliSearchIndex> createIndex(String uid, {String? primaryKey}) async {
Future<PendingUpdate> createIndex(String uid, {String? primaryKey}) async {
final data = <String, dynamic>{
'uid': uid,
if (primaryKey != null) 'primaryKey': primaryKey,
};
data.removeWhere((k, v) => v == null);
final response = await http.postMethod<Map<String, dynamic>>(
'/indexes',
data: data,
);

return MeiliSearchIndexImpl.fromMap(this, response.data!);
return await _update(
http.postMethod<Map<String, dynamic>>('/indexes', data: data));
}

@override
Expand All @@ -60,15 +67,17 @@ class MeiliSearchClientImpl implements MeiliSearchClient {
}

@override
Future<void> deleteIndex(String uid) async {
Future<PendingUpdate> deleteIndex(String uid) async {
final index = this.index(uid);
await index.delete();

return await index.delete();
}

@override
Future<void> updateIndex(String uid, String primaryKey) async {
Future<PendingUpdate> updateIndex(String uid, String primaryKey) async {
final index = this.index(uid);
await index.update(primaryKey: primaryKey);

return index.update(primaryKey: primaryKey);
}

@override
Expand Down
24 changes: 24 additions & 0 deletions lib/src/client_task_impl.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import 'package:meilisearch/src/client_impl.dart';
import 'package:meilisearch/src/update_status.dart';

import 'pending_update.dart';

class ClientTaskImpl implements PendingUpdate {
final int updateId;
final MeiliSearchClientImpl client;

ClientTaskImpl(this.client, this.updateId);

factory ClientTaskImpl.fromMap(
MeiliSearchClientImpl client,
Map<String, dynamic> map,
) =>
ClientTaskImpl(client, map['uid'] as int);

@override
Future<UpdateStatus> getStatus() async {
Copy link
Contributor

@alallema alallema Jan 17, 2022

Choose a reason for hiding this comment

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

Did this method will become getTask ?

Copy link
Contributor

@alallema alallema Jan 17, 2022

Choose a reason for hiding this comment

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

I thought the name will be changed in the next PR but I think this getStatus method and getUpdateStatus should be renamed in getTask and getTasks.

Copy link
Member Author

@brunoocasali brunoocasali Jan 17, 2022

Choose a reason for hiding this comment

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

Yeah, I just tried to reduce the scope of changes haha, but I'll do that, thanks for the reminder ;)

final response = await client.http.getMethod(('/tasks/${this.updateId}'));

return UpdateStatus.fromMap(response.data);
}
}
4 changes: 2 additions & 2 deletions lib/src/index.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ abstract class MeiliSearchIndex {
set primaryKey(String? primaryKey);

/// Update the primary Key of the index.
Future<void> update({String primaryKey});
Future<PendingUpdate> update({String primaryKey});

/// Delete the index.
Future<void> delete();
Future<PendingUpdate> delete();

/// Search for documents matching a specific query in the index.
Future<SearchResult> search<T>(
Expand Down
17 changes: 7 additions & 10 deletions lib/src/index_impl.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,18 @@ class MeiliSearchIndexImpl implements MeiliSearchIndex {
//

@override
Future<void> update({String? primaryKey}) async {
Future<PendingUpdateImpl> update({String? primaryKey}) async {
final data = <String, dynamic>{
'primaryKey': primaryKey,
};
data.removeWhere((k, v) => v == null);
final response = await http.putMethod('/indexes/$uid', data: data);

_primaryKey = response.data['primaryKey'] as String?;
_createdAt = DateTime.parse(response.data['createdAt'] as String);
_updatedAt = DateTime.parse(response.data['updatedAt'] as String);
return await _update(http.putMethod('/indexes/$uid', data: data));
}

@override
Future<void> delete() async {
await http.deleteMethod('/indexes/$uid');
Future<PendingUpdateImpl> delete() async {
return await _update(http.deleteMethod('/indexes/$uid'));
}

@override
Expand Down Expand Up @@ -430,15 +427,15 @@ class MeiliSearchIndexImpl implements MeiliSearchIndex {
///

Future<List<UpdateStatus>?> getAllUpdateStatus() async {
final response = await http.getMethod('/indexes/$uid/updates');
final response = await http.getMethod('/indexes/$uid/tasks');

return (response.data as List)
return (response.data['results'] as List)
.map((update) => UpdateStatus.fromMap(update))
.toList();
}

Future<UpdateStatus> getUpdateStatus(int updateId) async {
final response = await http.getMethod(('/indexes/$uid/updates/$updateId'));
final response = await http.getMethod(('/tasks/$updateId'));
Copy link
Contributor

@alallema alallema Jan 17, 2022

Choose a reason for hiding this comment

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

I think this method should call '/indexes/$uid/tasks/$updateId' if this is index.getTask()

Copy link
Member Author

@brunoocasali brunoocasali Jan 17, 2022

Choose a reason for hiding this comment

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

Yeah, this was one of my biggest nightmares when I was implementing this...

Since we have this extension defined here test/utils/client.dart:73
It causes problems for me when I updated this test:

   test('Delete an existing index', () async {
      final uid = randomUid();
      await client.createIndex(uid).waitFor();

      final index = await client.getIndex(uid);
      await index.delete().waitFor(); // waitFor() => this causes the issue...

      expect(client.getIndex(uid), throwsA(isA<MeiliSearchApiException>()));
    });

The problem here is because .waitFor(); Will loop until the task is over, but since the extension tries to get the currentStatus based on each implementation, it will make a request to indexes/:uid/tasks/:task_uid, but if you look closely... For the task to be completed, the index should be removed before the task is done, so imagine this timeline:

  • request to the server to delete the index
  • server add that request to the tasks queue
  • #try-1 to check if the delete is done but is not.
  • #try-2 to check if the delete is done but is not.
  • server get the task be processed
  • #try-3 to check if the delete is done but is not, currently is being processed.
  • server finished the task, the index is gone.
  • #try-4 to check if the delete is done and it is but we have a problem (💥) why?
    • /indexes/:uid/tasks is not accessible anymore because the :uid (meaning, the index was removed remember?)
    • that’s why we receive an error instead of a successful response telling us that the task is finished (btw the error is ).
    {
  "message": "Index `my_index` not found.",
  "code": "index_not_found",
  "type": "invalid_request",
  "link": "https://docs.meilisearch.com/errors#index_not_found"
}

By the way, yes I could skip waitFor() here in this test, but in this case, I could not ensure the current behavior or have the exception when I try to access the index. (if you have some idea please tell me) - I thought about doing a mock, but idk if this will be possible.

That is the current situation here, I hope you understand if not, don’t hesitate to make a comment ;)

FYI @curquiza (maybe this is an API design situation that I need to be aware of)

Copy link
Contributor

@alallema alallema Jan 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I had the same issue on another SDK, I rewrite the test to get the Task from the client. Indeed if the index doesn't exist you can't retrieve the task from it.
Also, the waitForTask function must be accessible by the client and the index too.

Copy link
Member Author

@brunoocasali brunoocasali Jan 17, 2022

Choose a reason for hiding this comment

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

I had the same issue on another SDK, I rewrite the test to get the Task from the client.

Could you link here to this change you made?

Also, the waitForTask function must be accessible by the client and the index too.

*(on the dart integration you will have the waitFor for all the methods that respond with a Task object (formerly PendingUpdate)), so my waitForTask is already working for these both situations... (but I'm not sure if this is what you mean)

Copy link
Member

@curquiza curquiza Jan 18, 2022

Choose a reason for hiding this comment

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

Hello, sorry I read this kind of quickly, so I hope I correctly understood the situation.

The index.waitForTask and client.waitForTask methods should definitely NOT call the /indexes/:uid/tasks/:uid method, but the /tasks/:uid one, for the reason you described @brunoocasali, even for the method you called from the Index instance
We anticipated this issue, see this comment meilisearch/integration-guides#157 (comment), le second point.

Sorry again if this answer does not solve your problem @brunoocasali, tell me!


return UpdateStatus.fromMap(response.data);
}
Expand Down
65 changes: 37 additions & 28 deletions test/indexes_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,58 +17,67 @@ void main() {

test('Create index with right UID with a primary', () async {
final uid = randomUid();
await client.createIndex(uid, primaryKey: 'myId');
await client.createIndex(uid, primaryKey: 'myId').waitFor();
final index = await client.getIndex(uid);
expect(index.uid, uid);
expect(index.primaryKey, 'myId');
});

test('Update an index where the primary has not been set', () async {
var index = await client.createIndex(randomUid());
await index.update(primaryKey: 'nextId');
final uid = randomUid();
await client.createIndex(uid).waitFor();

var index = await client.getIndex(uid);
await index.update(primaryKey: 'nextId').waitFor();
index = await client.getIndex(uid);

expect(index.primaryKey, equals('nextId'));
});

test('Update an index from the client where the primary has not been set',
() async {
final uid = randomUid();
await client.createIndex(uid);
await client.updateIndex(uid, 'nextId');
await client.createIndex(uid).waitFor();
await client.updateIndex(uid, 'nextId').waitFor();
final index = await client.getIndex(uid);
expect(index.primaryKey, equals('nextId'));
});

test('Delete an existing index', () async {
final uid = randomUid();
var index = await client.createIndex(uid);
await index.delete();
await client.createIndex(uid).waitFor();

final index = await client.getIndex(uid);
await index.delete().waitFor();

expect(client.getIndex(uid), throwsA(isA<MeiliSearchApiException>()));
});

test('Delete index with right UID from the client', () async {
final uid = randomUid();
await client.createIndex(uid);
await client.deleteIndex(uid);
await client.createIndex(uid).waitFor();
await client.deleteIndex(uid).waitFor();

expect(client.getIndex(uid), throwsA(isA<MeiliSearchApiException>()));
});

test('Get an existing index', () async {
final uid = randomUid();
await client.createIndex(uid);
await client.createIndex(uid).waitFor();
var index = await client.getIndex(uid);
expect(index.uid, uid);
expect(index.primaryKey, null);
});

test('Get a non-existing index', () async {
test('throws exception with a non-existing index', () async {
expect(client.getIndex(randomUid('loremIpsum')),
throwsA(isA<MeiliSearchApiException>()));
});

test('Get all indexes', () async {
await client.createIndex(randomUid());
await client.createIndex(randomUid());
await client.createIndex(randomUid());
await client.createIndex(randomUid()).waitFor();
await client.createIndex(randomUid()).waitFor();
await client.createIndex(randomUid()).waitFor();
var indexes = await client.getIndexes();
expect(indexes.length, 3);
});
Expand All @@ -82,12 +91,12 @@ void main() {

test('Create index object with UID and add Document', () async {
final uid = randomUid();
var indexObject = client.index(uid);
final response = await indexObject.addDocuments([
var index = client.index(uid);
final response = await index.addDocuments([
{'book_id': 123, 'title': 'Pride and Prejudice'}
]).waitFor();
expect(response.status, 'processed');
final index = await client.getIndex(uid);
expect(response.status, 'succeeded');
index = await client.getIndex(uid);
expect(index.uid, uid);
});

Expand All @@ -105,27 +114,24 @@ void main() {
{'book_id': 123, 'title': 'Pride and Prejudice'},
{'book_id': 456, 'title': 'The Martin'},
]).waitFor();
expect(response.status, 'processed');
expect(response.status, 'succeeded');
final stats = await index.getStats();
expect(stats.numberOfDocuments, 2);
});

test('Getting all update statuses default', () async {
final index = await client.createIndex(randomUid());
final response = await index.getAllUpdateStatus();
expect(response, []);
});

test('Getting all update statuses', () async {
final index = client.index(randomUid());
final uid = randomUid();
await client.createIndex(uid).waitFor();
final index = await client.getIndex(uid);

await index.addDocuments([
{'book_id': 1234, 'title': 'Pride and Prejudice'}
]);
await index.addDocuments([
{'book_id': 5678}
]);
final update_status = await index.getAllUpdateStatus();
expect(update_status!.length, 2);
expect(update_status!.length, 3);
});

test('Getting update status', () async {
Expand All @@ -146,7 +152,10 @@ void main() {
});

test('Getting non-existant update status', () async {
final index = await client.createIndex(randomUid());
final uid = randomUid();
await client.createIndex(uid).waitFor();
final index = await client.getIndex(uid);

expect(() async => await index.getUpdateStatus(9999),
throwsA(isA<MeiliSearchApiException>()));
});
Expand Down