Reference client kick-off #1

alien-mcl · 2017-06-16T19:11:40Z

There is always someone that needs to start, thus please find this PR of the initial commit with basics of the future hydra reference client in typescript.

This is just the beginning - just a couple of coded classes, some propositions of interfaces and general concept of how it could look like, but I think it's good enough to start discussion and the development process. There is plenty of room for amendments so feel free to throw your ideas.

Main concepts driving what's already there:

client is stateless
client uses owned memory model for hypermedia controls (for JS/TS it can be JSON-LD frame, but still)
implementation tries to be a reference for future ports to other languages
implementation tries to be rooted in the use cases prepared some time ago

tpluscode · 2017-06-18T10:30:23Z

package.json

+    "version": "0.0.1",
+    "scripts": {
+        "test": "node node_modules/karma/bin/karma start",
+        "test-once": "node node_modules/karma/bin/karma start --single-run"


Unrelated to Hydra, I'd swap these scripts. Travis will run npm test and it makes more sense that it was the single run script

I have no opinion here. Whichever is better from a developer point of view (not a tool point of view)

Since start and test are standard script commands, I'd say just use them. It's also possible to chain script commands together, so I'd do the following:

"scripts": { "start": "node node_modules/karma/bin/karma start", "test": "npm start -- --single-run" }

@asbjornu +1 for chaining.

I would suggest keeping test and test-once as names since they are action specific and related by name.

Not sure if this is relevant but I remember reading that the watch parameter also need to be specified as false for the single run command.

Changed to 'test' (single run) and 'live-test' (for continuous running test session)

tpluscode · 2017-06-18T10:34:40Z

src/DataModel/IApiDocumentation.ts

+{
+    entryPoint: string;
+
+    getEntryPoint(): Promise<IData>;


I don't like how this is the functionality of the documentation. I treat the ApiDocumentation itself as a resource. Besides, this smells like it would violate Single Responsibility Principle

What would you think about smth like

IHydraApi api = new HydraApi("http://entrypoint"); IApiDocumentation docs = api.getApidDocumentation(); IResource entry = api.getEntryPoint();

As for the Single Responsibility Principle - I don't think it would be a violation - It's a functional inheritance and actual implementation could be a simple wrapper on top of an actual implementation.

Indeed there is an inconsistency in use cases - some places uses method chaining, i.e.:

var operation = client.get("http://example.com") .getApiDocumentation() .getEntryPoint()

And some other are using client as an invocation facility for operations, i.e.:

var operation = client.get("http://example.com") .getApiDocumentation() .getEntryPoint() .getOperations() .filter(op => op.method == "POST" && op.expects == "Event")[0]; client.invoke(operation, event);

I'm not sure which one is better. The latter seems cleaner, but it would force client to do quite a lot (i.e. make distinction on how to work with simple Url string, Link, Iri template or an operation).

As for your example, I wouldn't like to see developers create instances - I'd prefer our API to do it on it's own so developers don't need to bother about real implementation.

I wouldn't like to see developers create instances

Funny you'd say that. Previously I didn't actually implement it that way ;). Now that I look again at your code, maybe I was misjudging it a little.

My actual point is that ApiDocumentation seems to work both as a Hydra client and the documentation. I guess I'd simply move all the metadata business to this.client and maybe that's it

That's why I'm torn apart - now I somehow tend to stick to that latter approach where client does all the stuff - I'm just worried that the client would be overloaded with logic.
The thing is that code equivalent following that approach:

var api = client.get("http://example.com"); var apiDocumentation = client.get(api.getApiDocumentationUrl()); var entryPoint = client.get(apiDocumentation.entryPoint); var operation = entryPoint .filter(op => op.method == "POST" && op.expects == "Event")[0]; client.invoke(operation, event);

looks a bit messy

Oh, so procedural. Definitely not what I had in mind ;). But read again and notice that I meant moving the implementation details from the ApiDocumentation class. My idea was that it shouldn't care about fetch and metadataProvider etc. It would simply be

