Skip to content

Commit

Permalink
Development: Replace usage of standard streams with logger calls (#7399)
Browse files Browse the repository at this point in the history
  • Loading branch information
Strohgelaender authored Oct 21, 2023
1 parent 3bc78df commit c3e3662
Show file tree
Hide file tree
Showing 22 changed files with 53 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
import javax.tools.Diagnostic.Kind;
import javax.tools.JavaFileObject;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.auto.service.AutoService;

// TODO this is not working yet
Expand All @@ -25,12 +28,14 @@
@SupportedSourceVersion(SourceVersion.RELEASE_17)
public class BeanInfoProcessor extends AbstractProcessor {

private final Logger log = LoggerFactory.getLogger(BeanInfoProcessor.class);

@Override
public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
for (TypeElement annotation : annotations) {
Set<? extends Element> annotatedElements = new HashSet<>(roundEnv.getElementsAnnotatedWith(annotation));
annotatedElements.stream().filter(ele -> ele.getKind().isClass()).map(ele -> (TypeElement) ele).flatMap(ele -> findSupers(ele).stream())
.forEach(ele -> generateBeanInfo((TypeElement) ele));
.forEach(ele -> generateBeanInfo(ele));
}
return false;
}
Expand Down Expand Up @@ -69,7 +74,7 @@ private void generateBeanInfo(TypeElement typeElement) {
processingEnv.getMessager().printMessage(Kind.WARNING, "Cannot create " + beanInfoFQN + ", maybe it already exists?");
}
catch (IOException e) {
e.printStackTrace();
log.error("Error while generating bean info", e);
processingEnv.getMessager().printMessage(Kind.ERROR, "IO exception");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public ExecutorService localCIBuildExecutorService() {
log.info("Using ExecutorService with thread pool size {} and a queue size limit of {}.", threadPoolSize, queueSizeLimit);

ThreadFactory customThreadFactory = new ThreadFactoryBuilder().setNameFormat("local-ci-build-%d")
.setUncaughtExceptionHandler((thread, exception) -> log.error("Uncaught exception in thread " + thread.getName(), exception)).build();
.setUncaughtExceptionHandler((thread, exception) -> log.error("Uncaught exception in thread {}", thread.getName(), exception)).build();

RejectedExecutionHandler customRejectedExecutionHandler = (runnable, executor) -> {
throw new RejectedExecutionException("Task " + runnable.toString() + " rejected from " + executor.toString());
Expand Down Expand Up @@ -120,7 +120,7 @@ public DockerClient dockerClient() {
DockerHttpClient httpClient = new ApacheDockerHttpClient.Builder().dockerHost(config.getDockerHost()).sslConfig(config.getSSLConfig()).build();
DockerClient dockerClient = DockerClientImpl.getInstance(config, httpClient);

log.info("Docker client created with connection URI: " + dockerConnectionUri);
log.info("Docker client created with connection URI: {}", dockerConnectionUri);

return dockerClient;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public void overrideBuildPlanNotification(String projectKey, String buildPlanKey
deleteBuildPlanServerNotificationId(buildPlanKey, id);
}
catch (RestClientException e) {
log.error("Could not delete notification with id " + id + " for build plan " + buildPlanKey, e);
log.error("Could not delete notification with id {} for build plan {}", id, buildPlanKey, e);
}
});

Expand All @@ -158,7 +158,7 @@ public void deleteBuildTriggers(String projectKey, String buildPlanKey, VcsRepos
}
for (var id : triggerIds) {
deleteBuildPlanTriggerId(buildPlanKey, id);
log.debug("Deleted trigger with id " + id + " for build plan " + buildPlanKey);
log.debug("Deleted trigger with id {} for build plan {}", id, buildPlanKey);
}
}

Expand All @@ -171,7 +171,7 @@ public void deleteBuildTriggers(String projectKey, String buildPlanKey, VcsRepos
public void checkPrerequisites() throws ContinuousIntegrationException {
Optional<Long> credentialsId = getSharedCredential();
if (credentialsId.isEmpty()) {
log.error("No shared credentials found on Bamboo for git user " + gitUser + ". Migration will fail.");
log.error("No shared credentials found on Bamboo for git user {}. Migration will fail.", gitUser);
throw new ContinuousIntegrationException("No shared credential found for git user " + gitUser
+ " in Bamboo. Migration will fail. Please create a shared username and password credential for this user and run the migration again.");
}
Expand All @@ -183,7 +183,7 @@ public void overrideBuildPlanRepository(String buildPlanId, String name, String
if (this.sharedCredentialId.isEmpty()) {
Optional<Long> credentialsId = getSharedCredential();
if (credentialsId.isEmpty()) {
log.error("No shared credential found for git user " + gitUser + ". Migration will fail.");
log.error("No shared credential found for git user {}. Migration will fail.", gitUser);
throw new ContinuousIntegrationException("No shared credential found for git user " + gitUser
+ " in Bamboo. Migration will fail. Please create a shared username and password credential for this user and run the migration again.");
}
Expand All @@ -196,13 +196,13 @@ public void overrideBuildPlanRepository(String buildPlanId, String name, String
// if we do not find the edge case "solution" repository in the build plan, we simply continue
return;
}
log.info("Repository " + name + " not found for build plan " + buildPlanId + ", will be added now");
log.info("Repository {} not found for build plan {}, will be added now", name, buildPlanId);
}
else {
log.debug("Deleting repository " + name + " for build plan " + buildPlanId);
log.debug("Deleting repository {} for build plan {}", name, buildPlanId);
deleteLinkedRepository(buildPlanId, repositoryId.get());
}
log.debug("Adding repository " + name + " for build plan " + buildPlanId);
log.debug("Adding repository {} for build plan {}", name, buildPlanId);
addGitRepository(buildPlanId, bambooInternalUrlService.toInternalVcsUrl(repositoryUrl), name, this.sharedCredentialId.orElseThrow(), defaultBranch);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void overrideBuildPlanNotification(String projectKey, String buildPlanKey
jenkinsJobService.updateJob(projectKey, buildPlanKey, newConfig);
}
catch (IOException | TransformerException e) {
log.error("Could not fix build plan notification for build plan " + buildPlanKey + " in project " + projectKey, e);
log.error("Could not fix build plan notification for build plan {} in project {}", buildPlanKey, projectKey, e);
throw new JenkinsException(e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ private void evaluateErrorList() {
log.error("Failed to migrate students participations: {}", failedStudentParticipations);
log.warn("Please check the logs for more information. If the issues are related to the external VCS/CI system, fix the issues and rerun the migration. or "
+ "fix the build plans yourself and mark the migration as run. The migration can be rerun by deleting the migration entry in the database table containing "
+ "the migration with author: " + author() + " and date_string: " + date() + " and then restarting Artemis.");
+ "the migration with author: {} and date_string: {} and then restarting Artemis.", author(), date());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ private void evaluateErrorList() {
log.error("Failed to migrate students participations: {}", failedStudentParticipations);
log.warn("Please check the logs for more information. If the issues are related to the external VCS/CI system, fix the issues and rerun the migration. or "
+ "fix the build plans yourself and mark the migration as run. The migration can be rerun by deleting the migration entry in the database table containing "
+ "the migration with author: " + author() + " and date_string: " + date() + " and then restarting Artemis.");
+ "the migration with author: {} and date_string: {} and then restarting Artemis.", author(), date());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ public VcsRepositoryUrl getVcsTemplateRepositoryUrl() {
return new VcsRepositoryUrl(templateRepositoryUrl);
}
catch (URISyntaxException e) {
e.printStackTrace();
log.warn("Cannot create URI for templateRepositoryUrl: {} due to the following error: {}", templateRepositoryUrl, e.getMessage());
}
return null;
}
Expand All @@ -499,7 +499,7 @@ public VcsRepositoryUrl getVcsSolutionRepositoryUrl() {
return new VcsRepositoryUrl(solutionRepositoryUrl);
}
catch (URISyntaxException e) {
e.printStackTrace();
log.warn("Cannot create URI for solutionRepositoryUrl: {} due to the following error: {}", solutionRepositoryUrl, e.getMessage());
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ default VcsRepositoryUrl getVcsRepositoryUrl() {
return new VcsRepositoryUrl(repoUrl);
}
catch (URISyntaxException e) {
e.printStackTrace();
log.warn("Cannot create URI for repositoryUrl: {} due to the following error: {}", repoUrl, e.getMessage());
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private void setupGitLabCIConfiguration(VcsRepositoryUrl repositoryURL, Programm
updateVariable(repositoryPath, VARIABLE_TEST_RESULTS_DIR_NAME, "target/surefire-reports");
}
catch (GitLabApiException e) {
log.error("Error creating variable for " + repositoryURL.toString() + " The variables may already have been created.", e);
log.error("Error creating variable for {} The variables may already have been created.", repositoryURL, e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private User getOrCreateUser(Authentication authentication) {
// We create our own authorization and use the credentials of the user.
byte[] passwordBytes = Utf8.encode(password);
boolean passwordCorrect = ldapTemplate.compare(ldapUserDto.getUid().toString(), "userPassword", passwordBytes);
log.debug("Compare password with LDAP entry for user " + username + " to validate login");
log.debug("Compare password with LDAP entry for user {} to validate login", username);
// this is the normal case, where the password is validated
if (!passwordCorrect) {
throw new BadCredentialsException("Wrong credentials");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,14 @@ private LocalCIBuildResult runScriptAndParseResults(ProgrammingExerciseParticipa

localCIContainerService.startContainer(containerId);

log.info("Started container for build job " + containerName);
log.info("Started container for build job {}", containerName);

localCIContainerService.populateBuildJobContainer(containerId, assignmentRepositoryPath, testsRepositoryPath, auxiliaryRepositoriesPaths, auxiliaryRepositoryNames,
buildScriptPath);

localCIContainerService.runScriptInContainer(containerId);

log.info("Finished running the build script in container " + containerName);
log.info("Finished running the build script in container {}", containerName);

ZonedDateTime buildCompletedDate = ZonedDateTime.now();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ private void processNewPushToTestRepository(ProgrammingExercise exercise, String
// Something went wrong while retrieving the template participation.
// At this point, programmingMessagingService.notifyUserAboutSubmissionError() does not work, because the template participation is not available.
// The instructor will see in the UI that no build of the template repository was conducted and will receive an error message when triggering the build manually.
log.error("Something went wrong while triggering the template build for exercise " + exercise.getId() + " after the solution build was finished.", e);
log.error("Something went wrong while triggering the template build for exercise {} after the solution build was finished.", exercise.getId(), e);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ private Path checkForMatchesInProblemStatementAndCreateDirectoryForFiles(Path ou
}
catch (IOException e) {
exportErrors.add("Could not create directory for embedded files: " + e.getMessage());
log.warn("Could not create directory for embedded files. Won't include embedded files: " + e.getMessage());
log.warn("Could not create directory for embedded files. Won't include embedded files.", e);
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ void sendRelayRequest(String body, String relayServerBaseUrl) {
});
}
catch (RestClientException e) {
log.error("Could not send " + getDeviceType().toString() + " notifications");
log.error("Could not send {} notifications", getDeviceType().toString());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ public ResponseEntity<Resource> exportInstructorExercise(@PathVariable long exer
path = programmingExerciseExportService.exportProgrammingExerciseForDownload(programmingExercise, Collections.synchronizedList(new ArrayList<>()));
}
catch (Exception e) {
log.error("Error while exporting programming exercise with id " + exerciseId + " for instructor", e);
log.error("Error while exporting programming exercise with id {} for instructor", exerciseId, e);
throw new InternalServerErrorException("Error while exporting programming exercise with id " + exerciseId + " for instructor");
}
var finalZipFile = path.toFile();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public ResponseEntity<Void> processNewProgrammingSubmission(@PathVariable("parti
throw ex;
}
catch (VersionControlException ex) {
log.warn("User committed to the wrong branch for participation + " + participationId);
log.warn("User committed to the wrong branch for participation {}", participationId);
return ResponseEntity.status(HttpStatus.OK).build();
}

Expand Down
11 changes: 11 additions & 0 deletions src/test/java/de/tum/in/www1/artemis/ArchitectureTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.tngtech.archunit.base.DescribedPredicate;
import com.tngtech.archunit.core.domain.*;
import com.tngtech.archunit.lang.*;
import com.tngtech.archunit.library.GeneralCodingRules;

import de.tum.in.www1.artemis.service.WebsocketMessagingService;

Expand Down Expand Up @@ -96,6 +97,16 @@ void testFileWriteUsage() {
usage.check(allClasses);
}

@Test
void testLogging() {
GeneralCodingRules.NO_CLASSES_SHOULD_USE_JAVA_UTIL_LOGGING.check(allClasses);

// We currently need to access standard streams in readTestReports() to use the SurefireReportParser
// The ParallelConsoleAppender is used to print test logs to the console (necessary due to parallel test execution)
var classes = allClasses.that(not(simpleName("ProgrammingExerciseTemplateIntegrationTest").or(simpleName("ParallelConsoleAppender"))));
GeneralCodingRules.NO_CLASSES_SHOULD_ACCESS_STANDARD_STREAMS.check(classes);
}

// Custom Predicates for JavaAnnotations since ArchUnit only defines them for classes

private DescribedPredicate<? super JavaAnnotation<?>> simpleNameAnnotation(String name) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
import org.hamcrest.Matchers;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.beans.factory.annotation.Value;
Expand Down Expand Up @@ -47,6 +49,8 @@
@Profile("bitbucket")
public class BitbucketRequestMockProvider {

private final Logger log = LoggerFactory.getLogger(BitbucketRequestMockProvider.class);

@Value("${artemis.version-control.url}")
private URL bitbucketServerUrl;

Expand Down Expand Up @@ -301,7 +305,7 @@ public boolean matches(Object actual) {
return actual.equals(mapper.writeValueAsString(body));
}
catch (JsonProcessingException e) {
e.printStackTrace();
log.warn("Error while parsing", e);
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static com.google.gson.JsonParser.parseString;
import static org.assertj.core.api.Assertions.assertThat;

import java.io.IOException;
import java.time.ZonedDateTime;
import java.util.*;

Expand Down Expand Up @@ -269,7 +270,7 @@ public Submission addModelingSubmissionWithFinishedResultAndAssessor(ModelingExe
return participationUtilService.addSubmissionWithFinishedResultsWithAssessor(participation, submission, assessorLogin);
}

public ModelingSubmission addModelingSubmissionFromResources(ModelingExercise exercise, String path, String login) throws Exception {
public ModelingSubmission addModelingSubmissionFromResources(ModelingExercise exercise, String path, String login) throws IOException {
String model = FileUtils.loadFileFromResources(path);
ModelingSubmission submission = ParticipationFactory.generateModelingSubmission(model, true);
submission = addModelingSubmission(exercise, submission, login);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ private int invokeGradle(boolean recordTestwiseCoverage) {
}
catch (Exception e) {
// printing the cause because this contains the relevant error message (and not a generic one from the connector)
log.error("Error occurred while executing Gradle build: " + e.getCause());
log.error("Error occurred while executing Gradle build.", e.getCause());
return -1;
}
return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ void initTestCase() {
modelingSubmission3 = modelingExerciseUtilService.addModelingSubmissionFromResources(modelingExercise, "test-data/model-submission/model.54745.json",
TEST_PREFIX + "student3");
}
catch (Exception e) {
e.printStackTrace();
catch (IOException e) {
fail(e.getMessage(), e);
}
}
else if (exercise instanceof TextExercise) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void initRepo(String defaultBranch) {
remoteGit.close();
}
catch (IOException | GitAPIException ex) {
ex.printStackTrace();
fail(ex.getMessage(), ex);
}
}

Expand Down

0 comments on commit c3e3662

Please sign in to comment.