Handy Uri Template #80

jetersen · 2017-11-09T00:18:11Z

Just initial work, to get some feedback

stephenc

Let’s see what switch the rest looks like

stephenc · 2017-11-09T00:51:31Z

pom.xml

@@ -120,6 +120,11 @@
      <scope>test</scope>
    </dependency>
    <dependency>
+      <groupId>com.damnhandy</groupId>
+      <artifactId>handy-uri-templates</artifactId>
+      <version>2.1.6</version>


We’ll need to either pull this into a library plugin or use maskClasses to shield ourselves from class conflicts

Looking at the deps: https://github.com/damnhandy/Handy-URI-Templates/blob/master/pom.xml#L473-L533
They are all optional or test scoped except for joda-time.
If we were to put into a library plugin, this is the way to go right: https://github.com/jenkinsci/apache-httpcomponents-client-4-api-plugin with some POM magic?

@Casz yes, thats the way to go about library plugin. joda-time libraries are used at other places too. It might cause conflicts.

@stephenc if possible, please advice if I should be doing something special against joda-time or any of the optional dependencies. https://github.com/casz/handy-uri-templates-2-api-plugin/blob/master/pom.xml My maven knowledge is limited

It they are optional, then they will not be transitive.

I think joda-time may need its own api plugin, in which case you then have to add a jenkins plugin dependency to your api plugin... fun times

@stephenc I give it a shot for the joda-time too :)

Joda-time should be safe as well. However I will gladly wrap it in a plugin :)

<dependencies> <dependency> <groupId>org.joda</groupId> <artifactId>joda-convert</artifactId> <version>1.2</version> <scope>compile</scope> <optional>true</optional> </dependency> <dependency> <groupId>junit</groupId> <artifactId>junit</artifactId> <version>3.8.2</version> <scope>test</scope> </dependency> </dependencies>

Currently waiting for approval: https://issues.jenkins-ci.org/browse/HOSTING-476

jetersen · 2017-11-15T07:37:24Z

I'll have the next bit up later today

jetersen · 2017-11-16T07:29:41Z

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiClient.java

+        String url = UriTemplate.fromTemplate(REPO_URL_TEMPLATE + "/hooks/{uuid}")
+                .set("owner", owner)
+                .set("repo", repositoryName)
+                .set("uuid", Util.rawEncode(hook.getUuid()))


Don't think i need encoding here as Handy Uri should handle it

Yep, remove the rawEncode as it’s {uuid}. You’d need to use raw encode if the template was {+uuid}

jetersen · 2017-11-16T07:36:14Z

