feat: implement testPermission api of organizations by athakor · Pull Request #125 · googleapis/java-resourcemanager

athakor · 2020-03-30T06:25:28Z

Towards #89

codecov · 2020-03-30T06:27:51Z

Codecov Report

Merging #125 into master will decrease coverage by 1.30%.
The diff coverage is 28.57%.

@@             Coverage Diff              @@
##             master     #125      +/-   ##
============================================
- Coverage     87.12%   85.82%   -1.31%     
- Complexity      209      210       +1     
============================================
  Files            11       11              
  Lines           924      945      +21     
  Branches         99      100       +1     
============================================
+ Hits            805      811       +6     
- Misses           86      101      +15     
  Partials         33       33

Impacted Files	Coverage Δ	Complexity Δ
.../google/cloud/resourcemanager/ResourceManager.java	`95.65% <ø> (ø)`	`0.00 <0.00> (ø)`
...loud/resourcemanager/ResourceManagerException.java	`100.00% <ø> (ø)`	`5.00 <0.00> (ø)`
...rcemanager/spi/v1beta1/HttpResourceManagerRpc.java	`77.24% <0.00%> (-8.26%)`	`13.00 <0.00> (ø)`
...esourcemanager/spi/v1beta1/ResourceManagerRpc.java	`100.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...gle/cloud/resourcemanager/ResourceManagerImpl.java	`87.50% <85.71%> (-0.12%)`	`16.00 <1.00> (+1.00)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7112223...8ad5c75. Read the comment docs.

athakor · 2020-03-30T09:28:59Z

@chingor13 gentle ping

elharo · 2020-04-06T19:01:36Z

src/main/java/com/google/cloud/resourcemanager/ResourceManager.java

+   *
+   * @param resource the organization's resource name, e.g. "organizations/123"
+   * @param permissions the set of permissions to check for the resource. Permissions with wildcards
+   *     (such as '*' or 'storage.*') are not allowed


elharo · 2020-04-06T19:01:49Z

src/main/java/com/google/cloud/resourcemanager/ResourceManager.java

+   * @param resource the organization's resource name, e.g. "organizations/123"
+   * @param permissions the set of permissions to check for the resource. Permissions with wildcards
+   *     (such as '*' or 'storage.*') are not allowed
+   * @return A list of booleans representing whether the caller has the permissions specified (in


nit: A --> a

elharo · 2020-04-06T19:03:53Z

src/main/java/com/google/cloud/resourcemanager/spi/v1beta1/HttpResourceManagerRpc.java

  }
+
+  @Override
+  public List<Boolean> testOrgPermissions(String resource, List<String> permissions) {


This API is strange. One rarely encounters a list of booleans. What does someone do with it? How do you know which boolean maps to which permission?

elharo · 2020-04-06T19:04:47Z

src/main/java/com/google/cloud/resourcemanager/spi/v1beta1/ResourceManagerRpc.java

-  // TODO(ajaykannan): implement "Organization" functionality when available (issue #319)
+  /**
+   * Tests whether the caller has the given permissions on the specified Organization. Returns a
+   * list of booleans corresponding to whether or not the user has the permission in the same


Do we maybe just want the logical AND of all these permissions?

Or perhaps what you want to return is a Map<String, boolean> that lists the permissions and their results?

elharo · 2020-04-06T19:06:14Z

src/test/java/com/google/cloud/resourcemanager/ResourceManagerImplTest.java

+
+  @Test
+  public void testTestOrgPermissions() {
+    ResourceManagerRpcFactory rpcFactoryMock = EasyMock.createMock(ResourceManagerRpcFactory.class);


Prefer Mockito per Google Java policy

athakor · 2020-04-07T07:56:17Z

@elharo all the comment have been addressed PTAL

elharo · 2020-04-07T10:35:58Z

src/main/java/com/google/cloud/resourcemanager/spi/v1beta1/ResourceManagerRpc.java

-  // TODO(ajaykannan): implement "Organization" functionality when available (issue #319)
+  /**
+   * Tests whether the caller has the given permissions on the specified Organization. Returns the
+   * permissions and their results corresponding to whether or not the user has the permission in


no longer need " corresponding to whether or not the user has the permission in

the same position of input list."

elharo · 2020-04-07T12:19:05Z

src/main/java/com/google/cloud/resourcemanager/ResourceManager.java

+   *     "https://cloud.google.com/resource-manager/reference/rest/v1/organizations/testIamPermissions">
+   *     Resource Manager testIamPermissions</a>
+   */
+  Map<String, Boolean> testOrgPermissions(String resource, List<String> permissions);


Not new in this PR, and shouldn't be fixed now, but the distinction between ResourceManager and ResourceManagerImpl is likely unnecessary and just makes the code more complicated for no particular reason.

athakor added 3 commits March 27, 2020 17:51

feat: implement testPermission api of organization

13017f5

feat: modified java doc

3960773

feat: modified java doc

2d2c781

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 30, 2020

athakor requested a review from chingor13 March 30, 2020 06:29

athakor requested a review from a team April 6, 2020 09:51

elharo reviewed Apr 6, 2020

View reviewed changes

feat: fix review changes

96cdd7c

elharo reviewed Apr 7, 2020

View reviewed changes

feat: fix javadoc

8ad5c75

elharo approved these changes Apr 7, 2020

View reviewed changes

athakor merged commit 317a172 into googleapis:master Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement testPermission api of organizations#125

feat: implement testPermission api of organizations#125
athakor merged 5 commits intogoogleapis:masterfrom
athakor:resourcemanager-testPermission-89

athakor commented Mar 30, 2020 •

edited

Loading

Uh oh!

codecov bot commented Mar 30, 2020 •

edited

Loading

Uh oh!

athakor commented Mar 30, 2020 •

edited

Loading

Uh oh!

elharo Apr 6, 2020

Uh oh!

elharo Apr 6, 2020

Uh oh!

elharo Apr 6, 2020

Uh oh!

elharo Apr 6, 2020

Uh oh!

elharo Apr 6, 2020

Uh oh!

athakor commented Apr 7, 2020

Uh oh!

elharo Apr 7, 2020

Uh oh!

athakor Apr 7, 2020

Uh oh!

elharo Apr 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

athakor commented Mar 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

athakor commented Mar 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

athakor commented Apr 7, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

athakor commented Mar 30, 2020 •

edited

Loading

codecov bot commented Mar 30, 2020 •

edited

Loading

athakor commented Mar 30, 2020 •

edited

Loading