6
\$\begingroup\$

My company has a REST API, and one design issue that has come up a few times is that we use null to indicate "get everything".

For example /api/books?genre=romance will get all romance books, but /api/books will return all books from all genres.

The current technique used in the code is something like this

public ResponseEntity getBooks(@RequestParam(required = false) String genre) {

    List<BookDto> bookDtos = new ArrayList<>();
    List<Book> allBooks = getAllBooks();

    for (Book book : allBooks) {
       // This if condition is what I take issue with
       if (genre == null || book.genre.equals(genre)) {
           bookDtos.add(book.convertToDto());
       }
    }
    return bookDtos;

}

I think it is bad practice to use null here to convey the meaning of get everything. Is this correct?


My current solution is to distinguish between these two cases at the very top level like so:

public ResponseEntity getBooks(@RequestParam(required = false) String genre) {

    if (genre == null) {
        return getAllBooks();
    } else {
        return getBooks(genre);
    }
}

And then a few supporting functions that funnel all of the logic into the same place by massaging the inputs a little bit.

// This is the core code, that takes a list of genres and returns all books from all those genres.
public Collection<BookDto> getBooks(List<String> genre) {
    // And here is the code that will actually get the books from the database.
    // ...
    // ...
}

// This is a supporting function, that finds all genres and funnels that into the above function.
public Collection<BookDto> getAllBooks() {
    List<String> allGenres = getAllGenres()
    return getBooks(allGenres);
}

// This is a supporting function that builds a list from a single genre, to again funnel into the 1st function.
public Collection<BookDto> getBooks(String genre) {
    List<String> genres = new ArrayList<>();
    genres.add(yearCode);
    return getBooks(genres);
}

The idea here is to have a single place where the work actually takes place and the database is accessed, but to have multiple ways to access it. And more importantly to remove the idea of null == all in the codebase, aside from the very top level where the execution starts after the user requests /api/books.

It's always felt natural to me but I've not seen it used by anyone else (I've only worked at a few places, so this could just be inexperience on my part). Is the proposed design good? Is there a standard way of solving this problem?

\$\endgroup\$
  • \$\begingroup\$ Just a small note: your last method probably won't compile. Did you edit it here after copying? \$\endgroup\$ – Hubert Grzeskowiak Apr 11 '17 at 4:36
  • \$\begingroup\$ Not sure what REST API you are using but are you sure it's giving you null and not an empty string? Better check for both and do the same action. Or even better: write a request filter that turns nulls for strings to empty strings and only handle empty strings in your logic. \$\endgroup\$ – Hubert Grzeskowiak Apr 11 '17 at 4:38
  • 1
    \$\begingroup\$ "but /api/books will return all books from all genres." I am pretty sure it's a common pattern. I see it in every book on rest api. C# exemple of this patern used in a tutorial \$\endgroup\$ – Drag and Drop Apr 11 '17 at 6:48
  • 2
    \$\begingroup\$ It's the same pattern SQL is using for decades. \$\endgroup\$ – Philipp Apr 11 '17 at 8:07
  • \$\begingroup\$ more importantly why are you even getting all the books when i just want "romance" books. This would be expensive if there is I/O involved in getting the books list. \$\endgroup\$ – Sikorski Apr 11 '17 at 12:00
22
\$\begingroup\$

I personally think it's fine, but it helps to turn your thinking around from a "selector" to a "filter". Then, the default is to return all books, and you selectively filter down the set you're going to eventually respond with.

(Note I haven't written Java in a long time, so grain of salt here,) I might specify a BookFilter object which can incrementally have fields set from the parameters, and then have BookDTO.filterBooks. Then, your request method becomes:

public ResponseEntity getBooks(@RequestParam(required = false) String genre) {
    BookFilter filter = new BookFilter();
    if (genre != null) {
      filter.setGenre(genre);
    }

    return filterBooks(filter);
}

I know it's not directly applicable (your DTO probably doesn't work this way, based on what you described above), but I think what you're doing with the null check in your controller is pretty much the same thing with the tools you've got.

You might consider trying to refactor your DTO so it can work with filters however, because eventually you may want ?genre=scifi&published=1970+&publisher=penguin etc. etc., at which point it'll be a lot easier to manage a single filter object than a bunch of explicit queries.

\$\endgroup\$
3
\$\begingroup\$

Instead of allowing the API user to retrieve all items, which could be a huge quantity of data to transmit, I would force paging, so that only the first n items were returned and the URL must be modified to get more.

e.g. /api/books?page=2

...or some other query

\$\endgroup\$
1
\$\begingroup\$

This API pattern -- null means everything -- is actually quite old. Think SQL:

select * from books

vs

select * from books where genre in <genre-list>

This also gives us a hint on implementation: the predicate, if present, is an additional filter on the list.

So this is really to support @Jon's answer above. But I do have an additional comment on the implementation. Where are the Books stored? Database? In that case getAllBooks() and then filtering can be needlessly expensive. Consider pushing the filter to the storage.

The same filtering pattern can be applied to constructing SQL or whatever mechanism you are using for fetching the books from the system of records.

\$\endgroup\$
  • 2
    \$\begingroup\$ I don't know, genre isn't null in your first request, it's just not a specified criteria ; select * from books != select * from books where genre is null \$\endgroup\$ – Aaron Apr 11 '17 at 12:31
  • \$\begingroup\$ My analogy is to the actual API /api/books and /api/books/genre=something. OP writes that genre is null in the former case but in the url it is a non-specified criteria \$\endgroup\$ – Miserable Variable Apr 12 '17 at 2:22
  • \$\begingroup\$ Yes, in the URI it's unspecified, which translates to a null value for the RequestParam. I agree your analogy supports well the use of unspecified parameters in the URL, but I think OP's problem was with his servlet's code where he feels like null is a strange value to represent <genre-list>. \$\endgroup\$ – Aaron Apr 12 '17 at 9:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.