[Rev 01] RFR: 381: Retry failed materialization of hosted repository storage

[Robin Westberg]

On Thu, 7 May 2020 11:42:13 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Robin Westberg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Updated after review
>
> Had to look at this a bit since the control flow is a bit confusing with the early `return` from the constructor. How
> about putting the retry logic into a separate helper method? This is what I came up with:
> HostedRepositoryStorage(HostedRepository repository, Path localStorage, String ref, String fileName, String authorName,
> String authorEmail, String message, StorageSerializer<T> serializer, StorageDeserializer<T> deserializer) {
>     this.hostedRepository = repository;
>     this.ref = ref;
>     this.fileName = fileName;
>     this.authorEmail = authorEmail;
>     this.authorName = authorName;
>     this.message = message;
>     this.serializer = serializer;
>     this.deserializer = deserializer;
> 
>     this.localRepository = tryMaterialize(repository, localStorage, ref, fileName, authorName, authorEmail, message);
>     repositoryStorage = new RepositoryStorage<>(localRepository, fileName, authorName, authorEmail, message, serializer,
>     deserializer); current = current();
> }
> 
> private static Repository tryMaterialize(HostedRepository repository, Path localStorage, String ref, String fileName,
> String authorName, String authorEmail, String message) {
>     int retryCount = 0;
>     IOException lastException = null;
> 
>     while (retryCount < 10) {
>         try {
>             try {
>                 return Repository.materialize(localStorage, repository.url(), "+" + ref + ":storage");
>             } catch (IOException ignored) {
>                 // The remote ref may not yet exist
>                 Repository localRepository = Repository.init(localStorage, repository.repositoryType());
>                 var storage = Files.writeString(localStorage.resolve(fileName), "");
>                 localRepository.add(storage);
>                 var firstCommit = localRepository.commit(message, authorName, authorEmail);
> 
>                 // If the materialization failed for any other reason than the remote ref not existing, this will fail
>                 localRepository.push(firstCommit, repository.url(), ref);
>                 return localRepository;
>             }
>         } catch (IOException e) {
>             lastException = e;
>         }
>         retryCount++;
>     }
>     throw new UncheckedIOException("Retry count exceeded", lastException);
> }
> This would also allow keeping `localRepository` as a `final` field. What do you think?

Thanks for reviewing! I agree, your suggestion looks nicer, I'll apply it and push.

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

PR: https://git.openjdk.java.net/skara/pull/617