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

Class/method stability for fluent APIs #24

Open
bannmann opened this issue Nov 22, 2021 · 3 comments · May be fixed by #26
Open

Class/method stability for fluent APIs #24

bannmann opened this issue Nov 22, 2021 · 3 comments · May be fixed by #26

Comments

@bannmann
Copy link

Some fluent APIs use multiple intermediate types to restrict the available methods depending on what methods were called earlier in the call chain.

As an illustration, imagine a fictitious library that models SQL as Java method call chains:

// Compiles fine
Result result = db.select().from(table).where(criterion1).and(criterion2).execute();

// COMPILER ERROR: cannot find symbol and() on type Select1
Result result = db.select().from(table).and(criterion2).execute();

Adding new features to such APIs often causes changes in the state machine and thus re-numbering of intermediate types. Therefore, the library explicitly discourages users from defining variables such as Select2 select = db.select().from(table).where(criterion1);.

Consequently, in its SemVer documentation the library excludes the names of these intermediate types from the public API. However, it promises that any method chain that worked in one release will still work in a subsequent minor release (albeit with possibly different intermediate type names).

Could such a library use API Guardian? If so, how? To me, the closest match would be documenting the entry point (the one with select() with STABLE and the intermediate types (Select0, Select1 etc) with INTERNAL - but the methods on those intermediate types would be STABLE. I interpret this to mean "don't rely on this type name, but rest assured the method will stay available".

With this interpretation, I could (and intent to) use API Guardian for fluent APIs in puretemplate. However, that usage may be counterintuitive to users and even frowned upon by the API Guardian team because the Javadocs for @API say the following (emphasis mine):

If @API is present on a type, it is considered to hold for all public members of the type as well. However, a member of such an annotated type is allowed to declare a API.Status of lower stability.

My "INTERNAL class with STABLE methods" trick is the exact reverse of this.

So, what do you think of this? Please note that I'm fine with any answer; I just wanted to bring this to your attention.

Here are some alternatives I can imagine:

  1. We have no idea what you're talking about.
  2. We get it, but can't endorse this weird stuff.
  3. We will mention "INTERNAL class with STABLE methods" as an exception to the rule.
  4. We will change the "declare a API.Status of lower stability" wording to "declare a different API.Status", followed by two examples: one is the "with lower stability" case, the other is "INTERNAL class with STABLE methods, e.g. for fluent APIs".
  5. This is important, but INTERNAL on the class is wrong. We will introduce a special status called ANONYMOUS for this. (No, I'm not seriously suggesting that.)
@marcphilipp
Copy link
Member

Personally, I think we could replace "lower" with "different" in the documentation.

@sbrannen WDYT?

bannmann added a commit to bannmann/apiguardian that referenced this issue Dec 5, 2022
@bannmann bannmann linked a pull request Dec 5, 2022 that will close this issue
@bannmann
Copy link
Author

bannmann commented Dec 5, 2022

I created PR #26 mainly as a basis for discussion.

@marcphilipp, @sbrannen: comments welcome!

@sbrannen
Copy link
Member

Adding new features to such APIs often causes changes in the state machine and thus re-numbering of intermediate types. Therefore, the library explicitly discourages users from defining variables such as Select2 select = db.select().from(table).where(criterion1);.

Why does a concrete type show up in the middle of that chain if you don't actually support that type?

Based on my experience with API design, the types used in those intermediate "steps" should be supported types. If you want to hide an implementation type which may change, define the API based on stable interfaces.

That allows users to store an intermediate "step" in a variable with the type of the supported interface, while allowing implementers of the API to change the internals (implementation details) as they see fit.

For a concrete example of that approach, see the Stream APIs in the JDK.

In light of that, I don't really see a need to change the documentation for @API.

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

Successfully merging a pull request may close this issue.

3 participants