Add support for creation/download of Support Zip #197

padas2 · 2019-06-10T06:41:41Z

Initial cut of Pull Request to add support for interacting with SupportZip REST API.

Work so far done:

Added Code for creating and getting status of Support Zip.
Added Live and Mock tests for both of the above function calls.

Things yet to be done.

Need to implement fallbacks for SupportApi method calls.

…i as well, Need to create Mock Test and in-line supportApi tests

…low HAS-A pattern to avoid duplication

…but looks like autovalue does not allow it, need to investigate more

padas2 · 2019-06-11T19:06:12Z

@cdancy Could you please take a look at this PR and provide your inputs regarding the issue about duplication as well.

cdancy · 2019-06-11T19:08:49Z

@padas2 will do. Was out sick yesterday and planned on looking at things and providing comments end-of-day today.

cdancy · 2019-06-11T19:09:49Z

src/main/java/com/cdancy/bitbucket/rest/features/SupportApi.java

+@Path("/rest/troubleshooting/latest/support-zip")
+@SuppressWarnings("PMD.AvoidDuplicateLiterals")
+public interface SupportApi {
+    @Named("support-zip:create")


Lets put a line-break between the class name and this Named.

cdancy · 2019-06-11T19:10:26Z

src/main/java/com/cdancy/bitbucket/rest/features/SupportApi.java

+    @Consumes(MediaType.APPLICATION_JSON)
+    @Path("/local")
+    @POST
+    SupportZipDetails createSupportZip();


Can we put a Documentation annotation here as we do elsewhere?

Done though the lengths of links are very long(>= 210 chars long)

cdancy · 2019-06-11T19:12:48Z

Looking things over we'll need mock-tests to cover both endpoints.

Also ... both endpoints, and the objects they return, NEED to handle failure states with the error objects that are on the main README of this project and every endpoint implements. Take a look at how that is done and let me know if you have any questions. Both the mock and live tests will need further tests to account for the possible failure states.

cdancy · 2019-06-11T19:16:58Z

There is code duplication in 2 classes i.e. SupportZipDetails and SupportZipStatus.
Tried using the HAS-A pattern to include SupportZipDetails in SupportZipStatus class itself but looks
like it is not allowed by AutoValue.
One way to remove duplicate code would be to move components to a common class and have the 2
classes extend them, but that would violate the IS-A/HAS-A principle, so leaving that out..

How similar are they intended to be? If they are similar enough we can just use one object and provide the @nullable annotation when it's expected to be null.

cdancy · 2019-06-11T19:17:46Z

Also ... for downloading the zip lets leave that for a subsequent PR. I generally like to have 1 endpoint per PR to make things easier for reviewing.

…documentation detail annotation tag

padas2 · 2019-06-12T06:23:41Z

How similar are they intended to be? If they are similar enough we can just use one object and provide the @nullable annotation when it's expected to be null.

Not sure if they were meant to be similar but they sure have a lot of common components.
See below for response types in both the cases.
On sending the POST call to create a support zip, this is the response we get.

{
    "taskId": "b1481be1-e63d-48c2-bc73-c45d97261e0d",
    "progressPercentage": 0,
    "progressMessage": "",
    "warnings": [],
    "status": "IN_PROGRESS"
}

On trying to determine the status of the support Zip task id, this is the response we get.

{
    "taskId": "b1481be1-e63d-48c2-bc73-c45d97261e0d",
    "progressPercentage": 100,
    "progressMessage": "It was saved to C:\\Atlassian\\ApplicationData\\Bitbucket\\shared\\export\\Bitbucket_support_2019-06-12-11-45-43.zip.",
    "fileName": "Bitbucket_support_2019-06-12-11-45-43.zip",
    "warnings": [
        {
            "@class": "com.atlassian.troubleshooting.stp.action.DefaultMessage",
            "name": "File Truncated",
            "body": "The file atlassian-bitbucket-2019-05-19.log was truncated to 25Mb in size.  If you need to include the whole file, untick the 'Limit File Sizes' option."
        },
        {
            "@class": "com.atlassian.troubleshooting.stp.action.DefaultMessage",
            "name": "File Truncated",
            "body": "The file atlassian-bitbucket-2019-05-24.log was truncated to 25Mb in size.  If you need to include the whole file, untick the 'Limit File Sizes' option."
        },
        {
            "@class": "com.atlassian.troubleshooting.stp.action.DefaultMessage",
            "name": "File Truncated",
            "body": "The file atlassian-bitbucket-2019-05-23.log was truncated to 25Mb in size.  If you need to include the whole file, untick the 'Limit File Sizes' option."
        },
        {
            "@class": "com.atlassian.troubleshooting.stp.action.DefaultMessage",
            "name": "File Truncated",
            "body": "The file atlassian-bitbucket-2019-05-17.log was truncated to 25Mb in size.  If you need to include the whole file, untick the 'Limit File Sizes' option."
        }
    ],
    "status": "COMPLETE_WITH_WARNING"
}

padas2 · 2019-06-12T06:24:45Z

Looking things over we'll need mock-tests to cover both endpoints.

Also ... both endpoints, and the objects they return, NEED to handle failure states with the error objects that are on the main README of this project and every endpoint implements. Take a look at how that is done and let me know if you have any questions. Both the mock and live tests will need further tests to account for the possible failure states.

Sure.

cdancy · 2019-06-13T17:25:04Z

On trying to determine the status of the support Zip task id, this is the response we get.

Ok they look very similar. Lets go with creating just the one object for both endpoints and provide the @nullable annotation where necessary.

padas2 · 2019-06-13T19:06:52Z

On trying to determine the status of the support Zip task id, this is the response we get.

Ok they look very similar. Lets go with creating just the one object for both endpoints and provide the @nullable annotation where necessary.

So, I am going out of town for a couple of days.
Will resume working on this from Saturday.

cdancy · 2019-06-13T19:07:53Z

@padas2 sounds good!

padas2 · 2019-06-16T18:04:44Z

And @cdancy , I forgot to mention one more thing.
The REST API which we are using here to create the support zip is valid for both bitbucket server types and bitbucket data center types.
But we can send parameters as well in the REST call so as to include/exclude other files(such as thread dumps, configuration files) in the support zip.
Should that be included in this PR as well or maybe as part of another PR ?
What do you think ?

…calls to use only one object type, Added mock tests for both Support Api calls, Added test resources as well

cdancy · 2019-06-19T16:26:54Z

@padas2 would those be different endpoints or included in the response of the existing endpoints in this PR?

cdancy · 2019-06-19T16:35:04Z

Also ... PR looks good so far. Still need another pass through to implement ErrorsHolder on the SupportZip domain object. Take a look at the RequestStatus below for a simple case on how to implement this. This is a primary use-case for this library in that no call will fail but instead will be handed back the expected Object with some number of "errors" attached should the call fail for whatever reason. The user then can do something like:

// groovy syntax below

SupportZip supportZip = ...
if (supportZip.errors()) {
    if (supportZip.errors().last().message.equals("something bad we care about")) {
        throw new ExceptionOfSomeKind("The World is ending!!!", supportZip.errors());
    }
}

// continue as normal

I can help/guide you if need be just scream in my direction.

https://github.com/cdancy/bitbucket-rest/blob/master/src/main/java/com/cdancy/bitbucket/rest/domain/common/RequestStatus.java

padas2 · 2019-06-20T09:43:12Z

Also ... PR looks good so far. Still need another pass through to implement ErrorsHolder on the SupportZip domain object. Take a look at the RequestStatus below for a simple case on how to implement this. This is a primary use-case for this library in that no call will fail but instead will be handed back the expected Object with some number of "errors" attached should the call fail for whatever reason. The user then can do something like:
// groovy syntax below

SupportZip supportZip = ...
if (supportZip.errors()) {
    if (supportZip.errors().last().message.equals("something bad we care about")) {
        throw new ExceptionOfSomeKind("The World is ending!!!", supportZip.errors());
    }
}

// continue as normal
I can help/guide you if need be just scream in my direction.

https://github.com/cdancy/bitbucket-rest/blob/master/src/main/java/com/cdancy/bitbucket/rest/domain/common/RequestStatus.java

Was very busy for the last couple of days. Will try to get on it either today or tomorrow.

padas2 · 2019-06-20T10:11:15Z

@padas2 would those be different endpoints or included in the response of the existing endpoints in this PR?

Looks like I was mistaken.
On further verification, looks like there are different endpoints for server type and data center type.

For data center, the endpoint we gotta hit is (for generating support zips across all clusters):
http://<base-url.example.com:8080>/rest/troubleshooting/latest/support-zip/cluster

For server, the endpoint we gotta hit is:
http://<app-url.example.com:8080>/rest/troubleshooting/latest/support-zip/local

cdancy · 2019-06-20T14:41:50Z

Cool. Let me know if you need help and good work thus far. Wiring things up for use with jclouds, on top of the "errors" layer I've put on top, can be a bit daunting if you've never done so before.

…dding missing whitespace

padas2 · 2019-06-21T19:00:19Z

Hey Chris, just a small doubt.

How and where is the ordering of any of the autovalue classes's constructor defined ?
The thing I am not clear is, why do we always have to pass the Immutable list of errors as the first parameter in any autovalue class constructor ?

Also I tried passing it in the end, but it failed with the following error

Error:(54, 41) java: incompatible types: java.lang.String cannot be converted to java.util.List<com.cdancy.bitbucket.rest.domain.common.Error>

which was fixed once I put the errors list in first place.

cdancy · 2019-06-21T19:06:23Z

Also I tried passing it in the end, but it failed with the following error

Aaaahhhh ... that's just an AutoValue feature/quirk with how they build the generated classes. Take a look at the generated java on disk to see what they are doing. That's what I did many moons ago as the magic was too much for me to handle.

cdancy · 2019-06-27T19:12:32Z

src/main/java/com/cdancy/bitbucket/rest/fallbacks/BitbucketFallbacks.java

@@ -663,6 +674,11 @@ public static PullRequest createPullRequestFromErrors(final List<Error> errors)
                null, null, errors);
    }

+    public static SupportZip createSupportZipFromErrors(final List<Error> errors) {
+        return SupportZip.create(null, 0, null,
+                               null, null, null, errors);


This shouldn't actually work if none of the fields are annotated with @nullable.

cdancy · 2019-06-27T19:14:07Z

src/test/java/com/cdancy/bitbucket/rest/features/SupportApiLiveTest.java

+    @Test
+    public void testSupportZipStatus() {
+        SupportZip supportZipTask = api().createSupportZip();
+        supportZipTask = api().getSupportZipStatus(supportZipTask.taskId());


Lets also test the case where things fail. Meaning we can pass in a taskId which does not exist and the returned Errors list should be populated with objects.

cdancy · 2019-06-27T19:16:38Z

src/test/java/com/cdancy/bitbucket/rest/features/SupportApiMockTest.java

+        } finally {
+            mockWebServer.shutdown();
+        }
+    }


We also need mock tests for when things fail on both endpoints. To trigger the failure you could use a non-existent taskId for one, combined with an error code of whatever that is supposed to return, and the other I would use an error code response of say 500 to account for insufficient privileges to do this task.

padas2 · 2019-07-01T13:26:07Z

Hey @cdancy , so sorry I forgot to reply. But I have been too much occupied with other stuff.
Yes... I will do all the recommended changes and notify you once they are in place.

padas2 added 8 commits June 8, 2019 23:49

Added SupportApi to create Support Zip, Added Live test for supportAp…

65a3d8f

…i as well, Need to create Mock Test and in-line supportApi tests

Adding back unintentionally removed imports

7689062

Removing custom credentials

39c70d1

Started work on getting task status

bf3a0f8

Renaming SupportZipTask to SupportZipDetails, refactoring code to fol…

3d2f99f

…low HAS-A pattern to avoid duplication

Renaming SupportZip to SupportZipStatus, tried to use HAS-A pattern, …

a97730d

…but looks like autovalue does not allow it, need to investigate more

Removing changes introduced by IDE

39707c4

Removing unecessary empty lines

d3d2c8b

padas2 changed the title ~~Feature/endpoints for support zip~~ Add support for creation/download of Support Zip Jun 10, 2019

Again adding code removed by IDE

9313612

padas2 mentioned this pull request Jun 10, 2019

Adding API's to generate and download support zip from bitbucket server. #196

Open

padas2 added 4 commits June 10, 2019 12:38

Removed all windows line endings

e96591f

restoring ide removed commits

611d00c

Adding licensing info

b40793d

Fixing PMD violations

eca9dcd

cdancy self-assigned this Jun 11, 2019

cdancy added the enhancement label Jun 11, 2019

cdancy reviewed Jun 11, 2019

View reviewed changes

cdancy added this to the v2.5.4 milestone Jun 11, 2019

Adding whitespace between class name and top most annotation, Adding …

a3726ae

…documentation detail annotation tag

padas2 added 3 commits June 17, 2019 01:45

A Rather big commit containing following changes: Changed SupportApi …

6d6b096

…calls to use only one object type, Added mock tests for both Support Api calls, Added test resources as well

Fixed CheckStyle and Licensing errors

81e8958

Fixed PMD rule violations

e237b19

padas2 added 2 commits June 22, 2019 00:00

Initial changes for introducing fallbacks for support zip failures

b26abdf

Reverting accidental conversion of Line endings back to LF type and a…

b64e3f3

…dding missing whitespace

cdancy reviewed Jun 27, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for creation/download of Support Zip #197

Add support for creation/download of Support Zip #197

padas2 commented Jun 10, 2019 •

edited

padas2 commented Jun 11, 2019

cdancy commented Jun 11, 2019

cdancy Jun 11, 2019

padas2 Jun 12, 2019

cdancy Jun 11, 2019

padas2 Jun 12, 2019 •

edited

cdancy commented Jun 11, 2019

cdancy commented Jun 11, 2019

cdancy commented Jun 11, 2019

padas2 commented Jun 12, 2019

padas2 commented Jun 12, 2019

cdancy commented Jun 13, 2019

padas2 commented Jun 13, 2019

cdancy commented Jun 13, 2019

padas2 commented Jun 16, 2019 •

edited

cdancy commented Jun 19, 2019

cdancy commented Jun 19, 2019 •

edited

padas2 commented Jun 20, 2019

padas2 commented Jun 20, 2019

cdancy commented Jun 20, 2019

padas2 commented Jun 21, 2019 •

edited

cdancy commented Jun 21, 2019

cdancy Jun 27, 2019

cdancy Jun 27, 2019

cdancy Jun 27, 2019

padas2 commented Jul 1, 2019

Add support for creation/download of Support Zip #197

Are you sure you want to change the base?

Add support for creation/download of Support Zip #197

Conversation

padas2 commented Jun 10, 2019 • edited

padas2 commented Jun 11, 2019

cdancy commented Jun 11, 2019

cdancy Jun 11, 2019

Choose a reason for hiding this comment

padas2 Jun 12, 2019

Choose a reason for hiding this comment

cdancy Jun 11, 2019

Choose a reason for hiding this comment

padas2 Jun 12, 2019 • edited

Choose a reason for hiding this comment

cdancy commented Jun 11, 2019

cdancy commented Jun 11, 2019

cdancy commented Jun 11, 2019

padas2 commented Jun 12, 2019

padas2 commented Jun 12, 2019

cdancy commented Jun 13, 2019

padas2 commented Jun 13, 2019

cdancy commented Jun 13, 2019

padas2 commented Jun 16, 2019 • edited

cdancy commented Jun 19, 2019

cdancy commented Jun 19, 2019 • edited

padas2 commented Jun 20, 2019

padas2 commented Jun 20, 2019

cdancy commented Jun 20, 2019

padas2 commented Jun 21, 2019 • edited

cdancy commented Jun 21, 2019

cdancy Jun 27, 2019

Choose a reason for hiding this comment

cdancy Jun 27, 2019

Choose a reason for hiding this comment

cdancy Jun 27, 2019

Choose a reason for hiding this comment

padas2 commented Jul 1, 2019

padas2 commented Jun 10, 2019 •

edited

padas2 Jun 12, 2019 •

edited

padas2 commented Jun 16, 2019 •

edited

cdancy commented Jun 19, 2019 •

edited

padas2 commented Jun 21, 2019 •

edited