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

How to distinguish controllers that return an object or a list? #7

Open
php-coder opened this issue Jul 15, 2020 · 4 comments
Open
Labels
decision Requires making a decision

Comments

@php-coder
Copy link
Owner

php-coder commented Jul 15, 2020

At this moment we have get verb that accepts a SELECT query. Some endpoints return a single object (GET /category/1) while others return a list (GET /categories). In addition, when we expect an object, code should return 404 error for an empty result, while for a list, it should return an empty list. Example:

- path: /categories/:id
  get:  SELECT id,name FROM categories WHERE id = :id
- path: /categories
  get:  SELECT id,name FROM categories

How we can distinguish such cases so we can generate different code?

@php-coder php-coder added the decision Requires making a decision label Jul 15, 2020
@php-coder php-coder changed the title How to distinguish controllers that return a single object or a list? How to distinguish controllers that return an object or a list? Jul 15, 2020
@php-coder
Copy link
Owner Author

php-coder commented Jul 15, 2020

Option 1: use rules for guessing what is expected to be returned

For example, SELECT COUNT(*) FROM ... will always return a single object (as well as usage of other aggregated functions. If a query has LIMIT 1 or WHERE id = :id it also returns a single row.

Pros:

  • user doesn't need to think as everything works out-of-the-box

Cons:

  • user should know (or learn) our conventions
  • we'll have to maintain a lot of rules
  • there always will be a case when we missed user's expectations anyway (what if id field isn't unique or what if my query has WHERE slug = ? and slug field is unique but we don't take it into account)

@php-coder
Copy link
Owner Author

php-coder commented Jul 15, 2020

Option 2: let user to specify what is expected in the result

  • 2.1: introduce a separate field for that
- path: /categories/:id
  get:  SELECT ...
  return: object     <-- NEW

- path: /categories
  get:  ...
  return: list       <-- NEW

The name also could be return_type: object|list, single_result: true|false, list: true|false, return_one: true|false

  • 2.2: introduce a separate field with a list of hints
- path: /categories/:id
  get:  SELECT ...

- path: /categories
  get:  ...
    hints:         <-- NEW
    - "result:list"

The name also could be returns:list, multiple_results, etc

  • 2.3: (re-)use a method field. We can introduce get_list in addition to get:
- path: /categories/:id
  get:  ...
- path: /categories
  get_list: ...   <-- NEW

Here we have many names: get/get_many or get_one/get_many or get_one/get_list or find_one/find_many.

Pros:

  • explicit better than implicit
  • 2.2: a general-purpose field can be useful in future to handle other cases (like a custom response code)
  • 2.3: it doesn't make a config more complex because get_list is short enough

Cons:

  • 2.1: a new field makes a configuration more verbose and less elegant
  • 2.2-2.3: with a new verbs we diverge from a well-known HTTP verbs and goes into kind-of-DSL area (especially with find_*)

@theshamuel
Copy link

Hey @php-coder, I suppose, that option 2.1 is more suitable for fixing your issue. Because but no matter as you wish you cannot avoid adding new field, so it had better been more clear for understanding for user. That is why modify get_ and creating one common hints looks a little bit weird for me. If you use field return: list/object I think it is really obvious what for it.

@php-coder
Copy link
Owner Author

After discussions I started to think about introducing a single field. I had something like this in mind:

input:
  ...
output:
  multiple: true      <-- look here
  type: PersonDto
  fields:
    id: int
    name: string

Because I can't predict future and I can't suggest an option that I would be happy with, I've decided to go with the simplest and fastest approach right now. I will change API when I have more use-cases. In 44c061d I introduced get_list verb.

I leave the issue open because I anticipate that the current solution will be revised in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decision Requires making a decision
Projects
None yet
Development

No branches or pull requests

2 participants