function getEntryPoint() { return this.client.getResource(this.entryPoint); }

Otherwise the snippets in you comment above look good to me. Well, maybe I'd make the operation more self-aware, but that's a different discussion

var operation = {}; var event; operation.invoke(event);

Well, not so different. For simple Urls we'll need to call client's method, but for strongly typed resources we will use their methods - it's not really consistent.
As for the implementation hint - I had to be really tired when I was writing it :). It's so obvious this should be client's method called here

From my experience in implementing hypermedia clients with System.Net.HttpClient, I find that having a central infrastructural and generic Client class that deals with HTTP communication, error handling, serialization, deserialization, etc., and then passing it around to the different specifics of the library works well.

Equivalently, I think it's wise to make ApiDocumentation rely on Client to do the heavy lifting and then just applying the specifics within the ApiDocumentation class.

'ApiDocumentation; is now light and depends on a newly introduced HydraClient.getResource method,

tpluscode · 2017-06-18T10:40:18Z

src/DataModel/IData.ts

@@ -0,0 +1,6 @@
+import {IMetadata} from "./IMetadata";
+
+export interface IData extends Object


I don't like this and IMetadata. They seems too generic and separated from the Hydra terminology. I find using such disconnected terms confusing.

For IData, did you mean something like IHydraApi?

By IData i meant raw data. The thing that actually client wanted. Metadata - hypermedia controls, data structure description is something that helps a client to understand what it can/needs to do.
I'm open for naming suggestions. Hypermedia controls is to narrow for me (i.e. for structure definitions).

I wouldn't like to see developers create instances

I think the client wants resource representations. What if instead IData we used IResource as the common denominator?

And metadata? I think I like the idea. You wat to separate the controls from the actual representation, right? A place to put operations, etc? So what if it was called IHypermedia or IControls (or IHypermediaControls?).

var resource = client.getResource('http://example.com'); resource.controls.getApiDocumentation(); resource.controls.getOperations(); resource.controls.getLinks();

Indeed I'd love to have raw data and hydra stuff somehow separated. As for the IData replacements - IResource is already used to denote actual resource. I wouldn't like to add a metadata property there. Bu I see direction your thoughts are going - I need to give it some thoughs.
As for metadata - maybe IHypermedia. I've got feeling that it's still to narrow as there may be something that doesn't qualify.

@asbjornu thoughts?

I don't find this that much of an issue. Sure, you get stuff mixed a little but that's why these are representations and not resources. Hypermedia is part of the representation, right?

This library doesn't know what higher level libraries, apps, UIs want to do with the data so I wouldn't introduce such (leaky) separations/classifications here.

I agree that this separation might not come easily. I agree that library doesn't know what higher level libraries, apps and UIs want to do with the data. I still think that client should know what is a hypermedia control - it's supposed to consume those.
Anyway - from API point of view - client should expose somehow both the payload (separated or not) and hypermedia controls it discovers within that payload.
@tpluscode example is a good one in that matter.

Bundling hypermedia controls into a property seems like a clean approach. 👍 But what are they, exactly? Affordances? Operations? Actions? Controls? Naming things is hard.

Renamed with IWebResource. I wanted to leave IResource to match the term used in the Hydra spec. The metadata property is renamed to hypermedia.

tpluscode · 2017-06-18T10:43:10Z

src/DataModel/IMetadataDescription.ts

+import {IOperation} from "./IOperation";
+import HydraClient from "../HydraClient";
+
+export interface IMetadataDescription extends IMetadata


Similarly to the above. Hydra specification doesn't speak about any metadata anywhere. The properties below suggest to me that maybe this would actually be the IApiDocumentation interface? And the current IApiDocumentation could become simply IApi?

I'm sorry I contradict earlier comments but I'm coming up with this stuff as I go through and these are my thoughts :)

