Support API V2 : Get / Create / Delete Permission targets - Issue #256#383
Support API V2 : Get / Create / Delete Permission targets - Issue #256#383
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
yahavi
added
new feature
github-actions
bot
removed
the
safe to test
|
Nice contribution 👍 ! Would be nice if could be added ! |
|
@yahavi Is it possible to have an ETA (future 2.x version linked) on this pull-request 😊 ? Or if any update should be done ? |
eyalbe4
left a comment
eyalbe4
left a comment
There was a problem hiding this comment.
Let's also add the new APIs to the README.
| PermissionTargetV2Builder build(PermissionV2 build); | ||
| PermissionTargetV2Builder releaseBundle(PermissionV2 releaseBundle); | ||
| PermissionTargetV2 build(); | ||
|
|
| if(permissionTarget == null || StringUtils.isBlank(permissionTarget.getName())){ | ||
| throw new IllegalArgumentException("Permission target name is required") | ||
| } | ||
| if (permissionTarget.getRepo() == null || permissionTarget.getRepo().getRepositories()==null || permissionTarget.getRepo().getRepositories().isEmpty()) { |
There was a problem hiding this comment.
Missing white-spaces before null
permissionTarget.getRepo().getRepositories()==null
| if (permissionTarget.getRepo() == null || permissionTarget.getRepo().getRepositories()==null || permissionTarget.getRepo().getRepositories().isEmpty()) { | ||
| throw new UnsupportedOperationException("At least 1 repository is required in permission target (could be 'ANY', 'ANY LOCAL', 'ANY REMOTE')") | ||
| } | ||
| if(permissionTarget.getBuild() != null && permissionTarget.getBuild().getRepositories() !=null){ |
There was a problem hiding this comment.
Missing white-space after if
if(permissionTarget.getBuild()
| if (permissionTarget.getRepo() == null || permissionTarget.getRepo().getRepositories()==null || permissionTarget.getRepo().getRepositories().isEmpty()) { | ||
| throw new UnsupportedOperationException("At least 1 repository is required in permission target (could be 'ANY', 'ANY LOCAL', 'ANY REMOTE')") | ||
| } | ||
| if(permissionTarget.getBuild() != null && permissionTarget.getBuild().getRepositories() !=null){ |
There was a problem hiding this comment.
Missing white-space before {
getBuild().getRepositories() !=null){
| } | ||
| if(permissionTarget.getBuild() != null && permissionTarget.getBuild().getRepositories() !=null){ | ||
| List<String> buildRepositories = permissionTarget.getBuild().getRepositories(); | ||
| if(buildRepositories.size() !=1 || !buildRepositories.contains("artifactory-build-info")){ |
There was a problem hiding this comment.
Missing white-spaces in this line.
| import org.jfrog.artifactory.client.model.PermissionV2; | ||
|
|
||
| import java.util.Optional; | ||
|
|
There was a problem hiding this comment.
Missing new-lines according to the code conventions in a few places in this file.
|
|
||
| public class PermissionV2BuilderImpl implements PermissionV2Builder { | ||
|
|
||
|
|
| } | ||
| } | ||
| } | ||
| protected void deletePermissionTargetV2IfExists(String name){ |
There was a problem hiding this comment.
Missing new-line between methods.
| final String GROUP_TEST_2="group_test_2_"+System.currentTimeMillis(); | ||
| final String PERMISSION_TARGET_NAME="permission_target_v2_"+System.currentTimeMillis(); | ||
| try{ | ||
| //Eventual clean up of previous test run |
There was a problem hiding this comment.
Missing white-space after //
(The same appliues for the other comments in this file).
| final String GROUP_TEST="group_test_"+System.currentTimeMillis(); | ||
| final String PERMISSION_TARGET_NAME="permission_target_v2_"+System.currentTimeMillis(); | ||
| try{ | ||
| // remove eventual previous test run leftovers |
There was a problem hiding this comment.
Let's have all comments start with an upper-case.
4 participants
Hello team !
This PR provides support for API V2 Permission target management : GET / CREATE_OR_UPDATE / DELETE
It respects code actual architecture and added features are tested.
Thanks !