Skip to content
This repository was archived by the owner on Jun 3, 2025. It is now read-only.

Commit 213400e

Browse files
author
Irene
committed
Fix tests & remove TODOs
Fix the program not shutting down due to threads created by EventStore Fix error messages Configure logger to not log to stdout Sending data requires course information: load course information before sending, which means data analytics is only sent if the user is in a course directory
1 parent a833372 commit 213400e

19 files changed

Lines changed: 161 additions & 187 deletions

‎src/main/java/fi/helsinki/cs/tmc/cli/Application.java‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,20 @@
22

33
import fi.helsinki.cs.tmc.cli.analytics.AnalyticsFacade;
44
import fi.helsinki.cs.tmc.cli.analytics.TimeTracker;
5+
import fi.helsinki.cs.tmc.cli.backend.CourseInfo;
56
import fi.helsinki.cs.tmc.cli.backend.Settings;
67
import fi.helsinki.cs.tmc.cli.command.SubmitCommand;
78
import fi.helsinki.cs.tmc.cli.core.AbstractCommand;
89
import fi.helsinki.cs.tmc.cli.core.CliContext;
9-
import fi.helsinki.cs.tmc.cli.core.CommandFactory;
1010
import fi.helsinki.cs.tmc.cli.io.*;
1111
import fi.helsinki.cs.tmc.cli.updater.AutoUpdater;
12+
import fi.helsinki.cs.tmc.cli.core.CommandFactory;
1213

14+
15+
import fi.helsinki.cs.tmc.cli.utils.OptionalToGoptional;
1316
import fi.helsinki.cs.tmc.core.TmcCore;
17+
import fi.helsinki.cs.tmc.core.holders.TmcSettingsHolder;
18+
import fi.helsinki.cs.tmc.core.utilities.TmcRequestProcessor;
1419
import fi.helsinki.cs.tmc.langs.util.TaskExecutor;
1520
import fi.helsinki.cs.tmc.langs.util.TaskExecutorImpl;
1621
import fi.helsinki.cs.tmc.spyware.EventSendBuffer;
@@ -98,6 +103,13 @@ private boolean runCommand(String name, String[] args) {
98103
private Optional<Thread> sendAnalytics(AbstractCommand command) {
99104
Optional<Thread> thread = Optional.empty();
100105
if (command instanceof SubmitCommand || timeTracker.anHourHasPassedSinceLastSubmit()) {
106+
this.context.loadUserInformation();
107+
// get course info returns null
108+
CourseInfo courseInfo = this.context.getCourseInfo();
109+
if (courseInfo == null) {
110+
return Optional.empty();
111+
}
112+
TmcSettingsHolder.get().setCourse(OptionalToGoptional.convert(Optional.of(courseInfo.getCourse())));
101113
thread = this.context.getAnalyticsFacade().sendAnalytics();
102114
timeTracker.restart();
103115
}
@@ -199,7 +211,6 @@ public void run(String[] args) {
199211
}
200212
}
201213

202-
203214
public static void main(String[] args) {
204215
Settings settings = new Settings();
205216
TaskExecutor tmcLangs = new TaskExecutorImpl();
@@ -208,6 +219,8 @@ public static void main(String[] args) {
208219
AnalyticsFacade analyticsFacade = new AnalyticsFacade(settings, eventSendBuffer);
209220
Application app = new Application(new CliContext(null, core, new WorkDir(), settings, analyticsFacade));
210221
app.run(args);
222+
// Because of EventSendBuffer
223+
TmcRequestProcessor.instance.shutdown();
211224
}
212225

213226
private boolean versionCheck() {
@@ -220,8 +233,8 @@ private boolean versionCheck() {
220233
try {
221234
time = Long.parseLong(previousTimestamp);
222235
} catch (NumberFormatException ex) {
223-
io.errorln("The previous update date isn't number.");
224-
logger.warn("The previous update date isn't number.", ex);
236+
io.errorln("The previous update date isn't a number.");
237+
logger.warn("The previous update date isn't a number.", ex);
225238
return false;
226239
}
227240
previous = new Date(time);
@@ -247,24 +260,4 @@ public boolean runAutoUpdate() {
247260
return updated;
248261
}
249262

250-
//TODO rename this as getColorProperty and move it somewhere else
251-
public Color getColor(String propertyName) {
252-
String propertyValue = context.getProperties().get(propertyName);
253-
Color color = ColorUtil.getColor(propertyValue);
254-
if (color == null) {
255-
switch (propertyName) {
256-
case "progressbar-left":
257-
return Color.CYAN;
258-
case "progressbar-right":
259-
return Color.CYAN;
260-
case "testresults-left":
261-
return Color.GREEN;
262-
case "testresults-right":
263-
return Color.RED;
264-
default:
265-
return Color.NONE;
266-
}
267-
}
268-
return color;
269-
}
270263
}

‎src/main/java/fi/helsinki/cs/tmc/cli/analytics/AnalyticsFacade.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import fi.helsinki.cs.tmc.spyware.SpywareSettings;
1111

1212
import java.awt.*;
13+
import java.security.spec.ECField;
1314
import java.util.List;
1415
import java.util.Map;
1516
import java.util.Optional;
@@ -18,7 +19,6 @@
1819
public class AnalyticsFacade {
1920
private EventSendBuffer eventSendBuffer;
2021
private SpywareSettings settings;
21-
private static final Logger log = Logger.getLogger(AnalyticsFacade.class.getName());
2222

2323
public AnalyticsFacade(SpywareSettings settings, EventSendBuffer eventSendBuffer) {
2424
this.settings = settings;

‎src/main/java/fi/helsinki/cs/tmc/cli/backend/AccountList.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ public AccountList() {
1616
this.accountArray = new ArrayList<>();
1717
}
1818

19-
//TODO This should not be used
2019
public Account getAccount() {
2120
if (this.accountArray.size() > 0) {
2221
// Get last used account by default

‎src/main/java/fi/helsinki/cs/tmc/cli/backend/CourseInfo.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ public List<String> getExerciseNames() {
7777
return names;
7878
}
7979

80-
//TODO This is exactly same method as TmcUtil.findExercise(course, name)
8180
public Exercise getExercise(String name) {
8281
for (Exercise exercise : this.course.getExercises()) {
8382
if (exercise.getName().equals(name)) {

‎src/main/java/fi/helsinki/cs/tmc/cli/backend/CourseInfoIo.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package fi.helsinki.cs.tmc.cli.backend;
22

3+
import com.google.common.base.Optional;
34
import fi.helsinki.cs.tmc.core.domain.Course;
45

56
import com.google.gson.Gson;
7+
import fi.helsinki.cs.tmc.core.holders.TmcSettingsHolder;
68
import org.slf4j.Logger;
79
import org.slf4j.LoggerFactory;
810

‎src/main/java/fi/helsinki/cs/tmc/cli/backend/TmcUtil.java‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ public static boolean tryToLogin(CliContext ctx, Account account, String passwor
5959
TmcCore core = ctx.getTmcCore();
6060
ctx.useAccount(account);
6161
Callable<Void> callable = core.authenticate(ProgressObserver.NULL_OBSERVER, password);
62-
//TODO restore the settings object
6362

6463
try {
6564
callable.call();
@@ -123,19 +122,6 @@ public static Course findCourse(CliContext ctx, String name) {
123122
return null;
124123
}
125124

126-
//TODO This is exactly same method as CourseInfo.getExercise(course, name)
127-
public static Exercise findExercise(Course course, String name) {
128-
List<Exercise> exercises;
129-
exercises = course.getExercises();
130-
131-
for (Exercise item : exercises) {
132-
if (item.getName().equals(name)) {
133-
return item;
134-
}
135-
}
136-
return null;
137-
}
138-
139125
public static List<Exercise> downloadExercises(
140126
CliContext ctx, List<Exercise> exercises, ProgressObserver progobs) {
141127
try {
@@ -272,7 +258,6 @@ private static void handleTmcExceptions(CliContext ctx, Exception exception) {
272258
}
273259

274260
logger.error("Command failed in tmc-core", exception);
275-
//TODO we seem to write twice error message; here and in the commands.
276261
io.errorln("Command failed, check tmc-cli.log file for more info");
277262
}
278263

‎src/main/java/fi/helsinki/cs/tmc/cli/command/ConfigCommand.java‎

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,7 @@
1515
import org.apache.commons.cli.CommandLine;
1616
import org.apache.commons.cli.Options;
1717

18-
import java.util.HashMap;
19-
import java.util.Map;
20-
import java.util.Set;
21-
import java.util.Arrays;
22-
import java.util.HashSet;
18+
import java.util.*;
2319

2420
@Command(name = "config", desc = "Set/unset TMC-CLI properties")
2521
public class ConfigCommand extends AbstractCommand {
@@ -122,34 +118,34 @@ public void setter(String value) throws BadValueTypeException {
122118
ALLOWED_KEYS.put("testresults-left", new PropertyFunctions() {
123119
@Override
124120
public String getter() {
125-
return context.getProperties().get("testresults-right");
121+
return context.getProperties().get("testresults-left");
126122
}
127123

128124
@Override
129125
public void setter(String value) throws BadValueTypeException {
130-
addBarColorToProperties("testresults-right", value);
126+
addBarColorToProperties("testresults-left", value);
131127
}
132128
});
133129
ALLOWED_KEYS.put("progressbar-left", new PropertyFunctions() {
134130
@Override
135131
public String getter() {
136-
return context.getProperties().get("testresults-right");
132+
return context.getProperties().get("progressbar-left");
137133
}
138134

139135
@Override
140136
public void setter(String value) throws BadValueTypeException {
141-
addBarColorToProperties("testresults-right", value);
137+
addBarColorToProperties("progressbar-left", value);
142138
}
143139
});
144140
ALLOWED_KEYS.put("progressbar-right", new PropertyFunctions() {
145141
@Override
146142
public String getter() {
147-
return context.getProperties().get("testresults-right");
143+
return context.getProperties().get("progressbar-right");
148144
}
149145

150146
@Override
151147
public void setter(String value) throws BadValueTypeException {
152-
addBarColorToProperties("testresults-right", value);
148+
addBarColorToProperties("progressbar-right", value);
153149
}
154150
});
155151
}
@@ -243,19 +239,21 @@ public void run(CliContext context, CommandLine args) {
243239
}
244240

245241
private void printAllProperties() {
246-
// how to handle that some values are stored in settings and some in properties?
247-
ALLOWED_KEYS.entrySet().stream().forEach(e -> {
248-
String key = e.getKey();
249-
String value = e.getValue().getter();
250-
io.println(key + "=" + ( value != null ? value: "<no value set>" ));
251-
});
242+
ALLOWED_KEYS.entrySet()
243+
.stream()
244+
.sorted(Comparator.comparing(Map.Entry::getKey))
245+
.forEach(e -> {
246+
String key = e.getKey();
247+
String value = e.getValue().getter();
248+
io.println(key + "=" + ( value != null ? value: "<no value set>" ));
249+
});
252250
}
253251

254252
private void deleteProperties(String[] keys) {
255253
if (this.quiet) {
256254
for (String key : keys) {
257-
if (properties.containsKey(key)) {
258-
properties.remove(key);
255+
if (ALLOWED_KEYS.keySet().contains(key) && properties.containsKey(key)) {
256+
properties.remove(key);
259257
}
260258
}
261259
return;
@@ -268,8 +266,8 @@ private void deleteProperties(String[] keys) {
268266
}
269267

270268
for (String key : keys) {
271-
if (!properties.containsKey(key)) {
272-
io.error("Key " + key + " doesn't exist.");
269+
if (!properties.containsKey(key) || !ALLOWED_KEYS.keySet().contains(key)) {
270+
io.error("Key " + key + " doesn't exist or cannot be removed.");
273271
return;
274272
}
275273
}

‎src/main/java/fi/helsinki/cs/tmc/cli/command/DownloadExercisesCommand.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ public void run(CliContext context, CommandLine args) {
8282
// todo: If -c switch, use core.downloadCompletedExercises() to download user's old
8383
// submissions. Not yet implemented in tmc-core.
8484

85-
Color color1 = ctx.getApp().getColor("progressbar-left");
86-
Color color2 = ctx.getApp().getColor("progressbar-right");
85+
Color color1 = ctx.getColorProperty("progressbar-left", ctx.getApp());
86+
Color color2 = ctx.getColorProperty("progressbar-right", ctx.getApp());
8787
CliProgressObserver progobs = new CliProgressObserver(io, color1, color2);
8888

8989
ctx.useAccount(finder.getAccount());

‎src/main/java/fi/helsinki/cs/tmc/cli/command/LoginCommand.java‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,15 @@ public void run(CliContext context, CommandLine args) {
4949
}
5050

5151
if (this.ctx.checkIsLoggedIn(true)) {
52-
io.println("You are already logged in as " + this.ctx.getSettings().getUsername().get());
53-
io.println("Change your organization with the command organization");
52+
io.println("You are already logged in as " + this.ctx.getSettings().getUsername().get() +
53+
(this.ctx.getSettings().getOrganization().isPresent() ?
54+
" and your current organization is " + this.ctx.getSettings().getOrganization().get().getName() :
55+
""));
56+
io.println("Change your organization with the command organization.");
57+
io.println("Run tmc config -l to see your current settings.");
5458
return;
5559
}
5660

57-
this.ctx.getAnalyticsFacade().saveAnalytics("login");
58-
5961
if (!TmcUtil.hasConnection(ctx)) {
6062
io.errorln("You don't have internet connection currently.");
6163
io.errorln("Check the tmc-cli logs if you disagree.");
@@ -70,26 +72,28 @@ public void run(CliContext context, CommandLine args) {
7072
username = getLoginInfo(args, username, "u", "username: ");
7173
password = getLoginInfo(args, null, "p", "password: ");
7274

73-
OrganizationCommand organizationCommand = new OrganizationCommand();
7475
Account account = new Account(username, null);
75-
this.ctx.getSettings().setAccount(account);
76+
ctx.useAccount(account);
7677

7778
if (!TmcUtil.tryToLogin(ctx, account, password)) {
7879
this.ctx.getSettings().setAccount(new Account());
7980
return;
8081
}
8182

83+
OrganizationCommand organizationCommand = new OrganizationCommand();
8284
Optional<Organization> organization = organizationCommand.chooseOrganization(ctx, args);
8385
if (!organization.isPresent()) {
8486
return;
8587
}
8688
account.setOrganization(organization);
8789

8890
boolean sendDiagnostics = getAnswerFromUser(username, account.getServerAddress(),
89-
"Do you want to send crash reports for client development?");
91+
"Do you want to send crash reports for client development?",
92+
this.ctx.getSettings().getSendDiagnostics());
9093
account.setSendDiagnostics(sendDiagnostics);
9194
boolean sendAnalytics = getAnswerFromUser(username, account.getServerAddress(),
92-
"Do you want to send analytics data for research?");
95+
"Do you want to send analytics data for research?",
96+
this.ctx.getSettings().isSpywareEnabled());
9397
account.setSendAnalytics(sendAnalytics);
9498

9599
AccountList list = SettingsIo.loadAccountList();
@@ -99,6 +103,8 @@ public void run(CliContext context, CommandLine args) {
99103
return;
100104
}
101105

106+
this.ctx.getAnalyticsFacade().saveAnalytics("login");
107+
102108
io.println("Login successful.");
103109
}
104110

@@ -122,18 +128,20 @@ private String getLoginInfo(CommandLine line, String oldValue, String option, St
122128
return value;
123129
}
124130

125-
private boolean getAnswerFromUser(String username, String server, String prompt) {
131+
private boolean getAnswerFromUser(String username, String server, String prompt, boolean defaultValue) {
126132
AccountList savedAccounts = SettingsIo.loadAccountList();
127133
if (savedAccounts.getAccount(username, server) != null) {
128-
// not the first time logging in, diagnostics not asked
129-
return this.ctx.getSettings().getSendDiagnostics();
134+
// not the first time logging in
135+
return defaultValue;
130136
}
131137
while (true) {
132-
String sendDiagnostics = io.readLine(prompt + " (y/n) ");
133-
String answer = sendDiagnostics.trim().toLowerCase();
138+
String sendInfo = io.readLine(prompt + " (Y/n) ");
139+
String answer = sendInfo.trim().toLowerCase();
134140
if (answer.isEmpty() || answer.startsWith("y")) {
141+
io.println("Set to yes");
135142
return true;
136143
} else if (answer.startsWith("n")) {
144+
io.println("Set to no");
137145
return false;
138146
}
139147
io.println("Please answer y(es) or n(o).");

‎src/main/java/fi/helsinki/cs/tmc/cli/command/RunTestsCommand.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,10 @@
1919
import org.apache.commons.cli.CommandLine;
2020
import org.apache.commons.cli.Options;
2121

22-
import org.slf4j.Logger;
23-
import org.slf4j.LoggerFactory;
24-
2522
import java.util.List;
2623

2724
@Command(name = "test", desc = "Run local exercise tests")
2825
public class RunTestsCommand extends AbstractCommand {
29-
30-
private static final Logger logger = LoggerFactory.getLogger(RunTestsCommand.class);
31-
3226
private boolean showPassed;
3327
private boolean showDetails;
3428

@@ -65,8 +59,8 @@ public void run(CliContext context, CommandLine args) {
6559

6660
CourseInfo info = context.getCourseInfo();
6761

68-
Color passedColor = context.getApp().getColor("testresults-left");
69-
Color failedColor = context.getApp().getColor("testresults-right");
62+
Color passedColor = context.getColorProperty("testresults-left", context.getApp());
63+
Color failedColor = context.getColorProperty("testresults-right", context.getApp());
7064
ResultPrinter resultPrinter =
7165
new ResultPrinter(io, showDetails, showPassed, passedColor, failedColor);
7266

0 commit comments

Comments
 (0)