Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Git module clone #287

Merged
merged 4 commits into from
Oct 7, 2020
Merged

Git module clone #287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6c524a3
Add git-clone to the git module.
paconte Sep 22, 2020
2f9c98a
Avoid UnsopportedOperationException when inserting elements in a fixe…
paconte Sep 22, 2020
f2976dc
Fix checkstyle errors.
paconte Sep 24, 2020
4bf414e
Remove tabs at the beguinning of the line.
paconte Oct 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions src/main/java/com/suse/salt/netapi/calls/modules/Git.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.suse.salt.netapi.results.GitResult;
import com.google.gson.reflect.TypeToken;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
Expand All @@ -18,23 +19,23 @@ public class Git {
private Git() { }

public static LocalCall<GitResult> status(String cwd, Optional<String> user) {
List<String> args = Arrays.asList(cwd);
List<String> args = new ArrayList<>(Arrays.asList(cwd));
user.ifPresent(usr -> args.add(usr));
return new LocalCall<>("git.status", Optional.of(args),
Optional.of(emptyMap()), new TypeToken<GitResult>(){});
}

public static LocalCall<String> add(String cwd, String filename,
String opts, String gitOpts, Optional<String> user) {
List<String> args = Arrays.asList(cwd, filename, opts, gitOpts);
List<String> args = new ArrayList<>(Arrays.asList(cwd, filename, opts, gitOpts));
user.ifPresent(usr -> args.add(usr));
return new LocalCall<>("git.add", Optional.of(args),
Optional.of(emptyMap()), new TypeToken<String>(){});
}

public static LocalCall<String> commit(String cwd, String message,
String opts, String gitOpts, Optional<String> user, Optional<String> filename) {
List<String> args = Arrays.asList(cwd, message, opts, gitOpts);
List<String> args = new ArrayList<>(Arrays.asList(cwd, message, opts, gitOpts));
user.ifPresent(usr -> args.add(usr));
filename.ifPresent(file -> args.add(file));
return new LocalCall<>("git.commit", Optional.of(args),
Expand All @@ -43,9 +44,32 @@ public static LocalCall<String> commit(String cwd, String message,

public static LocalCall<Boolean> branch(String cwd, String branch,
String opts, String gitOpts, Optional<String> user) {
List<String> args = Arrays.asList(cwd, branch, opts, gitOpts);
List<String> args = new ArrayList<>(Arrays.asList(cwd, branch, opts, gitOpts));
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why this would be needed (here and above), wouldn't Arrays.asList() already create and return a list of strings as List<String>?

Copy link
Contributor Author

@paconte paconte Oct 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, you are right. It returns a list of strings as List<String>. However the size of the list is fixed, this will not allow us to add other arguments to the list. Below the documentation of the method:


/**
     * Returns a fixed-size list backed by the specified array.  (Changes to
     * the returned list "write through" to the array.)  This method acts
     * as bridge between array-based and collection-based APIs, in
     * combination with {@link Collection#toArray}.  The returned list is
     * serializable and implements {@link RandomAccess}.
     *
     * <p>This method also provides a convenient way to create a fixed-size
     * list initialized to contain several elements:
     * <pre>
     *     List&lt;String&gt; stooges = Arrays.asList("Larry", "Moe", "Curly");
     * </pre>
     *
     * @param <T> the class of the objects in the array
     * @param a the array by which the list will be backed
     * @return a list view of the specified array
     */
    @SafeVarargs
    @SuppressWarnings("varargs")
    public static <T> List<T> asList(T... a) {
        return new ArrayList<>(a);
    }

If you add an element to that list, the application will throw an Exception. The current code actually send an exception, maybe it was not well tested. This is not the only line of code where this happens, so I made that change on in different places of the git module. See: 2f9c98a

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, it makes sense!

user.ifPresent(usr -> args.add(usr));
return new LocalCall<>("git.branch", Optional.of(args),
Optional.of(emptyMap()), new TypeToken<Boolean>(){});
}

public static LocalCall<Boolean> clone(String cwd, String url, Optional<String> name, String opts, String gitOpts,
Optional<String> user, Optional<String> httpsUser, Optional<String>httpsPass) {

List<String> args = new ArrayList<>(Arrays.asList(cwd, url));
args.add(name.orElse(null));
args.add(opts);
args.add(gitOpts);
args.add(user.orElse(null));
args.add(null); //password (only windows)
args.add(null); //identity (to implement)
args.add(httpsUser.orElse(null));
args.add(httpsPass.orElse(null));

LocalCall<Boolean> run = new LocalCall<>(
"git.clone", Optional.of(args), Optional.of(emptyMap()), new TypeToken<Boolean>(){});
// set a higher timeout if HTTPS user and pass are given, otherwise no timeout
if (httpsUser.isPresent()) {
return run.withTimeouts(Optional.of(4), Optional.of(1));
} else {
return run;
}
}
}
63 changes: 63 additions & 0 deletions src/test/java/com/suse/salt/netapi/calls/modules/GitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
import static org.junit.Assert.assertTrue;

import com.github.tomakehurst.wiremock.junit.WireMockRule;
import com.google.gson.JsonPrimitive;
import com.suse.salt.netapi.calls.LocalCall;
import com.suse.salt.netapi.client.SaltClient;
import com.suse.salt.netapi.client.impl.HttpAsyncClientImpl;
import com.suse.salt.netapi.datatypes.AuthMethod;
import com.suse.salt.netapi.datatypes.Token;
import com.suse.salt.netapi.datatypes.target.MinionList;
import com.suse.salt.netapi.errors.JsonParsingError;
import com.suse.salt.netapi.results.GitResult;
import com.suse.salt.netapi.results.Result;
import com.suse.salt.netapi.utils.ClientUtils;
Expand Down Expand Up @@ -54,6 +56,12 @@ public class GitTest {
static final String JSON_BRANCH_RESPONSE = ClientUtils.streamToString(
SaltUtilTest.class.getResourceAsStream("/modules/git/git_branch.json"));

static final String JSON_CLONE_SUCCESS_RESPONSE = ClientUtils.streamToString(
SaltUtilTest.class.getResourceAsStream("/modules/git/git_clone_success.json"));

static final String JSON_CLONE_ERROR_RESPONSE = ClientUtils.streamToString(
SaltUtilTest.class.getResourceAsStream("/modules/git/git_clone_error.json"));

static final AuthMethod AUTH = new AuthMethod(new Token());

private SaltClient client;
Expand Down Expand Up @@ -178,6 +186,61 @@ public void testBranch() {
assertEquals(true, output);
}

@Test
public void testCloneSuccess() {
// First we get the call to use in the tests
LocalCall<Boolean> call = Git.clone(
"/dev",
"https://github.com/SUSE/salt-netapi-client.git",
Optional.empty(),
"", "",
Optional.empty(), Optional.empty(), Optional.empty());
assertEquals("git.clone", call.getPayload().get("fun"));

// Test with a successful response
mockOkResponseWith(JSON_CLONE_SUCCESS_RESPONSE);

Map<String, Result<Boolean>> response =
call.callSync(client, new MinionList("myminion"), AUTH)
.toCompletableFuture().join();

assertNotNull(response.get("myminion"));

Boolean output = response.get("myminion").result().get();
assertEquals(true, output);
}

@Test
public void testCloneError() {
// First we get the call to use in the tests
LocalCall<Boolean> call = Git.clone(
"/dev",
"https://github.com/SUSE/salt-netapi-client.git",
Optional.empty(),
"", "",
Optional.empty(), Optional.empty(), Optional.empty());
assertEquals("git.clone", call.getPayload().get("fun"));

// Test with a error response
mockOkResponseWith(JSON_CLONE_ERROR_RESPONSE);

Map<String, Result<Boolean>> response =
call.callSync(client, new MinionList("myminion"), AUTH)
.toCompletableFuture().join();

assertNotNull(response.get("myminion"));


Boolean output = response.get("myminion").result().orElse(Boolean.FALSE);
assertEquals(false, output);
String errorMessage =
"ERROR: Command 'git clone -- https://github.com/SUSE/salt-netapi-client.error netapi' " +
"failed: Cloning into 'netapi'...\nfatal: could not read Username for 'https://github.com': " +
"No such device or address";
assertEquals(new JsonPrimitive(errorMessage),
((JsonParsingError) response.get("myminion").error().get()).getJson());
}

private static void mockOkResponseWith(String json) {
stubFor(any(urlMatching("/"))
.willReturn(aResponse()
Expand Down
60 changes: 60 additions & 0 deletions src/test/java/com/suse/salt/netapi/examples/GitModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package com.suse.salt.netapi.examples;

import com.suse.salt.netapi.AuthModule;
import com.suse.salt.netapi.calls.LocalCall;
import com.suse.salt.netapi.calls.modules.Git;
import com.suse.salt.netapi.client.SaltClient;
import com.suse.salt.netapi.client.impl.HttpAsyncClientImpl;
import com.suse.salt.netapi.datatypes.AuthMethod;
import com.suse.salt.netapi.datatypes.Token;
import com.suse.salt.netapi.datatypes.target.MinionList;
import com.suse.salt.netapi.results.Result;
import com.suse.salt.netapi.utils.HttpClientUtils;

import java.net.URI;
import java.util.Map;
import java.util.Optional;

public class GitModule {

// salt netapi parameters
private static final String SALT_API_URL = "http://192.168.1.12:8000";
private static final String USER = "gitUser";
private static final String PASSWORD = "gitUser";
// git.clone parameters
private static final String MINION_ID = "my_minion_id";
private static final String GIT_URL = "https://github.com/SUSE/salt-netapi-client.git";
private static final String CWD = "/home/saltuser/dev/";
private static final String NAME = "java_netapi";
private static final String HTTPS_USER = "saltgit";
private static final String HTTPS_PASS = "saltgit";

public static void main(String[] args) {
// the client
SaltClient client =
new SaltClient(URI.create(SALT_API_URL),
new HttpAsyncClientImpl(HttpClientUtils.defaultClient()));
Token token = client.login(USER, PASSWORD, AuthModule.PAM).toCompletableFuture().join();
AuthMethod tokenAuth = new AuthMethod(token);

// Cloning a repository via https with user name and password
LocalCall<Boolean> call = Git.clone(
CWD,
GIT_URL,
Optional.of(NAME),
"",
"",
Optional.empty(), Optional.of(HTTPS_USER), Optional.of(HTTPS_PASS));
// substitute above line for the below line of no user and password
// Optional.empty(), Optional.empty(), Optional.empty());

Map<String, Result<Boolean>> results =
call.callSync(client, new MinionList(MINION_ID), tokenAuth).toCompletableFuture().join();

System.out.println("Response from minions:");
results.forEach((minion, result) -> {
System.out.println(minion + " -> " + result.result().orElse(Boolean.FALSE));
result.error().ifPresent(err -> System.out.println(result.error()));
});
}
}
1 change: 1 addition & 0 deletions src/test/resources/modules/git/git_clone_error.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"return":[{"myminion": "ERROR: Command 'git clone -- https://github.com/SUSE/salt-netapi-client.error netapi' failed: Cloning into 'netapi'...\nfatal: could not read Username for 'https://github.com': No such device or address"}]}
1 change: 1 addition & 0 deletions src/test/resources/modules/git/git_clone_success.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"return":[{"myminion": true}]}