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 json_result documented against Sandbox Test API is wildly out-of-date #48

Open
QuintinWillison opened this issue Jan 9, 2020 · 7 comments

Comments

@QuintinWillison
Copy link
Contributor

QuintinWillison commented Jan 9, 2020

See: https://docs.ably.io/client-lib-development-guide/test-api/

I've been referring to AppSpec in Setup.java as I'm yet to find docs (not just live code) that properly defines this response.

Perhaps this API should be properly documented rather than just vaguely demonstrated by example?

┆Issue is synchronized with this Jira Task by Unito

@QuintinWillison QuintinWillison added the bug Something isn't working label Jan 9, 2020
@paddybyers
Copy link
Member

I agree a spec would be an improvement. What IDL should we use for this?

I know that we use the Rust IDL in the spec docs for APIs but we also use protobuf elsewhere as the IDL for request/response payloads, for input validation (eg in the admin API).

To kick things off, this is a draft protobuf based on a reverse-engineering of the current code:

message SimulationSpec {
	boolean tlsOnly;
	string notes;
	repeated KeySpec keys;
	repeated NamespaceSpec namespaces;
	repeated ChannelSpec channels;
}

message KeySpec {
	int32 status;
}

message NamespaceSpec {
	int32 status;
	string id;
	boolean persisted;
	boolean pushEnabled;
}

message ChannelSpec {
	string name;
	repeated PresenceSpec presence
}

message PresenceSpec {
	string data;
	string clientId;
}

message SimulationDetails {
	int32 status;
	uint64 created;
	uint64 modified;
	boolean tlsOnly;
	string labels;
	string notes;
	boolean enablePusherCompatibility;
	string id;
	string appId;
	string accountId;
	repeated KeyDetails keys;
	repeated NamespaceDetails namespaces;
	repeated ConnectionDetails connections;
	repeated ChannelDetails channels;
}

message KeyDetails {
	string id;
	string value;
	uint64 expires;
	string capability;
	string keyName;
	string keySecret;
	string keyStr;
}

message NamespaceDetails {
	string id;
	uint64 created;
	uint64 modified;
	boolean persisted;
	boolean pushEnabled;
	boolean hasStats;	
}

message ConnectionDetails {
	string name;
	string key;
}

message ChannelDetails {
	string name;
	repeated PresenceDetails presence;
	string connection;
}

message PresenceDetails {
	string data;
	string clientId;
}

service SimulationService {
	rpc createSimulation (AppSpec) returns (SimulationDetails);
}

@QuintinWillison
Copy link
Contributor Author

Having never used Rust, I just tried to look up the Rust IDL which you refer to @paddybyers but I can't find anything obvious out there in the wild. Can you point me in the right direction?

In the features spec we refer to "bespoke IDL" with no reference to Rust. I confused.

Anyway... For me, the sort of questions I ask when I see an interface include:

  • Properties / Fields:
    • Is it required? i.e. should a deserialiser expect to see it and must a serialiser have something to set
    • Can it be null? As discrete from not required. Explicitly setting something to null can, and often should, have a different meaning to omitting it entirely
    • If it's an object (map) then can it be empty?
    • If it's an array (list) then can it be empty?
    • If 'proper' idiomatic implementation of a property, from an interface accessibility standpoint, is via get / set methods (variably encouraged and/or enforced in various languages including Java and C#) then is this read-only? [likely relates to mutability of parent]
  • Classes / Interfaces:
    • Are they immutable?

It seems we're more focussed on the fact that these things we specify in our "IDL" are data structures, with less concern for the "interface" part of that story. IMO, nullability and mutability are very important and should be considered at inception of a design.

If we care about some or all of the things I mention above in a given context then my preference would be that we find an IDL that can express that without requiring "comments as annotations".

@SimonWoolf
Copy link
Member

SimonWoolf commented Jan 9, 2020

In the features spec we refer to "bespoke IDL" with no reference to Rust. I confused.

The first version of it was written by Toni, who based the syntax heavily on syntax Rust uses for type annotations. Since neither I now Paddy are that familiar with Rust yet, I suspect that the bits we've added may have somewhat drifted away from that.

I'm not sure protobuf is a great fit for an IDL to have in documentation - it's fairly limited in what it can express (e.g. all Messages are nullable, no primitive values are), it's not very readable (eg everything's flat so hard to parse the nesting in your head), and we could only ever use it to describe data structures and not full APIs - would be nice for something that can do both.

One thought: we have now started using Typescript, and typescript type specs are pretty powerful/flexible, we could start making heavier use of those, with the advantage that they can be copied & pasted between code and docs and vice versa for (some level of) compiler-checked guarantees of accuracy. (Maybe even automatically in the code->docs direction, so they never get out of date, though that would mean giving the docs repo compilation step access to the realtime repo). I think those satisfy all of @QuintinWillison's requirements (prop?: .. for optional properties; nothing is nullable without an explicit ... | null, doesn't have nonempty-list etc. built in but easily added with eg type NonEmptyArray<T> = [T, ...T[]];, builtin readonly modifier, ...)

{
  id?: string;
  status?: Status;
  tlsOnly?: boolean;
   ...
}

@paddybyers
Copy link
Member

https://heycam.github.io/webidl/ was an attempt at modelling something sufficiently generally that you could define APIs and have multiple language bindings implied by the IDL. However, the java binding has since been deleted from the spec because, in the process of making it rich enough to describe any javascript API, they created features that were simply not possible in multiple other languages.

But it would do what we need here I think.

@mattheworiordan
Copy link
Member

Urr, I think we're going about this the wrong way - this is just an API endpoint.

We provide an administrative API endpoint that is used solely by the client SDKs, and it returns a body that has a well defined schema. So isn't this simply an Open API spec (like our REST API spec) and perhaps a JSON Schema for the body content.

Then everyone can use this agnostic of language.

WDYT?

@paddybyers
Copy link
Member

See #47

@sync-by-unito
Copy link

sync-by-unito bot commented Oct 17, 2022

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-2790

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

No branches or pull requests

6 participants