...ain/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java

        Map<String,Object> content = JsonParser.mapper.readValue(response, new TypeReference<Map<String,Object>>(){});
        Map page = (Map) content.get("children");
        List<Map> values = (List<Map>) page.get("values");
        collectFileAndDirectories(parent, path, values, files);
        while (!(boolean)page.get("isLastPage")){
-            response = getRequest(url + String.format("&start=%s&limit=%s", start, 500));
+            start += (int) content.get("size");


We were never counting up the pages 😕
Here we are

bitbucket-branch-source-plugin/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java

Lines 725 to 729 in 0f6a106

while(!(boolean)content.get("isLastPage")){

start += (int) content.get("size");

response = getRequest(String.format(url+"&start=%s&limit=&s", start, 500));

content = collectLines(response, lines);

}

However not here

bitbucket-branch-source-plugin/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java

Lines 691 to 695 in 0f6a106

while (!(boolean)page.get("isLastPage")){

response = getRequest(String.format(url+"&start=%s&limit=%s", start, 500));

content = JsonParser.mapper.readValue(response, new TypeReference<Map<String,Object>>(){});

page = (Map) content.get("children");

}

Can you create a JIRA and assign yourself (or let me know the JIRA and i’ll Assign it to you)

Good catch

Jira issue created: https://issues.jenkins-ci.org/browse/JENKINS-48279

Hellspam · 2017-11-19T16:59:17Z

Can you maybe make a larger limit for the getBranches function while you are at it? Something like 100 maybe instead of the current (25 I think by default).

jetersen · 2017-12-13T09:31:55Z

I'll try and wrap it in a Jenkins plugin and put it up Jenkins repo tonight, and start the process of having it hosted 👍

jetersen · 2018-01-02T17:57:05Z

@stephenc rebased onto master and using snapshot while I wait for hosting 👍 Seems like we are getting rid of all the ugly string format and encoding. Could you please take a look, if I missed anything.

If you still think Yoda as a dependency to handy templates, needs to be wrapped. All the dependencies of Yodatime are optional or as test scope.

stephenc

@rsandell can you provide cross-review

stephenc · 2018-01-03T14:15:15Z

pom.xml

+    <dependency>
+      <groupId>org.jenkins-ci.plugins</groupId>
+      <artifactId>handy-uri-templates-2-api</artifactId>
+      <version>2.1.6-1.0-20180102.164938-1</version>


Merging blocked until this is released

it just got hosted, so I'll upload it tonight 👍

jetersen · 2018-01-03T15:49:12Z

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiClient.java

@@ -460,13 +505,17 @@ private BitbucketRepositoryHooks parsePaginatedRepositoryHooks(String response)
    @Override
    @CheckForNull
    public BitbucketTeam getTeam() throws IOException, InterruptedException {
+        String url = UriTemplate.fromTemplate("{+base}{/owner}")
+                .set("base", V2_TEAMS_API_BASE_URL)


perhaps we should change this template, I don't like the set "base"
WDYT?

jetersen · 2018-01-03T15:52:35Z

src/test/java/com/cloudbees/jenkins/plugins/bitbucket/endpoints/BitbucketCloudEndpointTest.java

@@ -39,8 +43,32 @@ public void smokes() {

    @Test
    public void getRepositoryUrl() {
+


👻 whitespace

jetersen

@rsandell @stephenc WDYT? :)
This is benefitting from handy uri template 🚀 :

bitbucket-branch-source-plugin/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java

Lines 1040 to 1069 in fed749a

    
               String branchUrl; 
        
               String title; 
        
               if (head instanceof PullRequestSCMHead) { 
        
                   PullRequestSCMHead pr = (PullRequestSCMHead) head; 
        
                   branchUrl = repoOwner + "/" + repository + "/pull-requests/" + pr.getId(); 
        
                   title = getPullRequestTitleCache().get(pr.getId()); 
        
                   ContributorMetadataAction contributor = getPullRequestContributorCache().get(pr.getId()); 
        
                   if (contributor != null) { 
        
                       result.add(contributor); 
        
                   } 
        
               } else { 
        
                   branchUrl = repoOwner + "/" + repository + "/branch/" + Util.rawEncode(head.getName()); 
        
                   title = null; 
        
               } 
        
               result.add(new BitbucketLink("icon-bitbucket-branch", getServerUrl() + "/" + branchUrl)); 
        
               result.add(new ObjectMetadataAction(title, null, getServerUrl() + "/" + branchUrl)); 
        
           } else { 
        
               String branchUrl; 
        
               String title; 
        
               if (head instanceof PullRequestSCMHead) { 
        
                   PullRequestSCMHead pr = (PullRequestSCMHead) head; 
        
                   branchUrl = "projects/" + repoOwner + "/repos/" + repository + "/pull-requests/" +pr.getId()+"/overview"; 
        
                   title = getPullRequestTitleCache().get(pr.getId()); 
        
                   ContributorMetadataAction contributor = getPullRequestContributorCache().get(pr.getId()); 
        
                   if (contributor != null) { 
        
                       result.add(contributor); 
        
                   } 
        
               } else { 
        
                   branchUrl = "projects/" + repoOwner + "/repos/" + repository + "/compare/commits" 
        
                           + "?sourceBranch=" + URLEncoder.encode(Constants.R_HEADS + head.getName(), "UTF-8");

jetersen · 2018-01-03T16:33:31Z

...ain/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java

@@ -726,13 +827,24 @@ public InputStream getFileContent(BitbucketSCMFile file) throws IOException, Int
        String path = encodePath(file.getPath());
        String ref = URLEncoder.encode(file.getRef(), "UTF-8");


This is no longer needed?

jetersen · 2018-01-03T16:33:37Z

...ain/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java

@@ -726,13 +827,24 @@ public InputStream getFileContent(BitbucketSCMFile file) throws IOException, Int
        String path = encodePath(file.getPath());


This is no longer needed?

jetersen · 2018-01-03T16:33:46Z

...ain/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java

@@ -689,15 +779,26 @@ private String deleteRequest(String path) throws IOException {
        String path = encodePath(directory.getPath());


This is no longer needed?

jetersen · 2018-01-03T16:33:55Z

...ain/java/com/cloudbees/jenkins/plugins/bitbucket/server/client/BitbucketServerAPIClient.java

@@ -689,15 +779,26 @@ private String deleteRequest(String path) throws IOException {
        String path = encodePath(directory.getPath());
        String ref = URLEncoder.encode(directory.getRef(), "UTF-8");


This is no longer needed?

jetersen · 2018-01-05T08:02:17Z

Tested with both bitbucket cloud and bitbucket server that URL's and scans still work 👍 🚀

https://bitbucket.org/caszum/testrepo/branch/bugfix%2Fmy%2Fsuper%2Fawesome%2Fbugfix

stephenc · 2018-01-05T14:41:53Z

Can we get uri templates on

bitbucket-branch-source-plugin/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java

Lines 1040 to 1069 in fed749a

    
               String branchUrl; 
        
               String title; 
        
               if (head instanceof PullRequestSCMHead) { 
        
                   PullRequestSCMHead pr = (PullRequestSCMHead) head; 
        
                   branchUrl = repoOwner + "/" + repository + "/pull-requests/" + pr.getId(); 
        
                   title = getPullRequestTitleCache().get(pr.getId()); 
        
                   ContributorMetadataAction contributor = getPullRequestContributorCache().get(pr.getId()); 
        
                   if (contributor != null) { 
        
                       result.add(contributor); 
        
                   } 
        
               } else { 
        
                   branchUrl = repoOwner + "/" + repository + "/branch/" + Util.rawEncode(head.getName()); 
        
                   title = null; 
        
               } 
        
               result.add(new BitbucketLink("icon-bitbucket-branch", getServerUrl() + "/" + branchUrl)); 
        
               result.add(new ObjectMetadataAction(title, null, getServerUrl() + "/" + branchUrl)); 
        
           } else { 
        
               String branchUrl; 
        
               String title; 
        
               if (head instanceof PullRequestSCMHead) { 
        
                   PullRequestSCMHead pr = (PullRequestSCMHead) head; 
        
                   branchUrl = "projects/" + repoOwner + "/repos/" + repository + "/pull-requests/" +pr.getId()+"/overview"; 
        
                   title = getPullRequestTitleCache().get(pr.getId()); 
        
                   ContributorMetadataAction contributor = getPullRequestContributorCache().get(pr.getId()); 
        
                   if (contributor != null) { 
        
                       result.add(contributor); 
        
                   } 
        
               } else { 
        
                   branchUrl = "projects/" + repoOwner + "/repos/" + repository + "/compare/commits" 
        
                           + "?sourceBranch=" + URLEncoder.encode(Constants.R_HEADS + head.getName(), "UTF-8");

too

jetersen · 2018-01-05T16:14:45Z

@stephenc it's there :)

https://github.com/jenkinsci/bitbucket-branch-source-plugin/pull/80/files#diff-dbebe1606f4711ed03640cf707dc6c23
https://github.com/casz/bitbucket-branch-source-plugin/blob/34374d3f9a625d768bd09c2974e5ec0f8d607777/src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java#L1040-L1092

I guess it could be made even BETTER 👍

jetersen · 2018-01-05T16:38:41Z

@stephenc slightly improved version 803dbe4

jetersen

I think this looks much cleaner

jetersen · 2018-01-05T17:26:26Z

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java

        }
+        if (head instanceof PullRequestSCMHead) {


The if-else above is now only for URL template and this is now deduped.

jetersen · 2018-01-05T17:26:40Z

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java

+        }
+        String url = template.expand();
+        result.add(new BitbucketLink("icon-bitbucket-branch", url));
+        result.add(new ObjectMetadataAction(title, null, url));


More deduping

jetersen · 2018-01-05T17:27:44Z

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java

+        template.set("owner", repoOwner).set("repo", repository);
+        String url = template.expand();
+        result.add(new BitbucketLink("icon-bitbucket-repo", url));
+        result.add(new ObjectMetadataAction(r.getRepositoryName(), null, url));


Some more needed deduping

vivek · 2018-01-09T23:55:28Z

Looks like its ready to be merged. @stephenc, looks good to you?

stephenc · 2018-01-10T00:34:59Z

Let’s go

[FIXED JENKINS-41883] Append global log on open

stephenc reviewed Nov 9, 2017

View reviewed changes

jetersen commented Nov 16, 2017

View reviewed changes

Hellspam mentioned this pull request Nov 29, 2017

JENKINS-48098 - Lazily evaluate timestamp for branch #86

Closed

stephenc mentioned this pull request Jan 2, 2018

[JENKINS-47254] Support for building tags for Bitbucket Server/Cloud #99

Closed

stephenc approved these changes Jan 3, 2018

View reviewed changes

stephenc requested a review from rsandell January 3, 2018 14:14

stephenc reviewed Jan 3, 2018

View reviewed changes

jetersen commented Jan 3, 2018

View reviewed changes

This was referenced Jan 3, 2018

Adding cache to limit BitBucket API rate hits #91

Merged

Use AuthenticationTokens #101

Merged

jetersen added 5 commits January 4, 2018 00:15

Handy Uri Template

3d3413b

Update to released version

7bce2ce

Handy URI all the things

1f27af9

base inside fromTemplate and no star

31d7a0e

whoops :)

34374d3

Slighty better handy uri template usage :)

803dbe4

Cleanup duplicate code

e40df9e

jetersen commented Jan 5, 2018

View reviewed changes

correct template syntax

52443a4

jetersen mentioned this pull request Jan 6, 2018

JENKINS-47978# Fix handling branch name with '/' #102

Merged

stephenc merged commit fc07854 into jenkinsci:master Jan 10, 2018

jetersen deleted the handyuri branch January 10, 2018 06:07

fengxx pushed a commit to fengxx/bitbucket-branch-source-plugin that referenced this pull request Mar 13, 2018

Merge pull request jenkinsci#80 from stephenc/jenkins-41883

4a57f3a

[FIXED JENKINS-41883] Append global log on open

jetersen mentioned this pull request Mar 20, 2019

Consider implementing branch source functionality jenkinsci/gitlab-plugin#499

Closed

	while(!(boolean)content.get("isLastPage")){
	start += (int) content.get("size");
	response = getRequest(String.format(url+"&start=%s&limit=&s", start, 500));
	content = collectLines(response, lines);
	}

	while (!(boolean)page.get("isLastPage")){
	response = getRequest(String.format(url+"&start=%s&limit=%s", start, 500));
	content = JsonParser.mapper.readValue(response, new TypeReference<Map<String,Object>>(){});
	page = (Map) content.get("children");
	}

		@@ -39,8 +43,32 @@ public void smokes() {

		@Test
		public void getRepositoryUrl() {

	String branchUrl;
	String title;
	if (head instanceof PullRequestSCMHead) {
	PullRequestSCMHead pr = (PullRequestSCMHead) head;
	branchUrl = repoOwner + "/" + repository + "/pull-requests/" + pr.getId();
	title = getPullRequestTitleCache().get(pr.getId());
	ContributorMetadataAction contributor = getPullRequestContributorCache().get(pr.getId());
	if (contributor != null) {
	result.add(contributor);
	}
	} else {
	branchUrl = repoOwner + "/" + repository + "/branch/" + Util.rawEncode(head.getName());
	title = null;
	}
	result.add(new BitbucketLink("icon-bitbucket-branch", getServerUrl() + "/" + branchUrl));
	result.add(new ObjectMetadataAction(title, null, getServerUrl() + "/" + branchUrl));
	} else {
	String branchUrl;
	String title;
	if (head instanceof PullRequestSCMHead) {
	PullRequestSCMHead pr = (PullRequestSCMHead) head;
	branchUrl = "projects/" + repoOwner + "/repos/" + repository + "/pull-requests/" +pr.getId()+"/overview";
	title = getPullRequestTitleCache().get(pr.getId());
	ContributorMetadataAction contributor = getPullRequestContributorCache().get(pr.getId());
	if (contributor != null) {
	result.add(contributor);
	}
	} else {
	branchUrl = "projects/" + repoOwner + "/repos/" + repository + "/compare/commits"
	+ "?sourceBranch=" + URLEncoder.encode(Constants.R_HEADS + head.getName(), "UTF-8");

		@@ -726,13 +827,24 @@ public InputStream getFileContent(BitbucketSCMFile file) throws IOException, Int
		String path = encodePath(file.getPath());
		String ref = URLEncoder.encode(file.getRef(), "UTF-8");

		@@ -689,15 +779,26 @@ private String deleteRequest(String path) throws IOException {
		String path = encodePath(directory.getPath());

		@@ -689,15 +779,26 @@ private String deleteRequest(String path) throws IOException {
		String path = encodePath(directory.getPath());
		String ref = URLEncoder.encode(directory.getRef(), "UTF-8");

Handy Uri Template #80

Handy Uri Template #80

Conversation

jetersen commented Nov 9, 2017

stephenc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jetersen Dec 13, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jetersen Dec 20, 2017 • edited

Choose a reason for hiding this comment

jetersen commented Nov 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jetersen Nov 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hellspam commented Nov 19, 2017

jetersen commented Dec 13, 2017

jetersen commented Jan 2, 2018 • edited

stephenc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jetersen left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jetersen commented Jan 5, 2018 • edited

stephenc commented Jan 5, 2018

jetersen commented Jan 5, 2018

jetersen commented Jan 5, 2018

jetersen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jetersen Jan 5, 2018 • edited

Choose a reason for hiding this comment

vivek commented Jan 9, 2018

stephenc commented Jan 10, 2018

jetersen Dec 13, 2017 •

edited

jetersen Dec 20, 2017 •

edited

jetersen Nov 16, 2017 •

edited

jetersen commented Jan 2, 2018 •

edited

jetersen left a comment •

edited

jetersen commented Jan 5, 2018 •

edited

jetersen Jan 5, 2018 •

edited