Skip to content
This repository has been archived by the owner on Dec 5, 2024. It is now read-only.

Commit

Permalink
Improved the sturdyness of repository deletion (#3)
Browse files Browse the repository at this point in the history
Refactoring of the extension, changing from a inheritance based structure, to a composition based one. This helps a lot with testing, and makes the code easier to understand, and it didn't changed any public interfaces, so it is entirely retro compatible.

The main points of the refactor are:

    Repository creation behavior isolated in the RepositoryFactory class
    The GithubRepository/SharedGithubRepository/GithubRepositoryFeature interceptors now inherits directly from AbstractMethodInterceptor.
    The underlying operations on Field-based interceptors are now in the RepositoryFieldOperations which is a slightly-changed, and has similar methods to the old GithubRepositoryFieldInterceptor.
    The slightly more complex object creation process is isolated in static factory methods in the interceptors.

Besides there were several some in the deletion code to make it more sturdy, the main ones being:

    deletions now are retried in case of non FileNotFoundException exceptions, and also checks if the repository still exists before trying deletion
    Catch blocks that ignored Exception exceptions in the deletion process, now are only ignoring FileNotFoundException (404) exceptions, to make more clear which errors are happening.
    Added tests for the specific situation of creating new repositories with the same name of existing ones.

The refactoring is isolated in the refactor/interceptors branch, for ease of comparison, and if so wished, a new PR with only the refactored code can be open as well.
Co-authored-by: Joaquim Neto <[email protected]>
  • Loading branch information
Joaquimmnetto authored Jul 13, 2021
1 parent 276aa53 commit f657579
Show file tree
Hide file tree
Showing 10 changed files with 327 additions and 237 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

package com.wooga.spock.extensions.github


import com.wooga.spock.extensions.github.interceptor.FieldInterceptor
import com.wooga.spock.extensions.github.interceptor.GithubRepositoryFeatureInterceptor
import com.wooga.spock.extensions.github.interceptor.GithubRepositoryInterceptor
import com.wooga.spock.extensions.github.interceptor.SharedGithubRepositoryInterceptor
Expand All @@ -27,23 +27,18 @@ import org.spockframework.runtime.model.FieldInfo

class GithubRepositoryExtension extends AbstractAnnotationDrivenExtension<GithubRepository> {


@Override
void visitFeatureAnnotation(GithubRepository annotation, FeatureInfo feature) {
def interceptor

interceptor = new GithubRepositoryFeatureInterceptor(annotation)
def interceptor = GithubRepositoryFeatureInterceptor.withMetadata(annotation)
interceptor.install(feature)
}

@Override
void visitFieldAnnotation(GithubRepository annotation, FieldInfo field) {
def interceptor

if (field.isShared()) {
interceptor = new SharedGithubRepositoryInterceptor(annotation)
} else {
interceptor = new GithubRepositoryInterceptor(annotation)
}
FieldInterceptor interceptor = field.isShared()?
SharedGithubRepositoryInterceptor.withMetadata(annotation, field) :
GithubRepositoryInterceptor.withMetadata(annotation, field)

interceptor.install(field)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Repository {

private interface GHRepositoryDelegateExcludes {
String getDefaultBranch()
void delete()
}

@Delegate(excludeTypes = [GHRepositoryDelegateExcludes.class])
Expand Down Expand Up @@ -61,9 +62,9 @@ class Repository {

boolean exists() {
try {
client.getRepository(fullName)
client.getRepositoryById(repository.id)
return true
} catch (ignored) {
} catch (FileNotFoundException) {
return false
}
}
Expand Down Expand Up @@ -129,6 +130,23 @@ class Repository {
findTag(tagName).orElse(null)
}

void delete(int retries=3) {
try {
if(this.exists()) {
repository.delete()
}
} catch (FileNotFoundException _) {
} catch (Exception e) {
if(retries > 0) {
sleep(500)
delete(retries-1)
} else {
throw e
}
}

}

Optional<GHTag> findTag(String tagName) {
//there is no other way at the moment to load a GHTag
Optional.ofNullable(listTags().find { it.name == tagName } as GHTag)
Expand Down Expand Up @@ -235,6 +253,8 @@ class Repository {
}
}



private static void tryToDelete(GHRef ref) {
try {
ref.delete()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,159 +1,142 @@
/*
* Copyright 2019 Wooga GmbH
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

package com.wooga.spock.extensions.github.interceptor

import com.wooga.spock.extensions.github.GithubRepository
import com.wooga.spock.extensions.github.Repository
import com.wooga.spock.extensions.github.api.RepositoryPostFix
import groovy.transform.InheritConstructors
import org.kohsuke.github.GHRepository
import org.kohsuke.github.GitHub
import org.kohsuke.github.GitHubBuilder
import org.spockframework.runtime.extension.AbstractMethodInterceptor
import org.spockframework.runtime.extension.IMethodInvocation
import org.spockframework.runtime.model.NodeInfo

@InheritConstructors
abstract class GithubRepositoryManagingInterceptor<T extends NodeInfo> extends AbstractMethodInterceptor {

protected final GithubRepository metadata

private GitHub client
private String repositoryName
private String repositoryOwner
private String token
private T info

T getInfo() {
this.info
}

GithubRepositoryManagingInterceptor(GithubRepository metadata) {
super()
this.metadata = metadata
}

void install(T info) {
this.info = info
}

void maybeDelete(String repoName) {
try {
def repository = getClient().getRepository(repoName)
repository.delete()
}
catch (Exception e) {
}
}

String getRepositoryBaseName() {
info.name.replaceAll(/\s|\[|\]|\(|\)|\{|\}/, '-')
}

String getRepositoryName() {
if (!repositoryName) {

String prefix = metadata.repositoryNamePrefix()
List<RepositoryPostFix> postFixProvider = metadata.repositoryPostFixProvider().collect {
it.getDeclaredConstructor().newInstance()
} as List<RepositoryPostFix>

String postfix = postFixProvider.inject("") { String postFix, provider ->
String post = provider.postFix
if (post && !post.empty) {
postFix += "-${provider.postFix}"
}
postFix
}

repositoryName = "${getRepositoryBaseName()}${postfix}"

if(prefix) {
repositoryName = "${prefix}-${repositoryName}"
}
}

repositoryName
}

String getRepositoryFullName() {
"${getRepositoryOwner()}/${getRepositoryName()}"
}

String getRepositoryOwner() {
if (!repositoryOwner) {
repositoryOwner = System.getenv(metadata.usernameEnv())
}

repositoryOwner
}

String getToken() {
if (!token) {
token = System.getenv(metadata.tokenEnv())
}

token
}

GitHub getClient() {
if (!client) {
def builder = new GitHubBuilder()
client = builder.withOAuthToken(System.getenv()[metadata.tokenEnv()])
.withEndpoint(metadata.endpoint())
.withRateLimitHandler(metadata.rateLimitHandler().getDeclaredConstructor().newInstance())
.build()
}

client
}

Repository setupRepository(IMethodInvocation invocation) {
maybeDelete(getRepositoryFullName())

//create github repo
def builder = getClient().createRepository(getRepositoryName())
builder.description(metadata.repositoryDescription())
builder.autoInit(metadata.autoInit())

if (metadata.autoInit()) {
builder.licenseTemplate(metadata.licenseTemplate())
if (metadata.gitignoreTemplate()) {
builder.gitignoreTemplate(metadata.gitignoreTemplate())
}
}

builder.private_(metadata.createPrivateRepository())
builder.issues(metadata.setupIssues())
builder.wiki(metadata.setupWiki())
builder.allowSquashMerge(metadata.allowSquashMerge())
builder.allowRebaseMerge(metadata.allowRebaseMerge())
builder.allowMergeCommit(metadata.allowMergeCommit())
builder.downloads(metadata.enableDownloads())


GHRepository repository = builder.create()
new Repository(repository, getClient(), getRepositoryOwner(), getToken())
}

abstract void destroyRepository(IMethodInvocation invocation)

abstract void resetRepository(IMethodInvocation invocation)

abstract void captureResetRef(IMethodInvocation invocation)
/*
* Copyright 2019 Wooga GmbH
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

package com.wooga.spock.extensions.github


import com.wooga.spock.extensions.github.api.RepositoryPostFix
import groovy.transform.InheritConstructors
import org.kohsuke.github.GHRepository
import org.kohsuke.github.GitHub
import org.kohsuke.github.GitHubBuilder
import org.spockframework.runtime.model.NodeInfo

@InheritConstructors
class RepositoryFactory {

protected final GithubRepository metadata

private GitHub client
private String repositoryOwner
private String token

static String getRepositoryBaseName(NodeInfo info) {
info.name.replaceAll(/\s|\[|\]|\(|\)|\{|\}/, '-')
}

RepositoryFactory(GithubRepository metadata) {
this.metadata = metadata
}

Repository setupRepository(String repositoryBaseName) {
def repositoryName = createRepositoryName(repositoryBaseName)
deleteRepoIfExists(getRepositoryFullName(repositoryName))

//create github repo
def builder = getClient().createRepository(repositoryName)
builder.description(metadata.repositoryDescription())
builder.autoInit(metadata.autoInit())

if (metadata.autoInit()) {
builder.licenseTemplate(metadata.licenseTemplate())
if (metadata.gitignoreTemplate()) {
builder.gitignoreTemplate(metadata.gitignoreTemplate())
}
}

builder.private_(metadata.createPrivateRepository())
builder.issues(metadata.setupIssues())
builder.wiki(metadata.setupWiki())
builder.allowSquashMerge(metadata.allowSquashMerge())
builder.allowRebaseMerge(metadata.allowRebaseMerge())
builder.allowMergeCommit(metadata.allowMergeCommit())
builder.downloads(metadata.enableDownloads())


GHRepository repository = builder.create()
new Repository(repository, getClient(), getRepositoryOwner(), getToken())
}

protected void deleteRepoIfExists(String repoName) {
try {
def repository = getClient().getRepository(repoName)
repository?.delete()
}
catch (FileNotFoundException e) {
}
}

protected String createRepositoryName(String repositoryBaseName) {
String prefix = metadata.repositoryNamePrefix()
List<RepositoryPostFix> postFixProvider = metadata.repositoryPostFixProvider().collect {
it.getDeclaredConstructor().newInstance()
} as List<RepositoryPostFix>

String postfix = postFixProvider.inject("") { String postFix, provider ->
String post = provider.postFix
if (post && !post.empty) {
postFix += "-${provider.postFix}"
}
return postFix
}

def repositoryName = "${repositoryBaseName}${postfix}"
if(prefix) {
repositoryName = "${prefix}-${repositoryName}"
}
return repositoryName
}

protected String getRepositoryFullName(String repositoryName) {
return "${getRepositoryOwner()}/${repositoryName}"
}

protected String getRepositoryOwner() {
if (!repositoryOwner) {
repositoryOwner = getClient().myself.login
}
repositoryOwner
}

protected String getToken() {
if (!token) {
token = mustGetEnvVar(metadata.tokenEnv())
}
token
}

protected GitHub getClient() {
if (!client) {
def builder = new GitHubBuilder()
client = builder.withOAuthToken(getToken())
.withEndpoint(metadata.endpoint())
.withRateLimitHandler(metadata.rateLimitHandler().getDeclaredConstructor().newInstance())
.build()
}
return client
}

private static String mustGetEnvVar(String envVar){
def value = System.getenv(envVar)
if (!value) {
throw new IllegalArgumentException("Couldn't find environment variable ${envVar}")
}
return value
}


}
Loading

0 comments on commit f657579

Please sign in to comment.