Skip to content
This repository has been archived by the owner on Oct 17, 2020. It is now read-only.

Emit errors through Search API #1000

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Prev Previous commit
Next Next commit
Add error channel for sending search errors
- Also remove extra logging statement in searchShortLink
  • Loading branch information
Coteh committed Sep 13, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 09599a10badafe6e3f0458dee120526e1daf720e
2 changes: 1 addition & 1 deletion backend/app/adapter/routing/handle/search.go
Original file line number Diff line number Diff line change
@@ -174,7 +174,7 @@ func (f *Filter) resourcesString() []string {
return resources
}

func newSearchResponse(result search.ResourceResult) SearchResponse {
func newSearchResponse(result search.Result) SearchResponse {
shortLinks := make([]ShortLink, len(result.ShortLinks))
for i := 0; i < len(result.ShortLinks); i++ {
shortLinks[i] = newShortLink(result.ShortLinks[i])
57 changes: 25 additions & 32 deletions backend/app/usecase/search/search.go
Original file line number Diff line number Diff line change
@@ -21,12 +21,6 @@ type Search struct {

// Result represents the result of a search query.
type Result struct {
Resources ResourceResult
Err error
}

// ResourceResult represents the resources obtained from a search query.
type ResourceResult struct {
ShortLinks []entity.ShortLink
Users []entity.User
}
@@ -46,9 +40,11 @@ func (e ErrUserNotProvided) Error() string {
}

// Search finds resources based on specified criteria.
func (s Search) Search(query Query, filter Filter) (ResourceResult, error) {
func (s Search) Search(query Query, filter Filter) (Result, error) {
resultCh := make(chan Result)
errCh := make(chan error)
defer close(resultCh)
defer close(errCh)

orders := toOrders(filter.orders)

@@ -58,60 +54,57 @@ func (s Search) Search(query Query, filter Filter) (ResourceResult, error) {
result, err := s.searchResource(filter.resources[i], orders[i], query, filter)
if err != nil {
s.logger.Error(err)
resultCh <- Result{
Resources: ResourceResult{},
Err: err,
}
resultCh <- Result{}
errCh <- err
return
}
resultCh <- Result{
Resources: result,
Err: nil,
}
resultCh <- result
errCh <- nil
}()
}

timeout := time.After(s.timeout)
var results []ResourceResult
var results []Result
var resultErr error
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed based on the previous comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is still needed, because we would like to return only the first error that occurred. What do you think?

for i := 0; i < len(filter.resources); i++ {
select {
case result := <-resultCh:
results = append(results, result)
case <-timeout:
return mergeResults(results), nil
}
select {
case err := <-errCh:
// Only return the first error encountered
if resultErr == nil {
resultErr = result.Err
resultErr = err
}
results = append(results, result.Resources)
case <-timeout:
return mergeResults(results), nil
}
}

return mergeResults(results), resultErr
}

func (s Search) searchResource(resource Resource, orderBy order.Order, query Query, filter Filter) (ResourceResult, error) {
func (s Search) searchResource(resource Resource, orderBy order.Order, query Query, filter Filter) (Result, error) {
switch resource {
case ShortLink:
return s.searchShortLink(query, orderBy, filter)
case User:
return s.searchUser(query, orderBy, filter)
default:
return ResourceResult{}, ErrUnknownResource{}
return Result{}, ErrUnknownResource{}
}
}

// TODO(issue#866): Simplify searchShortLink function
func (s Search) searchShortLink(query Query, orderBy order.Order, filter Filter) (ResourceResult, error) {
func (s Search) searchShortLink(query Query, orderBy order.Order, filter Filter) (Result, error) {
if query.User == nil {
err := ErrUserNotProvided{}
s.logger.Error(err)
return ResourceResult{}, err
return Result{}, ErrUserNotProvided{}
}

shortLinks, err := s.getShortLinkByUser(*query.User)
if err != nil {
return ResourceResult{}, err
return Result{}, err
}

var matchedAliasByAll, matchedAliasByAny, matchedLongLinkByAll, matchedLongLinkByAny []entity.ShortLink
@@ -145,14 +138,14 @@ func (s Search) searchShortLink(query Query, orderBy order.Order, filter Filter)

filteredShortLinks := filterShortLinks(mergedShortLinks, filter)

return ResourceResult{
return Result{
ShortLinks: filteredShortLinks,
Users: nil,
}, nil
}

func (s Search) searchUser(query Query, orderBy order.Order, filter Filter) (ResourceResult, error) {
return ResourceResult{}, nil
func (s Search) searchUser(query Query, orderBy order.Order, filter Filter) (Result, error) {
return Result{}, nil
}

func (s Search) getShortLinkByUser(user entity.User) ([]entity.ShortLink, error) {
@@ -201,8 +194,8 @@ func toOrders(ordersBy []order.By) []order.Order {
return orders
}

func mergeResults(results []ResourceResult) ResourceResult {
var mergedResult ResourceResult
func mergeResults(results []Result) Result {
var mergedResult Result

for _, result := range results {
mergedResult.ShortLinks = append(mergedResult.ShortLinks, result.ShortLinks...)