✨ Get staging profile task

[ryandens]

Closes #60

Documentation Updates

I think we should add an update to the "Tasks" section of the migration guide to say something like

./gradlew getStagingProfile

↓

./gradlew retrieveSonatypeStagingProfile

This reflects the main difference between the previous plugin project and this one: tasks are on a per NexusRepsitory basis, including the RetrieveStagingProfile task.

Summary of changes

First adds a new AbstractNexusStagingRepositoryTask subclass called RetrieveStagingProfile. we register a new RetrieveStagingProfile task for each NexusRepository registered in the plugin's extension. This enables users to run
./gradlew retrieve<NEXUS_REPOSITORY_NAME>StagingProfile and see the staging profile ID for the repository in stdout.

This was initially as far as I was planning on going with this, but something @szpak mentioned in issue #60 caught my attention

Or maybe there should be also an @output field set to allow others to read it (but I'm not sure why anyone would like to consume that)

After reading this, it struck me that we wouldn't want there to be inconsistencies between how GetStagingProfile gets the staging profile and how InitializeNexusStagingRepository gets the staging profile. So, I went about a small refactor of the plugin to remove the responsibility of figuring out the staging profile id from InitializeNexusStagingRepository and instead make it a required input. Then, I configured InitializeNexusStagingRepository to depend on the GetStagingProfile which now has all the logic for getting the staging profile ID, outputting it to stdout, storing the ID in a file in the build directory, and managing the optimization of not doing the API lookup if the staging profile id is provided via configuration.

This not only has benefits for code re-use, but it also helps those who don't configure the stagingProfileId ever. Since we output the staging profile ID to an artifact, it can be cached and reused on subsequent builds which decreases the configuration burden on the user while still giving them access to this performance optimization.

I know changing InitializeNexusStagingRepository was not technically in the scope of what was discussed in #60 so if you would like me to remove it from the scope of this change I'd be happy to. But, it seemed to me like refactoring InitializeNexusStagingRepository was the responsible thing to do as I was introducing some logic duplication around how this plugin gets the staging profile id.

Update on 2/27/21: After discussion, it was deemed that this refactor was problematic due to the conflicting nature of the different use cases of the InitializeNexusStagingRepository task and the diagnostic task to retrieve the staging profile

Possible other tasks

Another thought that occurred to me was adding a task called getStagingProfiles or getAllStagingProfiles which is simply an alias for executing get<NEXUS_REPOSITORY_NAME>StagingProfile on each configured NexusRepository. I ultimately decided it was not worthwhile/might cause confusion but I'm willing to reconsider.

Testing gaps?

After writing up this pull request description, I realized my implementation for storing the staging repository ID was flawed, as I at first would overwrite the file storing the staging profile ID every time a new GetStagingProfile was run, which could lead to confusing results with multiple NexusRepository destinations. The existing tests didn't catch it because there were no tests that published to two different repositories at the same time. I had to make some tweaks to the helper methods to accomplish this test, but I'm super open to feedback on how this was tested. Perhaps it would be better as a unit test than a compatTest?

[szpak]

Really informative PR description, thanks!

I went through the code and in general I like it. Chaining tasks makes sense, but there is one side effect related to logging. I put some remarks inline.

I also requested e2e tests on your PR.

[marcphilipp]

Why don't we use the new task to retrieve the staging profile id for the initialize task?

[marcphilipp]

@szpak Not sure if we should use this annotation. WDYT?

[szpak]

Personally, I like to use @Incubating for the new tasks/features to gather feedback and possibly have a clear way to fix spotted issues even in a non-backward compatible way in the following release. However, you don't like the "beta" status of that feature or just a Gradle annotation?

[marcphilipp]

Ok, fine with me but we should have some common understanding when to do this. IIUC you'd like to do this for all new features. Is that correct?

[szpak]

It would be safe, but maybe it would be to defensive, as we - in general - do not expose API to others (at least, it is not the main goal of that extension). How does it look like in JUnit? Are you satisfied with the strategy adopted there?

[szpak]

Why don't we use the new task to retrieve the staging profile id for the initialize task?

@marcphilipp I asked the PR's author about that a comment earlier - see my reasoning. It simplifies the general construction and we eventually still use the common logic from the client.

[marcphilipp]

Ok, fine with me but we should have some common understanding when to do this. IIUC you'd like to do this for all new features. Is that correct?

[marcphilipp]

There's no need to add the test method to the namespace since the extension context is already scoped to the method level.

[ryandens]

Thanks! I couldn't remember the rules around this so I went looking in the user guide and derived it from this example: https://junit.org/junit5/docs/current/user-guide/#extensions-lifecycle-callbacks-before-after-execution - but fixed!

[marcphilipp]

Good point! That example should indeed be simplified.

[szpak]

Thanks @ryandens for your PR (and patience ;-) ). It will be a part of 1.1.0.

[ryandens]

Awesome, thanks for accepting my contribution and working through the feedback with me! Hoping this makes it easier for other folks to adopt this plugin 😄