katharsis-project / katharsis-framework

Katharsis adds powerful layer for RESTful endpoints providing implementenation of JSON:API standard
http://katharsis.io
Apache License 2.0
135 stars 65 forks source link

Katharsis client is lack of per requst customizing #422

Open jianglibo opened 7 years ago

jianglibo commented 7 years ago

Katharsis client provides exactly the same interface as server side, It's great. But you are unable to customize the headers by per request. If Katharsis client sits at the middle of browser and api server, per request header is needed for that situation, for example, jwt token.

chb0github commented 7 years ago

Agreed. Sharing interfaces is the wrong approach - that's an RPC approach not a REST approach. I didn't write it but welcome a PR.

remmeier commented 7 years ago

why that? there is a clear contract by the json api specification that manifests in that interface. Authentication should not go directly into the method call, but rather should handled transparently by a filter.

on the client side there is a HttpAdapter, you can cast it to either implementation (apache or okhttp). There you find listeners to hook into the HTTP requests to add security, monitoring, etc.

remmeier commented 7 years ago

katharsis-brave makes use of that as well.

remmeier commented 7 years ago

but as @chb0github suggests maybe a slightly lower level interface to make calls could also be worthwhile in some occasions.

chb0github commented 7 years ago

It's pretty simple: JSONApi is about the pay AND the return codes AND the query params

if I have an API on my server side of public Iterator<Thing> findAll(...) what's the HTTP return code from that? How do I access the headers? Those questions have actually come up in various tickets. Katharsis is an opinionated server-side framework so it doesn't matter server side. But client side it definitely can matter. Subsequent code after the Katharsis filter may add headers that are relevant to the client - they are inaccessible in the given implementation.

The client needs access to the full entity to be most effective.

HTTP is a poor substitute for RPC.

remmeier commented 7 years ago

virtually all use cases I have come across so far have been related to monitoring, security tokens, corrs handling and things like that, all of which are best taken care of as a filter in the underlying http/rest implementation, not in Katharsis itself.

KatharsisClient provides access to the json api query parameters, (type-safe) payload and the more interesting return codes by means of the exception mappers. But yes, definately, a lower level api having access to everything would be great to have. For typical use cases the already existing implementation seems perfectly fine.

SchedulerRepository in katharsis-client provides a bit of a more advanced example with type-safe results, meta data, links and everything. That is quite aggreable to work with. The stubs are also able to not only make JSON API calls, but also (so far) JAX-RS calls (a mix of both is what I see in many projects). I do have the same interface generated for Typescript (so far). At some point I can make that public.

regarding access to headers there is https://github.com/katharsis-project/katharsis-framework/issues/354

jianglibo commented 7 years ago
KatharsisClient client = new KatharsisClient("http://localhost:8080/api");
ResourceRepositoryV2<Task, Long> taskRepo = client.getRepositoryForType(Task.class);
List<Task> tasks = taskRepo.findAll(new QuerySpec(Task.class));

Yes, it's pretty neat. Let's compare to Spring resttemplate, the key issue is the Spring resttemplate is a lightweight implementation, new a new one and customize it, use it, then discard it.

I don't agree with @chb0github on the need to get response details, I think following Jsonapi spec is enough. So if can make KatharsisClient lightweight, and the code change to the kind of below, the implement now is good.

KatharsisClient client = new KatharsisClient("http://localhost:8080/api").header("jwtToken",thetoken).header("x","y"));
ResourceRepositoryV2<Task, Long> taskRepo = client.getRepositoryForType(Task.class);
List<Task> tasks = taskRepo.findAll(new QuerySpec(Task.class));
remmeier commented 7 years ago

add guess HttpAdapter.setRequestHeader would be nice.

currently is is more like:

HttpClientAdapter okHttpAdapter = (HttpClientAdapter) adapter;
okHttpAdapter.addListener(....);

with a

@Override
    public void onBuild(OkHttpClient.Builder builder) {

method. @chb0github is right that sometimes it can be useful, so a lower-level api used by the stubs would be great.

jianglibo commented 7 years ago

img_2147 @remmeier I cannot image out how to set header by per request. For the every http request come from the browser which carries a jwttoken I use the Katharsis client with that token to request backend Katharsis server, where does the Katharsis client to get the token from? All these tokens didn't exist when the Katharsis client is built.

remmeier commented 7 years ago

basically all the systems I have seen put security information into some kind of request/thread-local to make it accessible anywhere without having to pass it trough with every method call.

you could make use of that with okHttpAdapter.addListener (resp. the apache http adapter). From there you get a builder to initialize the HTTP client. There you can hook in a listener that listens to requests and can set those additional headers. For okhttp it is addInterceptor or addNetworkInterceptor I think.

jianglibo commented 7 years ago

@remmeier I figure it out finally. Thank you! To solve my problem mentioned is just to write a filter to catch incoming token and put it into to a thread-local holder, and in the time building Katharsis client adds a listener to obtain the underlying builder and add an intercept to it, in the intercept implementation extract the token from thread-local holder and put it to request header.