RFR: 2400: Avoid overwriting mbox archive

[Erik Joelsson]

On Mon, 18 Nov 2024 21:44:55 GMT, Zhao Song <zsong at openjdk.org> wrote:

> Sometimes, the ArchiveWorkItem fails to retrieve the current contents of an mbox file while processing a PR. When this happens, it will resend all emails. Erik pointed out that in GitLab, creating a new file uses a POST request, while updating an existing file uses PUT. Similarly, GitHub requires the current "SHA" to update a file.
> 
> The ArchiveWorkItem can determine whether it found existing content in the mbox archive and choose the right method accordingly. If it incorrectly fails to retrieve the existing content from GitLab or GitHub, it will attempt to create a new file. This attempt should fail, triggering a retry of the WorkItem, which would ideally succeed in retrieving the existing content.

forge/src/main/java/org/openjdk/skara/forge/HostedRepository.java line 111:

> 109:      * @param authorEmail Email of author and committer for commit
> 110:      */
> 111:     void writeFileContents(String filename, String content, Branch branch, String message, String authorName, String authorEmail, boolean createNewFile);

Please update the javadoc with the new parameter and make it clear what should happen when set to true or false. If set to true, the call will fail if the file exists. If set to false, the call will fail if the file does not exist.

forge/src/main/java/org/openjdk/skara/forge/github/GitHubRepository.java line 294:

> 292: 
> 293:     @Override
> 294:     public void writeFileContents(String filename, String content, Branch branch, String message, String authorName, String authorEmail, boolean createNewEmail) {

This variable name doesn't make sense here.

test/src/main/java/org/openjdk/skara/test/TestHostedRepository.java line 232:

> 230: 
> 231:     @Override
> 232:     public void writeFileContents(String filename, String content, Branch branch, String message, String authorName, String authorEmail, boolean createNewFile) {

This implementation should also enforce the new behavior so we catch any mistakes in testing.

-------------

PR Review Comment: https://git.openjdk.org/skara/pull/1696#discussion_r1847386217
PR Review Comment: https://git.openjdk.org/skara/pull/1696#discussion_r1847387062
PR Review Comment: https://git.openjdk.org/skara/pull/1696#discussion_r1847389068