Skip to content

Instantly share code, notes, and snippets.

@baymac
Last active November 30, 2020 08:09
Show Gist options
  • Save baymac/deb8a7b5a8b62b3c72d0e2d434e31757 to your computer and use it in GitHub Desktop.
Save baymac/deb8a7b5a8b62b3c72d0e2d434e31757 to your computer and use it in GitHub Desktop.
Notes for GitLab Branch Source Development

GitLabSCMSource

  • Inside getRemote() method is the set variables in the uri template okay?

  • In retrieve(head,listener) using a different version of getBranch(project, head.getName()) and not supplying username like gitea plugin. Recheck if it still works.

  • Unable to get sha of gitlab mr head. No api call for that. So used mr.getSha() instead of mr.getHead().getSha().

^ Fixed it by getting the head sha by mr.getDiffRef().getHeadSha().

  • See how head is populated in retrieve(head,listener).

  • Used enum for checking if mr is opened. Check if it is working.

^ It is working.

  • Check the path of project field. It has to be of the format {username}/{project name}.

  • In retrieve(criteria, observer, event, listener) method, there is a need to check if the project is forked from some other repository but that can also be tested if GitLabApi is authenticated. Without authentication all forked projects return null for the forkedFromProject field.

^ In Gitea:

if (request.isFetchPRs()) {
    if (giteaRepository.isMirror()) {
        listener.getLogger().format("%n  Ignoring pull requests as repository is a mirror...%n");
    } else {
        request.setPullRequests(c.fetchPullRequests(giteaRepository));
    }
}

Fixed it by:

if (request.isFetchMRs()) {
    // If not authenticated GitLabApi cannot detect if it is a fork
    // If `forkedFromProject` is null it doesn't mean anything
    if (gitlabProject.getForkedFromProject() == null) {
        listener.getLogger()
                .format("%n  Unable to detect if it is a mirror or not still fetching MRs anyway...%n");
        request.setMergeRequests(gitLabApi.getMergeRequestApi().getMergeRequests(project));
    }
    else {
        listener.getLogger().format("%n  Ignoring merge requests as project is a mirror...%n");
    }
} 
  • Extra check if PAT is supplied to GitLabApi
// To verify the supplied PAT is valid
if(!gitLabApi.getAuthToken().equals("")) {
    gitLabApi.getUserApi().getCurrentUser();
}
  • See logs if this works: (line ~184)
listener.getLogger().format("%n Checking branch %s%n",
        HyperlinkNote.encodeTo(
                UriTemplate.buildFromTemplate(gitlabProject.getHttpUrlToRepo())
                        .literal("/blob")
                        .build()
                        .set("branch", b.getName())
                        .expand(),
                b.getName()
        )
);
  • Origin project name will always the same as the source project name. So keeping originProject = project in merge request strategy(line ~260)

  • While creating a new BranchSCMHead for the MR, using m.getSourceBranch() as the constructor param. (line ~270). Same in case of origin using m.getTargetBranch().

  • DiffRefs doesn't populate for a list of MergeRequests:

Previously

for(final MergeRequest m : gitLabApi.getMergeRequestApi().getMergeRequests(project, Constants.MergeRequestState.OPENED)) {
    ...
}

After

List<MergeRequest> mrs = gitLabApi.getMergeRequestApi().getMergeRequests(project, Constants.MergeRequestState.OPENED);
for (MergeRequest mr : mrs) {
    // Since by default GitLab4j do not populate DiffRefs for a list of Merge Requests
    // It is required to get the individual diffRef using the Iid.
    final MergeRequest m = gitLabApi.getMergeRequestApi().getMergeRequest(project, mr.getIid());
    ...
}
  • Understand what is retrieve(thingName, listener) method for.

  • Cannot call diffRefs on request since it will always return null.

  • Should use "tree"or "blob"?

  • What should be the urls for the icons in the DescriptorImpl?

GitLabSCMBuilder

  • Should I removed api/v4 or api/v3 from url end?
  • Does MR refspec work?
  • What is the use of checkoutUriTemplate method? Does sshUserPrivateKey work as expected?
  • What does and GitLabSCMBuilder and build method do?

MergeRequestSCMHead

  • See if it can be improved by doing something similar to Argelbargel's GBS

  • What is refSpec?

https://git-scm.com/book/en/v2/Git-Internals-The-Refspec https://stackoverflow.com/questions/44333437/git-what-is-refspec

BranchSCMHead

GitLabBrowser

  • Does all methods work as expected - diffLink, getFileLink, getDiffLink, getChangeSetLink?
  • Modifed the set for diff method.

MergeWithGitSCMExtension

  • What work does it do?

MergeRequestRevision

BranchSCMRevision

GitLabSCMSourceContext

GitLabSCMSourceRequest

  • See how add the counterpart of GiteaConnection as a member variable. Using GitLabApi for now.

  • Add support for TAGs

WebHookRegistration - Enum

GitLabSCMFileSystem

  • Need to make lastModified() work. It returns a long.

  • BuilderImpl requires to implement 2 new methods supportsDescriptor(scmDescriptor) & supportsDescriptor(scmSourceDescriptor). Need to see what these methods are required for.

GitLabSCMFile

  • Need to make children() method work. It returns an Iterable<SCMFile.

  • Need to make lastModified() work. It returns a long.

  • In overrided method type() trying to get file, throws error is not file. So there is no point of checking if it is a file. Fixed by returning nonexistent type in the catch block.

  • In visitSource(observer) method