No problem - that's why it's a PR - we can discuss it

+1 to what @tpluscode said

This one was removed

tpluscode · 2017-06-18T10:44:16Z

src/DataModel/IMetadataProvider.ts

+     * @param {Response} response Raw fetch response holding data to be parsed.
+     * @returns {Promise<IData>}
+     */
+    parse(response: Response): Promise<IData>;


Does it really parse? How about process or provide?

Turtle implementation would actually parse it to get a JavaScript compatible objects.

Sure, but I think that parsing is just part of what IMetadataProvider does, is all.

So maybe process - provide is already in the name of the interface thus it is also part of what it doas.

Renamed with IHypermediaProcessor. Method is now process.

tpluscode · 2017-06-18T10:47:31Z

src/DataModel/IResource.ts

+export interface IResource
+{
+    "@id": string;
+    "@type"?: Array<string>


That's interesting. I've nothing against @ but I think your intention was to make the representation serialization-agnostic, right? This seems clearly JSON-LD-inspired.

Yes - I don't like @id either. The issue is that it's not something I can do with i.e. JSON-LD framing. Initially I had here an iri property.

Actually, JSON-LD can do that

He, good point. Didn't try it.
Still, it's a matter of a good replacement:
@id : iri (id or url will be already used in most cases)
@type : ? (type may be used as well, class is usually a keyword and may cause mayhem)

Why do we actually use properties here and not methods? The programming interface doesn't need to match what's send on the wire

Being just pragmatic - I'd prefer to have resource's identifier as a property. Also, we'd need to extend every single JSON object from the payload with that method's implementation - it's a drag.

Something you'd still want to do to make a property read-only. A getter method would be just the same

Also, we'd need to extend every single JSON object from the payload with that method's implementation - it's a drag.

I think this is actually a deeper question. Do we want to use JSON-LD as data representation? Both internally and accessible through Heracles' interfaces? If not, the point above is moot. If yes, this should be made explicit.

No - currently I'm using JSON-LD framing to transform hypermedia controls I discover to an internal model. It is rooted in the hydra's structure, but still it is supposed to be separated from the JSON-LD serialization.
I also just don't want to parse JSON-LD too much as it is still JavaScript object which can be used with little modifications.

Properties renamed to iri and isA (anyone got better name for the latter one?)

tpluscode · 2017-06-18T10:51:28Z

package.json

+    "dependencies": {
+        "jsonld": "^0.4.12",
+        "typescript": "^2.3.3",
+        "whatwg-fetch": "^2.0.3"


I've got someone asking in "my" heracles to use isomorphic-fetch so that the client can be used both in browser and Node. Let's do that from get-go.

Good point.

Replaced with isomorphic-fetch

tpluscode · 2017-06-18T10:53:24Z

src/HydraClient.ts

+     */
+    public async getApiDocumentation(url: string): Promise<IApiDocumentation>
+    {
+        let response = await window.fetch(await this.getApiDocumentationUrl(url));


Hm, and what if url was the ApiDoc already?

In Typescript it would show a compilation error :). But seriously - obviously it is not covered here yet. Indeed I was thinking about various scenarios (i.e. programmer provides api or entry point directly) - these needs to be taken into account.

Due to that reason I'd move this method to IResource. If the resource has already been retrieved, it's trivial to figure out whether it was a ApiDocumentation already or whether a reference to it was included in the HTTP headers (which would then be returned instead)

I'm not sure I'm following you. IResource is a plain design-time interface and contains no implementation. By moving any methods to this pseudo-type we'd need to ammend any JSON object with an implementation.
Anyway, I'm not sure how method getApiDocumentation is related to IResource.

IMO IResource should be a wrapper that contains a complete response including headers. It doesn't need to break down a response into individual resources from the get-go. In fact, it probably needs to do very very little processing at all till the user expresses what it wants to do with it.

Well, worth of considering. This way it could be possible to delay or in some cases skip parsing at all.
Consequence is that many of those internal methods used for hypermedia discovery will be thenable, thus methods exposed also should.

If it's supposed to contain headers, we might look into the design of System.Net.HttpClient, which has HttpRequestMessage and HttpResponseMessage objects that are envelopes containing properties such as Headers, StatusCode, Content, etc. to inspect the lower level details of the request. The HTTP body is inside the Content property, which can be read as a stream, multipart or byte array and then deserialized to a typed class.

Would this design work for Heracles too?

Fetch comes with something similar. There are types that represents a response with access to it's body and headers. It's already in use in that code of this PR.

Passing ApiDocumentation url is not yet supported. Need some extra time for that.
Also lazy loading/response wrapping is not yet implemented - I'd leave that for a while as I've got feeling that would complicate things. Maybe next use cases will prove that this approach is any better.

I've created issue #2 that addresses an option of having that lazy processing implemented.

tpluscode · 2017-06-18T10:57:01Z

src/HydraClient.ts

+            throw new Error(HydraClient.responseFormatNotSupported);
+        }
+
+        let apiDocumentation = <IApiDocumentation>(await metadataProvider.parse(response))


Ah, that's what I meant earlier let apiDocumentation = [...] entryPoint. Don't think that's right ;)

I'm not sure what do you mean

I mean that you retrieve entryPoint but cast it as IApiDocumentation variable

Ugh - it's getApiDocumentation method and it retrieves ApiDocumentation. First url passed is a website's Url from which it tries to obtain an ApiDocumentation url which should contain an entry point definition.

Oh right then, sorry

lanthaler · 2017-06-19T18:21:28Z

src/DataModel/IClass.ts

@@ -0,0 +1,5 @@
+import {IResource} from "./IResource";
+
+export interface IClass extends IResource


What is this used for? To represent hydra:Class?

Yes, someday when we'll reach that point.

Again, I don't think the code needs (or even should) match the representations on the wire 1:1

On the wire - indeed. But I think client's model should be rooted in hydra's model somehow.

lanthaler · 2017-06-19T18:25:25Z

src/DataModel/IMetadata.ts

@@ -0,0 +1,6 @@
+import HydraClient from "../HydraClient";
+
+export interface IMetadata


I'd get rid of this class for the reasons mentioned above

This one is still here, but I got rid of IMetadataDescriptor. This one also got renamed.

lanthaler · 2017-06-19T18:28:49Z

src/DataModel/IMetadataProvider.ts

+/**
+ * @interface Describes an abstract meta-data providing facility which translates from raw @link Response to an abstract data model.
+ */
+export interface IMetadataProvider


Could you please elaborate a bit on why this class is needed? What is the abstract data model? IData and IMetadata? Something else?

I wanted to introduce an intermediate layer that would isolate the client and it's hypermedia controls internal data model from actual RDF serialization received. This would be a job for a IMetadataProvider to make that translation. In case of JSON-LD it would be i.e. framing.
As for the IData and IHypermedia - it is an attempt to isolate raw data that the client expected from the hypermedia controls so these can be easily used in the code.

I agree with the general idea of separation. Although I'm anxious to see how framing alone will be enough. I've been there too and I had to resort to actual triple processing as an intermediate processing step

Well - framing alone won't do it. There is already a very simple attempt to that topic in JsonLdMetadataProvider.ts (i.e. parseMetadata)

lanthaler · 2017-06-19T18:31:30Z

src/DataModel/JsonLd/JsonLdMetadataProvider.ts

+    public async parse(response: Response, removeFromPayload: boolean = false): Promise<IData>
+    {
+        let payload = await response.json();
+        let expanded = await jsonLd.flatten(payload);


s/expanded/flattened/

Yep - forgot to replace variable name while experimenting with JSON-LD api.

Changed to flattened.

lanthaler · 2017-06-19T18:34:35Z

src/HydraClient.ts

+import ApiDocumentation from "./ApiDocumentation";
+
+/**
+ * @class HydraClient Represents a Hypermedia Driven Application (HyDrA) Core Vocabulary client.


Let's just use "Hydra" here without spelling it out. We could add a link back to our homepage though

But having a class description like Hydra client is useless. We can also drop it completely (I'd prefer to leave any description for sake of jsdocs)

Well, it could be something along the lines of

Heracles is a generic client for Hydra-powered Web APIs. To learn more about Hydra...

Changed the description accordingly.

lanthaler · 2017-06-19T18:36:12Z

src/HydraClient.ts

+ */
+export default class HydraClient
+{
+    private static _instance: HydraClient ;


Making this a singleton will reduce testability quite a bit

This is optional. Default constructor is public and can be invoked manually.

What is it good for then?

I don't need to care about creating instances. Anyway - it is safe to be removed.

Removed the singleton.

lanthaler · 2017-06-19T18:40:25Z

src/HydraClient.ts

+    public getMetadataProvider(response: Response): IMetadataProvider
+    {
+        return HydraClient._metadataProviders.find(provider =>
+            !!provider.supportedMediaTypes.find(mediaType => mediaType === response.headers.get("Content-Type")));


Hmm... looks like MediaTypeProcessor would actually be a more accurate name than MetadataProvider.

Yes, sounds better.

Renamed to IHypermediaProcessor.

lanthaler · 2017-06-19T18:54:13Z

Fantastic work Karol. Thanks for kick-starting this

alien-mcl · 2017-06-25T09:28:59Z

I've just pushed a commit that addresses the comments. I'll try to review for any unaddressed issue ASAP.

lanthaler · 2017-06-26T19:51:22Z

@alien-mcl, let me know when all comments have been addressed (including opening issues for them) and I'll proceed with merging the PR as agreed in today's telecom.

alien-mcl · 2017-06-26T21:00:40Z

There is last outstanding comment/question regarding src/DataModel/IClass.ts here . I'll opt for leaving that as it is. I believe it will be required in near future for supported classes modelling.
Other comments are addressed or answered.

lanthaler · 2017-06-27T19:04:52Z

Sounds good. I'll go ahead and merge this PR. Thanks a lot @alien-mcl. Great work

Initial commit.

80e3949

tpluscode suggested changes Jun 18, 2017

View reviewed changes

lanthaler reviewed Jun 19, 2017

View reviewed changes

Changes after PR feerback.

05f75bc

lanthaler merged commit 1f36476 into HydraCG:master Jun 27, 2017

		@@ -0,0 +1,6 @@
		import {IMetadata} from "./IMetadata";

		export interface IData extends Object

		@@ -0,0 +1,5 @@
		import {IResource} from "./IResource";

		export interface IClass extends IResource

		@@ -0,0 +1,6 @@
		import HydraClient from "../HydraClient";

		export interface IMetadata

Reference client kick-off #1

Reference client kick-off #1

Conversation

alien-mcl commented Jun 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kartikadur Jun 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpluscode Jun 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alien-mcl Jun 18, 2017 • edited Loading

Choose a reason for hiding this comment

tpluscode Jun 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alien-mcl Jun 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alien-mcl Jun 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alien-mcl Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alien-mcl Jun 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alien-mcl Jun 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kartikadur Jun 22, 2017 •

edited

Loading

tpluscode Jun 18, 2017 •

edited

Loading

alien-mcl Jun 18, 2017 •

edited

Loading

tpluscode Jun 18, 2017 •

edited

Loading

alien-mcl Jun 25, 2017 •

edited

Loading

alien-mcl Jun 18, 2017 •

edited

Loading

alien-mcl Jun 20, 2017 •

edited

Loading

alien-mcl Jun 25, 2017 •

edited

Loading

alien-mcl Jun 25, 2017 •

edited

Loading