Skip to content

Multizones#598

Merged
carycheng merged 24 commits into
masterfrom
multizones
May 10, 2018
Merged

Multizones#598
carycheng merged 24 commits into
masterfrom
multizones

Conversation

@carycheng
Copy link
Copy Markdown

No description provided.

@carycheng carycheng requested a review from mattwiller May 2, 2018 23:39
@boxcla
Copy link
Copy Markdown

boxcla commented May 2, 2018

Hi @carycheng, thanks for the pull request. Before we can merge it, we need you to sign our Contributor License Agreement. You can do so electronically here: http://opensource.box.com/cla

Once you have signed, just add a comment to this pull request saying, "CLA signed". Thanks!

@carycheng carycheng removed the request for review from mattwiller May 2, 2018 23:41
Comment thread doc/storage_policy.md Outdated
*[Get Assignment For Target](#get-assignment-for-target)
*[Update Existing Assignment](#update-existing-assignment)
*[Assign Storage Policy](#assign-storage-policy)
*[Delete Assignment](#delete-assignment)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need a space between the asterisks and titles to make these actually turn into bullets

Comment thread doc/storage_policy.md Outdated
BoxStoragePolicy.Info storagePolicyInfo = storagePolicy.getInfo();
```

[get-info]:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These links still need to be filled in

Comment thread doc/storage_policy.md Outdated
Get Storage Policy
------------------

Calling [`getInfo(String...)`][get-info] will return BoxStoragePolicy.Info object
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add the parameter names to the method signatures in all of these

Comment thread doc/storage_policy.md Outdated
Calling [`delete()`][delete] will remove the storage policy assignment from the user.

```java
BoxStoragePolicyAssignment assignment = new BoxStoragePolicyAssignment(api, "user_1234");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the MultiZones team is changing the IDs to be obfuscated, we should make the example match the real ID format (maybe just base64 encoded)

/**
* Represents a BoxStoragePolicy.
*/
@BoxResourceType("BoxStoragePolicy")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the string in this annotation is supposed to be the Box type, e.g. "storage_policy"

/**
* Represents a BoxStoragePolicyAssignment.
*/
@BoxResourceType("BoxStoragePolicyAssignment")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the string in this annotation is supposed to be the Box type, e.g. "storage_policy_assignment"

BoxJSONResponse response = (BoxJSONResponse) request.send();
return new Info(response.getJSON());
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably also add an assign() method to this class, otherwise it's not very useful to have instances

final String assignedToType = "user";
final String assignedToID = "5678";

final JsonObject fakeJSONResponse = JsonObject.readFrom("{\n"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be moved into fixtures to align with the new testing pattern


@Test
@Category(IntegrationTest.class)
public void testAssignStoragePolicySucceeds() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed?

Comment thread doc/storage_policy.md
--------------------------

Updating a storage policy assignment information is done by calling
[`updateInfo(BoxStoragePolicyAssignment.Info updatedInfo)`][update-info].
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link needs a URL

Comment thread doc/storage_policy.md Outdated
Calling the static [`getAll(BoxAPIConnection api)`][get-list-of-storage-policies]
will return an iterable that will page through all of the storage policies.
It is possible to specify maximum number of items per response and fields to retrieve by
calling the static [`getAll(BoxAPIConnection api, int limit, String fields ...)`][get-list-storage-policies-with-fields] method.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link is broken, looks like it doesn't match the identifier below with the URL

* @param userID the ID of the user you want to assign the Storage Policy to.
* @return information about this {@link BoxStoragePolicyAssignment}.
*/
public static BoxStoragePolicyAssignment.Info assign(BoxAPIConnection api, String storagePolicyID, String userID) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already a static one of these on BoxStoragePolicyAssignment — this one should probably be an instance method

public class BoxStoragePolicyAssignment extends BoxResource {

/**
* Storage PolicIES Assignment URL Template.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Strange capitalization in this comment

/**
* The default limit for returning a storage policy info.
*/
private static final int STORAGE_POLICY_INFO_LIMIT = 1;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this get used?


BoxStoragePolicyAssignment storagePolicyAssignment = new BoxStoragePolicyAssignment(api,
response.getJsonObject().get("entries").asArray().get(0).asObject().get("id").asString());
BoxStoragePolicyAssignment.Info info = storagePolicyAssignment.new
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern is very yucky — we should try to eliminate this weirdness in the future

@carycheng carycheng merged commit 5257101 into master May 10, 2018
@mattwiller mattwiller deleted the multizones branch December 15, 2018 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants