[META] Java Annotations for BWC and API enforcement #8127
Comments
nknize
added
enhancement
nknize
changed the title
|
@nknize do you mind if I pick it up? I think we could start from the Plugin APIs (those are by definition public APIs) and go up the dependency graph to build the complete picture. Sounds like a plan? |
|
Thank you for taking this one @reta! It will be super helpful! |
|
Had a time to give it some thoughts, I envision this feature as a 4 step process:
Define the initial set of annotations, their meaning and relations between themAs @nknize suggested, I think it would make sense to start with this 4 annotations (please see their description for the guarantees): /**
* Stable public APIs that retain source and binary compatibility within a minor release.
* These interfaces can change from one major release to another major release
* (e.g. from 1.0 to 2.0). The types marked with this annotations could only expose
* other {@link PublicApi} or {@link ExperimentalApi} types as public members.
*/
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface PublicApi {
}/**
* Experimental APIs that may not retain source and binary compatibility within major,
* minor or patch releases. The types marked with this annotations could only expose
* other {@link PublicApi} or {@link ExperimentalApi} types as public members.
*/
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface ExperimentalApi {
}/**
* Internal APIs that have no compatibility guarantees and should not be used outside
* of OpenSearch core components.
*/
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface InternalApi {
}
/**
* Marks the public APIs as deprecated and scheduled for removal in one of the upcoming
* major release. The types marked with this annotations could only be other {@link PublicApi}s.
*/
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface DeprecatedApi {
String since();
String forRemoval();
}Alternative arrangement under public final class Api {
private Api() {
}
/**
* Experimental APIs that may not retain source and binary compatibility within major,
* minor or patch releases. The types marked with this annotations could only expose
* other {@link Public} or {@link Experimental} types as public members.
*/
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface Experimental {
}
/**
* Internal APIs that have no compatibility guarantees and should be not used outside
* of OpenSearch core components.
*/
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface Internal {
}
/**
* Stable public APIs that retain source and binary compatibility within a major release.
* These interfaces can change from one major release to another major release
* (e.g. from 1.0 to 2.0). The types marked with this annotations could only expose
* other {@link Public} or {@link Experimental} types as public members.
*/
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface Public {
}
/**
* Marks the public APIs as deprecated and scheduled for removal in one of the upcoming
* major release. The types marked with this annotations could only be other {@link Public}s.
*/
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface Deprecated {
String since();
String forRemoval();
}
}Decorate the existing APIs with proper annotationsThe natural starting point is Introduce checks for enforcing the API restrictionsIdeally, the public API should never expose anything that is internal. To enforce that (or at least to understand the limits of how far we can go), we have to introspect each type annotated with
Introduce checks for enforcing the API backward compatibilityThis is the final goal, once we have the API outlined (annotated with |
|
@reta Thanks for digging in to this, really like the approach, couple of standouts, happy to move these comments to another issue if you are going to put together an RFC(s).
|
|
One more:
|
|
@reta What will this look like in a post-JPMS world? If my understanding is correct, everything exported by a JPMS module would be implicitly an |
Thanks @andrross , yes, the |
Thanks @peternied
I think embedding everything into the name is too much (to me those names look very overloaded). For example, the
Till JPMS, we have only Java visibility rules to control the API exports. Basically everything which is public but should not be so (there are literally thousands of such classes).
No sure I get that, sorry, the Api which is not ready is
That should be within major versions only, we document that (preferably deprecate one major version before if it makes sense), and go with the change. |
Thanks, @peternied, the Extensions is experimental feature [1] (as of latest release at least), we may not gain much from it by annotating everything with [1] https://opensearch.org/blog/introducing-extensions-for-opensearch/ |
I mean this in the context of new functionality is being built out or when we first do the 'classification' pass. Are there ways that we will guard against just marking everything While that would be fast and easy to mark everything Maybe this is more of a philosophical problem, if we don't have rules governing how to know if public should be a public API is it might be easy to get stuck into the same problem space we have today. Potential rules:
|
|
We might also be missing a category for deprecation, reusing the existing annotation should work, but we'd also want the rules to prevent picking up deprecated components. |
That's should be done by feature implementors I think (besides that, all other checks would guard against exposing the API inappropriately).
There are some shady APIs for sure, that is why the suggestion is to start with what we know (plugins, 100% public) and I am sure, more surprises would be on the way.
This is (to me) not very clear, the deprecation comes to public APIs which means that those API will be using deprecated components for some time. One of the options I see - introduce new annotation |
reta
changed the title
Is your feature request related to a problem? Please describe.
We provide
@opensearch.internal@opensearch.experimentaland@opensearch.apito define code that provides a certain set of BWC guarantees. The problem is, this isn't enforceable without maintainers reviewing and manually rejecting PRs.Describe the solution you'd like
Similar to Apache Spark's annotations, OpenSearch should introduce a new
o.o.core.annotationpackage that (at minimum) defines enforceable java annotations as follows:@Internal@Api@ExperimentalAnd provides a mechanism to define new annotations.
Describe alternatives you've considered
We're currently using javadoc annotations. Let's make these more enforceable through java
public @interface Api {}.The text was updated successfully, but these errors were encountered: