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

The README is not enough documentation to actually start using this library #1031

Open
ePaul opened this issue Dec 31, 2020 · 2 comments
Open

Comments

@ePaul
Copy link
Member

ePaul commented Dec 31, 2020

Description

There seems to be some information missing in the README to actually use the library.

Expected Behavior

Reading the README (or some other documentation linked from there) should be enough to actually start using the library, i.e. sending out HTTP requests and doing something with the response.

Actual Behavior

I've read the README (and several of the other documents linked from there), and still have no clue on what exactly to do.
Reading the source of some of the classes helped somewhat, but not fully.

Possible Fix

  • Make sure there is example code which is complete (i.e. including all needed imports).
  • If any other dependencies are needed (i.e. some of the ~ 20 riptide-* projects), make this visible.
  • List/link the relevant classes/methods I need to use, or might want to use.
  • If there is some other documentation which already has this↑ (maybe a Javadoc site?), make sure it's clearly visibly linked.

Context

I remembered from some presentations at work (at Zalando) that "Riptide" is the new way of doing HTTP requests (and getting responses) nowadays. I got a (private) problem where I want to automate filling a form to book an appointment, for which I need some HTTP requests (and handling the responses), so I said "let's try this out".

Steps to Reproduce

Being on vacation, I don't have any access to production code which is using Riptide (from which I could copy this), and I also didn't find any example code in this repository.
I'm just looking at the README (and everything which is linked from there).

The first step in my project was

Call this URL (with GET) and when it returns a redirect (any 3xx, though in this case a 302), do something with the URL in the Location header.

I've set up an Http object using the riptide-spring-boot-starter.

@Autowired
@Qualifier("formularserver")
private Http formularServer;

Looking at the Requests section, sending the request seems easy:

formularServer
    .get(INITIAL_URL)

Now handling the response ...
This short section with one conceptual paragraph, and one piece of example code seems to be all there is on documentation for what is supposed the main feature of Riptide. And the example code doesn't even work when copying it, because all the imports are missing (specifically, the static imports for on, series(), contentType(), status(), anyStatus() and the Spring HTTP status code and series enum values).
(It also references user code, but I can understand this.)

With some browsing through the classes in riptide-core, I found that I need those imports (I guess * imports would also work):

import static org.springframework.http.HttpStatus.Series.REDIRECTION;
import static org.zalando.riptide.Bindings.on;
import static org.zalando.riptide.Navigators.series;

...

public void findForm() {
	formularServer
		.get(INITIAL_URL)
		.dispatch(series(),
				on(REDIRECTION)
				    .dispatch( /*TODO: now what? */ null, null));
}

... or possibly one of the .call(...) in the last line?

After some more digging I found that I can just implement my own Route, or rather a ThrowingConsumer<ClientHttpResponse> which will just fetch the header from the response itself.

public void findForm() {
	formularServer
		.get(INITIAL_URL)
		.dispatch(series(),
				on(REDIRECTION).call(this::printLocation)
				);
}
void printLocation(ClientHttpResponse response) {
	LOG.info("location header: ", response.getHeaders().get("Location"));
}

I still have the suspicion that there is some higher-level way of doing this, though, which I didn't find yet.

I didn't yet get to the next step of actually using this location header for making the next request, and then extracting data from the response to that request.

(Actually running this (with a debugger) shows that the Apache HTTP client which is used here already takes care of the redirect internally, so I guess this all was just wasted effort.)

Your Environment

  • Version used: library version 3.0.0-RC.15, README version of ec6b614.
@whiskeysierra
Copy link
Collaborator

Make sure there is example code which is complete (i.e. including all needed imports).

I generally avoid putting imports into code blocks in READMEs. They are the first lines of the block but the least important and therefore distract more than they help.

If we need some more viable samples then I'd suggest to either have a separate Maven module which just contains executable sample code. That way anyone can look at proper Java code (w/ imports) and we can actually check that it's correct by executing it as part of our test suite.

If any other dependencies are needed (i.e. some of the ~ 20 riptide-* projects), make this visible.

Apart from the dependencies declared in the respective POMs, what are you missing exactly?

List/link the relevant classes/methods I need to use, or might want to use.

Like the following paragraphs? https://github.com/zalando/riptide/blob/main/docs/concepts.md#route

I still have the suspicion that there is some higher-level way of doing this, though, which I didn't find yet.

Can you specify what you expect some "higher-level way" would look like?

I didn't yet get to the next step of actually using this location header for making the next request, and then extracting data from the response to that request.

Either you treat the header like a response and capture it so that you can use it later (i.e. CompletableFuture<URI>) or instead of logging, just execute the next request right there.

@ePaul
Copy link
Member Author

ePaul commented Jan 5, 2021

Make sure there is example code which is complete (i.e. including all needed imports).

I generally avoid putting imports into code blocks in READMEs. They are the first lines of the block but the least important and therefore distract more than they help.

I agree for imports of types where the type name is unique enough. For static imports of method names, where unaware people (like me) have no way of knowing in which class to look, it is needed. (Alternatively, include the class name for the method calls instead of using static imports.)

If we need some more viable samples then I'd suggest to either have a separate Maven module which just contains executable sample code. That way anyone can look at proper Java code (w/ imports) and we can actually check that it's correct by executing it as part of our test suite.

I'm fine with that too (if it can be found from the README).

If any other dependencies are needed (i.e. some of the ~ 20 riptide-* projects), make this visible.

Apart from the dependencies declared in the respective POMs, what are you missing exactly?

I'm not sure what was missing in my case, that's the point. There are many projects, and I don't know which ones I need for my simple use case.

List/link the relevant classes/methods I need to use, or might want to use.

Like the following paragraphs? https://github.com/zalando/riptide/blob/main/docs/concepts.md#route

Something like this, yes. I somehow missed this document (or this part of the document).
(And this even has exactly the Location header example I needed.)

I still have the suspicion that there is some higher-level way of doing this, though, which I didn't find yet.

Can you specify what you expect some "higher-level way" would look like?

I somehow thought that I pass a lambda which will get the header as a parameter, not that I need to dig through the ClientHttpResponse object myself. But this was before I understood that a Route is just a fancy Consumer<ClientHttpResponse>.

I didn't yet get to the next step of actually using this location header for making the next request, and
then extracting data from the response to that request.

Either you treat the header like a response and capture it so that you can use it later
(i.e. CompletableFuture<URI>) or instead of logging, just execute the next request right there.

Thanks, I'll try that. When I'm done I might propose some concrete additions to the README.

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

No branches or pull requests

3 participants