Skip to content
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

#6 Search Module #20

Merged
merged 5 commits into from
Dec 5, 2015
Merged

#6 Search Module #20

merged 5 commits into from
Dec 5, 2015

Conversation

hkulekci
Copy link
Member

@hkulekci hkulekci commented Nov 2, 2015

#6 Creating search module

  • Configuration file created
  • AbstractSearchService added to Search module
  • AbstractIndexService and AbstractQueryService added to Search module
  • Example UserIndexService and UserQueryService added
  • ElasticsearchClient Service and Factory added

@hkulekci hkulekci changed the title [WIP] Search Module [WIP] #6 Search Module Nov 2, 2015
@hkulekci
Copy link
Member Author

I create a basic implementation for Search module. I will try to create a user index and user search method. Right now, user search method create a match_all query on elasticsearch and index method, which is in AbstractIndexService class, directly indexes data to ES. We may consider adding a specific index() and search() methods for index and search services respectively.

@edigu
Copy link
Collaborator

edigu commented Nov 20, 2015

Looks good to me. Can you rebase on master to resolve conflicts? I think search layer can accept/return DTO's while indexing & searching data in future. It would be nice improvement.

*/
namespace Search\Exception;

class MissingDataException extends \InvalidArgumentException
Copy link
Collaborator

Choose a reason for hiding this comment

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

MissingArgumentException?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to create concerete exceptions for some situations. For example, I used that exception on AbstractIndexService#24. Maybe we can use same exception on AbstractSearchService#56 or AbstractSearchService#87.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand. In this case id is a requirement, part of the data for a valid elastica document, validating incoming data before trying to create documents sounds good. However when searching, its a parameter (argument) which may exists or not. I think separating exceptions to three distinct classes would more appropriate :MissingArgumentException, InvalidDocumentException and InvalidArgumentException

 - Configuration file created
 - AbstractSearchService added to Search module
 - AbstractIndexService and AbstractQueryService added to Search module
 - Example UserIndexService and UserQueryService added
 - ElasticsearchClient Service and Factory added
@hkulekci hkulekci changed the title [WIP] #6 Search Module #6 Search Module Dec 5, 2015
hkulekci added a commit that referenced this pull request Dec 5, 2015
@hkulekci hkulekci merged commit 8f1d84b into biberlabs:master Dec 5, 2015
@hkulekci hkulekci deleted the feature/search-module branch December 5, 2015 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants