bitbucket-rest icon indicating copy to clipboard operation
bitbucket-rest copied to clipboard

Add support for creation/download of Support Zip

Open padas2 opened this issue 6 years ago • 19 comments
trafficstars

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.

padas2 avatar Jun 10 '19 06:06 padas2

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

padas2 avatar Jun 11 '19 19:06 padas2

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

cdancy avatar Jun 11 '19 19:06 cdancy

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 avatar Jun 11 '19 19:06 cdancy

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 avatar Jun 11 '19 19:06 cdancy

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.

cdancy avatar Jun 11 '19 19:06 cdancy

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 avatar Jun 12 '19 06:06 padas2

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.

padas2 avatar Jun 12 '19 06:06 padas2

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.

cdancy avatar Jun 13 '19 17:06 cdancy

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.

padas2 avatar Jun 13 '19 19:06 padas2

@padas2 sounds good!

cdancy avatar Jun 13 '19 19:06 cdancy

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 ?

padas2 avatar Jun 16 '19 18:06 padas2

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

cdancy avatar Jun 19 '19 16:06 cdancy

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

cdancy avatar Jun 19 '19 16:06 cdancy

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 avatar Jun 20 '19 09:06 padas2

@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

padas2 avatar Jun 20 '19 10:06 padas2

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.

cdancy avatar Jun 20 '19 14:06 cdancy

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.

padas2 avatar Jun 21 '19 19:06 padas2

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 avatar Jun 21 '19 19:06 cdancy

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 avatar Jul 01 '19 13:07 padas2