ably
Define the public API and hide all non-public methods and fields
At the moment almost everything is public and accessible by SDK users. This means that the users have access even to the things which were not meant to be visible. If they start modifying them the SDK can go into unexpected states and cause problems. We should only expose the things that are meant to be public and nothing more.
As a reference to what should be public and what should not, we can use the feature spec (there's an IDL in there but it is not complete, unfortunately). Another source of truth may be the docs and tutorials or other SDK APIs (like ably-cocoa)
We should be careful to not modify the already existing APIs so the code changes probably should be limited to changes in fields and methods visibility (e.g. from public to private)
┆Issue is synchronized with this Jira Task by Unito
Hi @KacperKluka, I do not see any reference to what needs to be public or private in those documents. Also, how do we know that ably-cocoa is set up correctly?
If I'm not wrong then everything you find in those documents is meant to be public, since it is presented to the public (users). :thinking: On the other hand, everything that is not there is probably meant to be private implementation details of the ably-java SDK. I remember that @QuintinWillison was working on a document that would describe the complete API of an Ably SDK but I'm not sure if that's completed :thinking:
This is a very delicate piece of work and my expectation was that it would be somewhat exploratory in nature, so this question is very valid and not surprising to me. What I mean by that is that there's a level of discovery, based on customer use cases, to work out which APIs must (logically) need to be public in order for the SDK to deliver customer expectations at all.
I expect the document @KacperKluka describes is this rendered view, currently incubating, built by this codebase.
SDK interfaces which were intended to be public should all be defined in the spec, but the reality is that this is not always the case. I already found some examples in this SDK (ably-java / ably-android) of APIs which are not in the spec - see these comments (not necessarily exhaustive!).
We've also had a lot of work going on in our internal DevX/EdX pod, lead by @Srushtika, assisted by @lawrence-forooghian and @maratal, to enumerate and define canonical documentation for all of our APIs. That work might be helpful. That work is located here (private, likely-temporary repository) - I will send @qsdigor and @KacperKluka invites to that repository momentarily.
There's a lot of work going on in this area, all in a general effort to do better across the board at Ably in respect of having "canonical" origins for key, shared details of our features and their interfaces. While this work is in progress, unfortunately we're going to have to be creative in bridging the gaps.
FYI, @stmoreau @deanna-lad
@QuintinWillison Making things hidden is a lot harder than originally thought. Any class that has public modifier cannot be converted to package private as classes are used in different packages internally. Also, those classes can only hide a very small amount of their methods which is not very helpful.
One solution described here is to make a public interface for every class, this will complicate implementation even further and will cause execution issues. I do not recommend it as it is better to use default visibility modifiers than to do hacks like this.
The second solution is to convert Java SDK to Kotlin which has an internal modifier. The internal modifier is new in Kotlin and would be ideal for our case as classes would be package visible but not outside our SDK. If we go for Kotlin, the Java internal modifier becomes a public modifier. There is @JvmSynthetic annotation which works well but won't hide classes themselves. We can solve this by changing the name for java only which prefixes space in the class name, as this is an illegal java identifier and won't be visible in java. @JvmName(" example") internal fun example() { }
The third option is to use Java 9 module declarations. We will need to upgrade our SDK to java 9 and implement module rules.
The fourth option is to continue with java and just hide the methods we can which is not a lot. This is the simplest solution but it is not what we wanted to achieve in the first place.
There's additional discussion around this in this Slack thread (internal), from which I quote my response to @qsdigor around this:
As far as I recall, from my discussions with @KacperKluka earlier in the year, we don't have to move to Java 9 in order to take a more modular approach for encapsulation purposes. We should be able to stick with Java (definitely not wholesale convert to Kotlin) and still be able to split the codebase into more projects/modules - in a similar way to the approach taken with AAT's Gradle structure - core vs common, etc.. https://github.com/ably/ably-asset-tracking-android/blob/main/settings.gradle
I've also suggested that we have a synchronous conversation (call) to discuss this further as the choices we make here will have a large impact on how this SDK evolves going forwards.