Simplify PluginInfo.getLastRelease

[janhoy]

https://github.com/pf4j/pf4j-update/blob/2be901dcf4d4b2057c87302134df3660b3c24e37/src/main/java/org/pf4j/update/PluginInfo.java#L54

This API has changed to require PluginVersionManager. It makes the API cumbersome to use whenever standard versioning is used.

Can't we assume that when integrating PF4J into a product, we'll stick to one versioning style across all plugins? In that case it makes more sense to let PluginInfo use UpdateManager's VersionManager?

[decebals]

The API break was necessary because the initial sem Version (com.github.zafarkhaja.semver.Version) is hidden now by a concrete VersionManager implementation. VersionManager is an abstraction that allows you to use other versioning schema instead of semver (that is the default versioning schema in pf4j). Sure, we want the same versioning schema (VersionManager) per application (PF4J instance). The solution proposed by me is to inject VersionManager in the PluginRelease methods where I need it. If you have other better solution I am glad to find out.

[janhoy]

Guess since PluginInfo is Serializable, it is not as simple as making UpdateManager's VersionManager public static and refer to it. So I guess the consumer of the API will need to inject VersionManager twice - once when creating PluginManager and once when checking PluginInfo...

[decebals]

The PluginInfo class is used in DefaultUpdateRepository, to deserialize information from JSON:

PluginInfo[] items = gson.fromJson(pluginsJsonReader, PluginInfo[].class);

My idea is to keep PluginInfo anemic (only properties) and to move the logic part (getLastRelease and hasUpdate methods) in UpdateManager where we have already an instance of VersionManger:

PluginRelease getLastRelease(String pluginId) { ... }

boolean hasUpdate(String pluginId) { ... }

[decebals]

@janhoy What you think? If you are OK with this solution I can summit a PR.

[janhoy]

I kind of like the pure object orientation (cohesion) of asking a class questions about its own data.

Could we instead require setVersionManager() to be called before allowing use of the methods requiring version? The framework can do this automaticalky after deserializing from json. Simply hide this as an implementation detail and shield the API users from having to think about it.

[decebals]

Show me some code and I will make a review. To call manually setVersionManager before I call getLasRelease is not intuitive (my opinion) but you say that the framework can do it automatically. We will see.

[janhoy]

I tried a solution but ended up declaring UpdateManager's versionManager as static which is not ideal. So feel free to proceed with your plan of pulling methods out into UpdateManager.