Azure / azure-sdk-tools

Tools repository leveraged by the Azure SDK team.
MIT License
114 stars 180 forks source link

[APIView] Consider Spring AutoConfiguration class as Non-public API #4773

Open stliu opened 1 year ago

stliu commented 1 year ago

Spring Cloud Azure has Spring Boot auto configuration support for various Azure SDK features. We do that by providing @Configuration class, it acts like a bean factory to the Spring IoC container, which means the auto configuration classes are only accessed by Spring IoC framework, not users directly.

But we can't put these into implementation package since customer may need to access the class name and class name only to disable any auto configuration class.

According to spring boot doc

Even though auto-configuration classes are public, the only aspect of the class that is considered public API is the name of the class which can be used for disabling the auto-configuration. The actual contents of those classes, such as nested configuration classes or bean methods are for internal use only and we do not recommend using those directly.

@SpringBootApplication(exclude = { DataSourceAutoConfiguration.class })
public class MyApplication {

}

For example, this https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/keyvault/secrets/AzureKeyVaultSecretAutoConfiguration.java auto configuration class, all the methods in it, are only accessed by Spring IoC, and not customer according to spring doc and conversion.

So, we'd like to have APIView tool aware of this spring feature and DO NOT consider any class with org.springframework.context.annotation.Configuration annotation as public API despite it's in implementation package or not.

JonathanGiles commented 1 year ago

I would propose using the new 'hidden API' feature that was added in #4618. We would then mark all of the API within the class as hidden, but retain the class as part of the public API scope that should be reviewed (to ensure correct naming and package placement). Let me know your thoughts based on the images you see in #4618.

stliu commented 1 year ago

We would then mark all of the API within the class as hidden, but retain the class as part of the public API scope that should be reviewed (to ensure correct naming and package placement).

this sounds nice, just confirm, once we have the public API (public method in the class) marked as hidden, change in the public API between two versions wont' block or raise any alert in apiview anymore?

JonathanGiles commented 1 year ago

No, I believe apiview will still treat it as API - it will just make reviews quicker

stliu commented 1 year ago

okay, let's give it a try and see how things go here. @saragluna could you mark those configuration classes as hidden API?

saragluna commented 1 year ago

Will do. @JonathanGiles is there an example I could follow to set up?

JonathanGiles commented 1 year ago

This is the first of its kind in two ways, and so we need to tread carefully.

For starters, we have never used the 'hidden' API feature yet, so that needs to be added in to the Java parser (but this is easily done). More importantly, we need to find a way of introducing 'custom' functionality like this, and ideally do it without coding it directly into the parser itself.

Ideally, we would do something pluggable, in a similar fashion to how diagnostics are implemented, so that our parser does not become Azure-specific. This is easier said than done though. I suspect we will need to introduce a pluggable API that is done around about here, so that each compilation unit can be inspected by plugins for certain properties (e.g. annotations), and if found, modify the tokenization.