Merge branch 'develop' into development/fix-PostIntegrationTest

ruleset.xml

@@ -131,7 +131,10 @@ http://pmd.sourceforge.net/ruleset/2.0.0 ">
     <rule ref="category/java/errorprone.xml/ProperLogger"/>
     <rule ref="category/java/errorprone.xml/ReturnFromFinallyBlock"/>
     <rule ref="category/java/codestyle.xml/ShortMethodName"/>
-    <rule ref="category/java/codestyle.xml/ShortVariable"/>
+    <rule ref="category/java/codestyle.xml/ShortVariable">
+        // It's okay to use short variable names in DTOs, e.g. "id" or "name"
+        <exclude-pattern>.*/de/tum/in/www1/artemis/web/rest/dto/.*</exclude-pattern>
+    </rule>
     <rule ref="category/java/design.xml/SimplifiedTernary"/>
     <rule ref="category/java/design.xml/SimplifyBooleanAssertion"/>
     <rule ref="category/java/design.xml/SimplifyBooleanExpressions"/>

src/main/java/de/tum/in/www1/artemis/web/rest/CourseResource.java

@@ -21,10 +21,7 @@
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageImpl;
 import org.springframework.data.domain.PageRequest;
-import org.springframework.http.HttpHeaders;
-import org.springframework.http.HttpStatus;
-import org.springframework.http.MediaType;
-import org.springframework.http.ResponseEntity;
+import org.springframework.http.*;
 import org.springframework.web.bind.annotation.*;
 import org.springframework.web.multipart.MultipartFile;
 import org.springframework.web.server.ResponseStatusException;
@@ -761,7 +758,11 @@ public ResponseEntity<Resource> downloadCourseArchive(@PathVariable Long courseI
 
         File zipFile = archive.toFile();
         InputStreamResource resource = new InputStreamResource(new FileInputStream(zipFile));
-        return ResponseEntity.ok().contentLength(zipFile.length()).contentType(MediaType.APPLICATION_OCTET_STREAM).header("filename", zipFile.getName()).body(resource);
+        ContentDisposition contentDisposition = ContentDisposition.builder("attachment").filename(zipFile.getName()).build();
+        HttpHeaders headers = new HttpHeaders();
+        headers.setContentDisposition(contentDisposition);
+        return ResponseEntity.ok().headers(headers).contentLength(zipFile.length()).contentType(MediaType.APPLICATION_OCTET_STREAM).header("filename", zipFile.getName())
+                .body(resource);
     }
 
     /**

src/main/java/de/tum/in/www1/artemis/web/rest/ExamResource.java

@@ -24,10 +24,7 @@
 import org.springframework.core.io.Resource;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
-import org.springframework.http.HttpHeaders;
-import org.springframework.http.HttpStatus;
-import org.springframework.http.MediaType;
-import org.springframework.http.ResponseEntity;
+import org.springframework.http.*;
 import org.springframework.web.bind.annotation.*;
 import org.springframework.web.servlet.support.ServletUriComponentsBuilder;
 
@@ -1150,8 +1147,12 @@ public ResponseEntity<Resource> downloadExamArchive(@PathVariable Long courseId,
         Path archive = Path.of(examArchivesDirPath, exam.getExamArchivePath());
 
         File zipFile = archive.toFile();
+        ContentDisposition contentDisposition = ContentDisposition.builder("attachment").filename(zipFile.getName()).build();
+        HttpHeaders headers = new HttpHeaders();
+        headers.setContentDisposition(contentDisposition);
         InputStreamResource resource = new InputStreamResource(new FileInputStream(zipFile));
-        return ResponseEntity.ok().contentLength(zipFile.length()).contentType(MediaType.APPLICATION_OCTET_STREAM).header("filename", zipFile.getName()).body(resource);
+        return ResponseEntity.ok().headers(headers).contentLength(zipFile.length()).contentType(MediaType.APPLICATION_OCTET_STREAM).header("filename", zipFile.getName())
+                .body(resource);
     }
 
     /**

src/main/webapp/app/course/manage/course-management.service.ts

@@ -417,11 +417,9 @@ export class CourseManagementService {
      * if the archive does not exist.
      * @param courseId The id of the course
      */
-    downloadCourseArchive(courseId: number): Observable<HttpResponse<Blob>> {
-        return this.http.get(`${this.resourceUrl}/${courseId}/download-archive`, {
-            observe: 'response',
-            responseType: 'blob',
-        });
+    downloadCourseArchive(courseId: number): void {
+        const url = `${this.resourceUrl}/${courseId}/download-archive`;
+        window.open(url, '_blank');
     }
 
     /**

src/main/webapp/app/exam/manage/exam-management.service.ts

@@ -468,11 +468,9 @@ export class ExamManagementService {
      * @param courseId
      * @param examId The id of the exam
      */
-    downloadExamArchive(courseId: number, examId: number): Observable<HttpResponse<Blob>> {
-        return this.http.get(`${this.resourceUrl}/${courseId}/exams/${examId}/download-archive`, {
-            observe: 'response',
-            responseType: 'blob',
-        });
+    downloadExamArchive(courseId: number, examId: number): void {
+        const url = `${this.resourceUrl}/${courseId}/exams/${examId}/download-archive`;
+        window.open(url, '_blank');
     }
 
     /**

src/main/webapp/app/shared/components/course-exam-archive-button/course-exam-archive-button.component.ts

@@ -1,4 +1,4 @@
-import { ChangeDetectorRef, Component, Input, OnDestroy, OnInit, TemplateRef, ViewChild } from '@angular/core';
+import { Component, Input, OnDestroy, OnInit, TemplateRef, ViewChild } from '@angular/core';
 import { AlertService } from 'app/core/util/alert.service';
 import { CourseManagementService } from 'app/course/manage/course-management.service';
 import { JhiWebsocketService } from 'app/core/websocket/websocket.service';
@@ -9,7 +9,6 @@ import { ExamManagementService } from 'app/exam/manage/exam-management.service';
 import { Course } from 'app/entities/course.model';
 import { Exam } from 'app/entities/exam.model';
 import dayjs from 'dayjs/esm';
-import { downloadZipFileFromResponse } from 'app/shared/util/download.util';
 import { ButtonSize } from '../button.component';
 import { ActionType } from 'app/shared/delete-dialog/delete-dialog.model';
 import { Subject } from 'rxjs';
@@ -70,7 +69,6 @@ export class CourseExamArchiveButtonComponent implements OnInit, OnDestroy {
         private translateService: TranslateService,
         private modalService: NgbModal,
         private accountService: AccountService,
-        private changeDetectionRef: ChangeDetectorRef,
     ) {}
 
     ngOnInit() {
@@ -127,13 +125,11 @@ export class CourseExamArchiveButtonComponent implements OnInit, OnDestroy {
         if (this.archiveMode === 'Exam' && this.exam) {
             this.examService.find(this.course.id!, this.exam.id!).subscribe((res) => {
                 this.exam = res.body!;
-                this.changeDetectionRef.detectChanges();
                 this.displayDownloadArchiveButton = this.canDownloadArchive();
             });
         } else {
             this.courseService.find(this.course.id!).subscribe((res) => {
                 this.course = res.body!;
-                this.changeDetectionRef.detectChanges();
                 this.displayDownloadArchiveButton = this.canDownloadArchive();
             });
         }
@@ -181,8 +177,6 @@ export class CourseExamArchiveButtonComponent implements OnInit, OnDestroy {
                 }
                 if (result === 'archive' || !this.canDownloadArchive()) {
                     this.archive();
-                } else {
-                    this.reloadCourseOrExam();
                 }
             },
             () => {},
@@ -211,15 +205,9 @@ export class CourseExamArchiveButtonComponent implements OnInit, OnDestroy {
 
     downloadArchive() {
         if (this.archiveMode === 'Exam' && this.exam) {
-            this.examService.downloadExamArchive(this.course.id!, this.exam.id!).subscribe({
-                next: (response) => downloadZipFileFromResponse(response),
-                error: () => this.alertService.error('artemisApp.courseExamArchive.archiveDownloadError'),
-            });
+            this.examService.downloadExamArchive(this.course.id!, this.exam.id!);
         } else {
-            this.courseService.downloadCourseArchive(this.course.id!).subscribe({
-                next: (response) => downloadZipFileFromResponse(response),
-                error: () => this.alertService.error('artemisApp.courseExamArchive.archiveDownloadError'),
-            });
+            this.courseService.downloadCourseArchive(this.course.id!);
         }
     }
 

src/test/java/de/tum/in/www1/artemis/course/CourseTestService.java

@@ -2176,9 +2176,11 @@ public void testDownloadCourseArchiveAsInstructor_not_found() throws Exception {
     public void testDownloadCourseArchiveAsInstructor() throws Exception {
         // Archive the course and wait until it's complete
         Course updatedCourse = testArchiveCourseWithTestModelingAndFileUploadExercises();
+        Map<String, String> expectedHeaders = new HashMap<>();
+        expectedHeaders.put("Content-Disposition", "attachment; filename=\"" + updatedCourse.getCourseArchivePath() + "\"");
 
         // Download the archive
-        var archive = request.getFile("/api/courses/" + updatedCourse.getId() + "/download-archive", HttpStatus.OK, new LinkedMultiValueMap<>());
+        var archive = request.getFile("/api/courses/" + updatedCourse.getId() + "/download-archive", HttpStatus.OK, new LinkedMultiValueMap<>(), expectedHeaders);
         assertThat(archive).isNotNull();
         assertThat(archive).exists();
         assertThat(archive.getPath().length()).isGreaterThanOrEqualTo(4);

src/test/java/de/tum/in/www1/artemis/exam/ExamIntegrationTest.java

@@ -1069,7 +1069,10 @@ void testDownloadExamArchiveAsInstructor() throws Exception {
 
         // Download the archive
         var exam = examRepository.findByCourseId(course.getId()).stream().findFirst().orElseThrow();
-        var archive = request.getFile("/api/courses/" + course.getId() + "/exams/" + exam.getId() + "/download-archive", HttpStatus.OK, new LinkedMultiValueMap<>());
+        Map<String, String> expectedHeaders = new HashMap<>();
+        expectedHeaders.put("Content-Disposition", "attachment; filename=\"" + exam.getExamArchivePath() + "\"");
+        var archive = request.getFile("/api/courses/" + course.getId() + "/exams/" + exam.getId() + "/download-archive", HttpStatus.OK, new LinkedMultiValueMap<>(),
+                expectedHeaders);
         assertThat(archive).isNotNull();
 
         // Extract the archive

src/test/java/de/tum/in/www1/artemis/util/RequestUtilService.java

@@ -69,9 +69,9 @@ public ObjectMapper getObjectMapper() {
      * @param file           the optional file to be sent
      * @param responseType   the expected response type as class
      * @param expectedStatus the expected status
+     * @param <T>            the type of the main object to send
+     * @param <R>            the type of the response object
      * @return the response as object of the given type or null if the status is not 2xx
-     * @param <T> the type of the main object to send
-     * @param <R> the type of the response object
      * @throws Exception if the request fails
      */
     public <T, R> R postWithMultipartFile(String path, T paramValue, String paramName, MockMultipartFile file, Class<R> responseType, HttpStatus expectedStatus) throws Exception {
@@ -87,9 +87,9 @@ public <T, R> R postWithMultipartFile(String path, T paramValue, String paramNam
      * @param files          the optional files to be sent
      * @param responseType   the expected response type as class
      * @param expectedStatus the expected status
+     * @param <T>            the type of the main object to send
+     * @param <R>            the type of the response object
      * @return the response as object of the given type or null if the status is not 2xx
-     * @param <T> the type of the main object to send
-     * @param <R> the type of the response object
      * @throws Exception if the request fails
      */
     public <T, R> R postWithMultipartFiles(String path, T paramValue, String paramName, List<MockMultipartFile> files, Class<R> responseType, HttpStatus expectedStatus)
@@ -380,9 +380,9 @@ public <T, R> R postWithResponseBody(String path, T body, Class<R> responseType)
      * @param responseType   the expected response type as class
      * @param expectedStatus the expected status
      * @param params         the optional parameters for the request
+     * @param <T>            the type of the main object to send
+     * @param <R>            the type of the response object
      * @return the response as object of the given type or null if the status is not 2xx
-     * @param <T> the type of the main object to send
-     * @param <R> the type of the response object
      * @throws Exception if the request fails
      */
     public <T, R> R putWithMultipartFile(String path, T paramValue, String paramName, MockMultipartFile file, Class<R> responseType, HttpStatus expectedStatus,
@@ -400,9 +400,9 @@ public <T, R> R putWithMultipartFile(String path, T paramValue, String paramName
      * @param responseType   the expected response type as class
      * @param expectedStatus the expected status
      * @param params         the optional parameters for the request
+     * @param <T>            the type of the main object to send
+     * @param <R>            the type of the response object
      * @return the response as object of the given type or null if the status is not 2xx
-     * @param <T> the type of the main object to send
-     * @param <R> the type of the response object
      * @throws Exception if the request fails
      */
     public <T, R> R putWithMultipartFiles(String path, T paramValue, String paramName, List<MockMultipartFile> files, Class<R> responseType, HttpStatus expectedStatus,
@@ -558,12 +558,17 @@ public <T> T get(String path, HttpStatus expectedStatus, Class<T> responseType,
     }
 
     public File getFile(String path, HttpStatus expectedStatus, MultiValueMap<String, String> params) throws Exception {
+        return getFile(path, expectedStatus, params, null);
+    }
+
+    public File getFile(String path, HttpStatus expectedStatus, MultiValueMap<String, String> params, @Nullable Map<String, String> expectedResponseHeaders) throws Exception {
         MvcResult res = mvc.perform(MockMvcRequestBuilders.get(new URI(path)).params(params).headers(new HttpHeaders())).andExpect(status().is(expectedStatus.value())).andReturn();
         restoreSecurityContext();
         if (!expectedStatus.is2xxSuccessful()) {
             assertThat(res.getResponse().containsHeader("location")).as("no location header on failed request").isFalse();
             return null;
         }
+        verifyExpectedResponseHeaders(expectedResponseHeaders, res);
 
         String tmpDirectory = System.getProperty("java.io.tmpdir");
         var filename = res.getResponse().getHeader("filename");

src/test/javascript/spec/component/course/course-management.service.spec.ts

@@ -426,15 +426,11 @@ describe('Course Management Service', () => {
         tick();
     }));
 
-    it('should download course archive', fakeAsync(() => {
-        const expectedBlob = new Blob(['abc', 'cfe']);
-        courseManagementService.downloadCourseArchive(course.id!).subscribe((resp) => {
-            expect(resp.body).toEqual(expectedBlob);
-        });
-        const req = httpMock.expectOne({ method: 'GET', url: `${resourceUrl}/${course.id}/download-archive` });
-        req.flush(expectedBlob);
-        tick();
-    }));
+    it('should download course archive', () => {
+        const windowSpy = jest.spyOn(window, 'open').mockImplementation();
+        courseManagementService.downloadCourseArchive(1);
+        expect(windowSpy).toHaveBeenCalledWith('api/courses/1/download-archive', '_blank');
+    });
 
     it('should archive the course', fakeAsync(() => {
         courseManagementService.archiveCourse(course.id!).subscribe((res) => expect(res.body).toEqual(course));

src/test/javascript/spec/component/exam/manage/exam-management.service.spec.ts

@@ -641,21 +641,11 @@ describe('Exam Management Service Tests', () => {
     }));
 
     it('should download the exam from archive', fakeAsync(() => {
-        // GIVEN
         const mockExam: Exam = { id: 1 };
-        const mockResponse = new Blob();
-        const expected = new Blob();
-
-        // WHEN
-        service.downloadExamArchive(course.id!, mockExam.id!).subscribe((res) => expect(res.body).toEqual(expected));
 
-        // THEN
-        const req = httpMock.expectOne({
-            method: 'GET',
-            url: `${service.resourceUrl}/${course.id}/exams/${mockExam.id}/download-archive`,
-        });
-        req.flush(mockResponse);
-        tick();
+        const windowSpy = jest.spyOn(window, 'open').mockImplementation();
+        service.downloadExamArchive(course.id!, mockExam.id!);
+        expect(windowSpy).toHaveBeenCalledWith('api/courses/456/exams/1/download-archive', '_blank');
     }));
 
     it('should archive the exam', fakeAsync(() => {

src/test/javascript/spec/component/shared/course-exam-archive-button.component.spec.ts

@@ -170,8 +170,7 @@ describe('Course Exam Archive Button Component', () => {
         }));
 
         it('should download archive for course', fakeAsync(() => {
-            const response: HttpResponse<Blob> = new HttpResponse({ status: 200 });
-            const downloadStub = jest.spyOn(courseManagementService, 'downloadCourseArchive').mockReturnValue(of(response));
+            const downloadStub = jest.spyOn(courseManagementService, 'downloadCourseArchive').mockImplementation(() => {});
 
             comp.downloadArchive();
 
@@ -253,14 +252,13 @@ describe('Course Exam Archive Button Component', () => {
             expect(comp.canCleanupCourse()).toBeFalse();
         }));
 
-        it('should download archive for exam', fakeAsync(() => {
-            const response: HttpResponse<Blob> = new HttpResponse({ status: 200 });
-            const downloadStub = jest.spyOn(examManagementService, 'downloadExamArchive').mockReturnValue(of(response));
+        it('should download archive for exam', () => {
+            const downloadStub = jest.spyOn(examManagementService, 'downloadExamArchive').mockImplementation(() => {});
 
             comp.downloadArchive();
 
             expect(downloadStub).toHaveBeenCalledOnce();
-        }));
+        });
 
         it('should archive course', fakeAsync(() => {
             const response: HttpResponse<void> = new HttpResponse({ status: 200 });