From 3f21a2d0ed539d465e1e03614ec9e47973049cb3 Mon Sep 17 00:00:00 2001 From: yuhuyoyo Date: Fri, 22 Jul 2022 13:55:10 -0400 Subject: [PATCH 1/4] created date and last udpated date can be null --- .../userfacing/resource/UFDataCollection.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/main/java/bio/terra/cli/serialization/userfacing/resource/UFDataCollection.java b/src/main/java/bio/terra/cli/serialization/userfacing/resource/UFDataCollection.java index 6c198e801..02994a1e2 100644 --- a/src/main/java/bio/terra/cli/serialization/userfacing/resource/UFDataCollection.java +++ b/src/main/java/bio/terra/cli/serialization/userfacing/resource/UFDataCollection.java @@ -16,6 +16,9 @@ import java.util.UUID; import java.util.function.Function; import java.util.stream.Collectors; +import javax.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * External representation of a data collection for command input/output. @@ -26,6 +29,7 @@ */ @JsonDeserialize(builder = UFDataCollection.Builder.class) public class UFDataCollection extends UFResource { + private static final Logger logger = LoggerFactory.getLogger(UFDataCollection.class); public final UUID dataCollectionWorkspaceUuid; // Fields from PF-1703 // TODO(PF-1724): After PF-1738 is done, add Created On and Last Updated @@ -33,8 +37,8 @@ public class UFDataCollection extends UFResource { public final String shortDescription; public final String version; public final List resources; - public final OffsetDateTime createdDate; - public final OffsetDateTime lastUpdatedDate; + public final @Nullable OffsetDateTime createdDate; + public final @Nullable OffsetDateTime lastUpdatedDate; /** Serialize an instance of the internal class to the command format. */ public UFDataCollection(DataCollection internalObj) { @@ -183,7 +187,11 @@ public UFDataCollection build() { public Builder() {} } - private void printDate(String prefix, String dateLabel, OffsetDateTime dateTime) { + private void printDate(String prefix, String dateLabel, @Nullable OffsetDateTime dateTime) { + if (dateTime == null) { + logger.warn("datetime is null"); + return; + } UserIO.getOut() .println(prefix + dateLabel + ":\t\t" + dateTime.format(DateTimeFormatter.ISO_LOCAL_DATE)); } From b15edf3f5c2be4e5e547083cc433f7a1fa15eeae Mon Sep 17 00:00:00 2001 From: yuhuyoyo Date: Fri, 22 Jul 2022 13:58:31 -0400 Subject: [PATCH 2/4] update comment --- .../cli/serialization/userfacing/resource/UFDataCollection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/bio/terra/cli/serialization/userfacing/resource/UFDataCollection.java b/src/main/java/bio/terra/cli/serialization/userfacing/resource/UFDataCollection.java index 02994a1e2..656156819 100644 --- a/src/main/java/bio/terra/cli/serialization/userfacing/resource/UFDataCollection.java +++ b/src/main/java/bio/terra/cli/serialization/userfacing/resource/UFDataCollection.java @@ -189,7 +189,7 @@ public Builder() {} private void printDate(String prefix, String dateLabel, @Nullable OffsetDateTime dateTime) { if (dateTime == null) { - logger.warn("datetime is null"); + logger.warn(String.format("datetime for %s is null", dateLabel)); return; } UserIO.getOut() From 91bfc5e2f54ca8b575c8d9adef0bb3e22fd991fc Mon Sep 17 00:00:00 2001 From: yuhuyoyo Date: Wed, 27 Jul 2022 10:30:08 -0400 Subject: [PATCH 3/4] throw user actionable exception when there is no ssh key. --- .../cli/command/resource/addref/GitRepo.java | 18 +++++++++++++ .../cli/command/user/sshkey/Generate.java | 3 +-- src/test/java/unit/GitRepoReferenced.java | 25 ++++++------------- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/main/java/bio/terra/cli/command/resource/addref/GitRepo.java b/src/main/java/bio/terra/cli/command/resource/addref/GitRepo.java index e4acd0701..80a5cde30 100644 --- a/src/main/java/bio/terra/cli/command/resource/addref/GitRepo.java +++ b/src/main/java/bio/terra/cli/command/resource/addref/GitRepo.java @@ -4,9 +4,15 @@ import bio.terra.cli.command.shared.options.Format; import bio.terra.cli.command.shared.options.ReferencedResourceCreation; import bio.terra.cli.command.shared.options.WorkspaceOverride; +import bio.terra.cli.exception.SystemException; +import bio.terra.cli.exception.UserActionableException; import bio.terra.cli.serialization.userfacing.input.AddGitRepoParams; import bio.terra.cli.serialization.userfacing.input.CreateResourceParams; import bio.terra.cli.serialization.userfacing.resource.UFGitRepo; +import bio.terra.cli.service.ExternalCredentialsManagerService; +import bio.terra.externalcreds.model.SshKeyPairType; +import org.springframework.http.HttpStatus; +import org.springframework.web.client.HttpStatusCodeException; import picocli.CommandLine; /** This class corresponds to the fourth-level "terra resource add-ref git-repo" command. */ @@ -41,6 +47,18 @@ protected void execute() { bio.terra.cli.businessobject.resource.GitRepo addedResource = bio.terra.cli.businessobject.resource.GitRepo.addReferenced(createParams.build()); formatOption.printReturnValue(new UFGitRepo(addedResource), GitRepo::printText); + ExternalCredentialsManagerService ecmService = ExternalCredentialsManagerService.fromContext(); + try { + ecmService.getSshKeyPair(SshKeyPairType.GITHUB); + } catch (SystemException e) { + if (e.getCause() instanceof HttpStatusCodeException + && ((HttpStatusCodeException) e.getCause()).getStatusCode() == HttpStatus.NOT_FOUND) { + throw new UserActionableException( + "You do not have a Terra ssh key, cloning the git repo in the GCP notebook will" + + " fail. Please run `terra user ssh-key generate` and store the output (public key) in" + + " your GitHub account https://github.com/settings/keys."); + } + } } /** Print this command's output in text format. */ diff --git a/src/main/java/bio/terra/cli/command/user/sshkey/Generate.java b/src/main/java/bio/terra/cli/command/user/sshkey/Generate.java index dddcac6a3..ab107fe3f 100644 --- a/src/main/java/bio/terra/cli/command/user/sshkey/Generate.java +++ b/src/main/java/bio/terra/cli/command/user/sshkey/Generate.java @@ -30,8 +30,7 @@ protected void execute() { confirmationPrompt.confirmOrThrow( "Generating a new Terra SSH key will replace the old Terra SSH key if it exists. " + "You must associate the new SSH public key with your GitHub account using " - + "https://docs.github.com/en/authentication/connecting-to-github-with-ssh/generating-a-new-ssh-key-and-adding-it-to-the-ssh-agent#adding-your-ssh-key-to-the-ssh-agent. " - + "Are you sure you want to proceed (y/N)?", + + "https://github.com/settings/keys. Are you sure you want to proceed (y/N)?", "Generating new SSH key is aborted"); var ecmService = ExternalCredentialsManagerService.fromContext(); var sshKeyPair = ecmService.generateSshKeyPair(SshKeyPairType.GITHUB); diff --git a/src/test/java/unit/GitRepoReferenced.java b/src/test/java/unit/GitRepoReferenced.java index 4179fc518..7b0d17bc8 100644 --- a/src/test/java/unit/GitRepoReferenced.java +++ b/src/test/java/unit/GitRepoReferenced.java @@ -30,6 +30,7 @@ public class GitRepoReferenced extends SingleWorkspaceUnit { @DisplayName("list and describe reflect adding a new referenced git repo") void listDescribeReflectAdd() throws IOException { workspaceCreator.login(); + TestCommand.runCommandExpectSuccess("user", "ssh-key", "generate", "--quiet"); // `terra workspace set --id=$id` TestCommand.runCommandExpectSuccess("workspace", "set", "--id=" + getUserFacingId()); @@ -86,13 +87,8 @@ void resolve() throws IOException { // `terra resource add-ref git-repo --name=$name --repo-url=$repoUrl String name = "resolve"; - TestCommand.runAndParseCommandExpectSuccess( - UFGitRepo.class, - "resource", - "add-ref", - "git-repo", - "--name=" + name, - "--repo-url=" + GIT_REPO_SSH_URL); + TestCommand.runCommandExpectSuccess( + "resource", "add-ref", "git-repo", "--name=" + name, "--repo-url=" + GIT_REPO_SSH_URL); // `terra resource resolve --name=$name --format=json` JSONObject resolved = @@ -113,13 +109,8 @@ void listReflectsDelete() throws IOException { // `terra resource add-ref git-repo --name=$name --repo-url=$repoUrl String name = "listReflectsDelete"; - TestCommand.runAndParseCommandExpectSuccess( - UFGitRepo.class, - "resource", - "add-ref", - "git-repo", - "--name=" + name, - "--repo-url=" + GIT_REPO_SSH_URL); + TestCommand.runCommandExpectSuccess( + "resource", "add-ref", "git-repo", "--name=" + name, "--repo-url=" + GIT_REPO_SSH_URL); // `terra resource delete --name=$name --format=json` TestCommand.runCommandExpectSuccess("resource", "delete", "--name=" + name, "--quiet"); @@ -189,8 +180,7 @@ void updateIndividualProperties() throws IOException { // `terra resource add-ref git-repo --name=$name --repo-url=$repoUrl --description=$description` String name = "updateIndividualProperties"; String description = "updateDescription"; - TestCommand.runAndParseCommandExpectSuccess( - UFGitRepo.class, + TestCommand.runCommandExpectSuccess( "resource", "add-ref", "git-repo", @@ -269,8 +259,7 @@ void updateMultipleOrNoProperties() throws IOException { // --repo-url=$gitUrl String name = "updateMultipleOrNoProperties"; String description = "updateDescription"; - TestCommand.runAndParseCommandExpectSuccess( - UFGitRepo.class, + TestCommand.runCommandExpectSuccess( "resource", "add-ref", "git-repo", From 8ceff66d5fd6fbff95ec8e967a75da4e6932bfab Mon Sep 17 00:00:00 2001 From: yuhuyoyo Date: Fri, 29 Jul 2022 08:24:13 -0400 Subject: [PATCH 4/4] add comment --- src/test/java/unit/GitRepoReferenced.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/unit/GitRepoReferenced.java b/src/test/java/unit/GitRepoReferenced.java index 7b0d17bc8..16c7a6cc1 100644 --- a/src/test/java/unit/GitRepoReferenced.java +++ b/src/test/java/unit/GitRepoReferenced.java @@ -30,6 +30,8 @@ public class GitRepoReferenced extends SingleWorkspaceUnit { @DisplayName("list and describe reflect adding a new referenced git repo") void listDescribeReflectAdd() throws IOException { workspaceCreator.login(); + + // Generate ssh key to avoid prompt about ssh key TestCommand.runCommandExpectSuccess("user", "ssh-key", "generate", "--quiet"); // `terra workspace set --id=$id`