GitLabSCMNavigatorContext

GitLabWebhookListener

  • Why is localconfiguration always null?

^ Because it is marked nonnull. Ask Joseph what to do about this. For now just skipped checking for null.

  • When you get GitLab gold account, test the Group webhooks and Project webhooks.

  • GitLab API doesn't support api calls on Group webhooks. See gitlab4j/gitlab4j-api#393

// Since GitLab doesn't allow API calls on Group WebHooks. 
// So fetching a list of web hooks in individual projects inside the group
List<ProjectHook> projectHooks = new ArrayList<>();
for(Project p : gitLabGroup.getProjects()) {
    gitLabApi.getProjectApi().getHooksStream(p).forEach(projectHooks::add);
}
  • When create webhook provide these functionality add secret token(see gitlab plugin), add more events give option for sslVerification

  • In register(owner, navigator, mode, credentialsId) method check if webhook on USER projects can be supported. Right now we are returning if gitlabOwner is a USER.

GitLabSCMNavigator

  • Unable to fetch GitLab Owner (User or Group) in one member variable. So using an enum instead.
private GitLabOwner fetchOwner(GitLabApi gitLabApi) {
    try {
        Group group = gitLabApi.getGroupApi().getGroup(projectOwner);
        return GitLabOwner.GROUP;
    } catch (GitLabApiException e) {
        try {
            User user = gitLabApi.getUserApi().getUser(projectOwner);
            return GitLabOwner.USER;
        } catch (GitLabApiException e1) {
            e1.printStackTrace();
        }
        e.printStackTrace();
    }
    return null;
}

Now moved fetchOwner(gitLabApi, projectOwner) to GitLabOwner enum.

  • In visitSource(observer) method, if projectOwner is a subgroup, it will only return projects in the subgroup and its subgroups. And if the projectOwner is a user, it will also return the projects in the groups and subgroups owned by user.

  • Find if there can be a better data structure for gitlabOwner

  • In visitSource(observer) method, using the following way to to skip user's group owned projects

if(gitlabOwner == GitLabOwner.USER && p.getNamespace().getKind().equals("group")) {
    // skip the user repos which includes all organizations that they are a member of
    continue;
}
  • In visitSource(observer) method, find if there can be an api call to check the project is empty. But now I am using a getTree method on the project and it throws error if the repository is empty.
try {
    gitLabApi.getRepositoryApi().getTree(p);
} catch (GitLabApiException e) {
    observer.getListener().getLogger().format("%n    Ignoring empty repository %s%n",
            HyperlinkNote.encodeTo(p.getWebUrl(), p.getName()));
    continue;
}
  • Printing User website Url instead of Group website url. Since GitLab doesn't have website for groups. But unfortunately this is not working our api. So API needs to be fixed.
if (gitlabOwner == GitLabOwner.USER) {
    String website = null;
    try {
        // This is a hack since getting a user via username finds user from a list of users
        // and list of users contain limited info about users which doesn't include website url
        User user = gitLabApi.getUserApi().getUser(projectOwner);
        website = gitLabApi.getUserApi().getUser(user.getId()).getWebsiteUrl();
    } catch (GitLabApiException e) {
        e.printStackTrace();
    }
    if (StringUtils.isBlank(website)) {
        listener.getLogger().println("User Website URL: unspecified");
        listener.getLogger().printf("User Website URL: %s%n",
                HyperlinkNote.encodeTo(website, StringUtils.defaultIfBlank(fullName, website)));
    }
}   
  • Modernize doFillServerUrlItems(context, serverUrl) jenkins permission checks. Need to be checked elsewhere as well.

  • setTraits(List) clashing with the parent class. Can you override a @DataboundSetter method? Fixed it by using GitHub BS Impl.

@DataBoundSetter
public void setTraits(@CheckForNull SCMTrait[] traits) {
    this.traits = new ArrayList<>();
    if (traits != null) {
        for (SCMTrait trait : traits) {
            this.traits.add(trait);
        }
    }
}
@Override
public void setTraits(@CheckForNull List<SCMTrait<? extends SCMTrait<?>>> traits) {
    this.traits = traits != null ? new ArrayList<>(traits) : new ArrayList<SCMTrait<? extends SCMTrait<?>>>();

}

GitLabSCMSourceBuilder

helpers package

^ Contains classes that do not contribute directly to the plugin.

GitLabOwner enum

  • Added fetchOwner(gitLabApi, projectOwner) as a static method.

GitLabAvatar

GitLabAvatarCache

BranchDiscoveryTrait

OriginMergeRequestDiscoveryTrait

ForkMergeRequestDiscoveryTrait

SSHCheckoutTrait

GitLabNotifier

GitLabMergeRequestSCMEvent

GitLabPushSCMEvent

AbstractGitLabSCMHeadEvent

  • Talk to oleg about forcing dependency (see: baymac/gitlab-branch-source-plugin#19)

  • Joseph asked to add support for subgroups. Where rn I am only doing Check the path of project field. It has to be of the format {username}/{project name}.

  • Fix Maven Checkstyle errors

  • Joseph's comment: I feel like your over using UriTemplate's build feature, you should use the build feature to provide you with the template strings: If you see this PR: https://github.com/jenkinsci/bitbucket-branch-source-plugin/pull/80/files at no point did I use UriTemplate.build apart from a single test.

@amit-gueta
Copy link

Hi,
why do we want to "Ignoring merge requests as project is a mirror"?
Can I disable it from ignoring once the repo is a fork? if I can, how?
Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment