Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@
import lombok.Getter;
import lombok.Setter;

import java.time.Instant;

@Getter
@Setter
public class Generated {
private String filename;
private String friendlyName;
private Instant createdAt = Instant.now();
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,16 @@
import org.openapitools.codegen.online.model.Generated;
import org.openapitools.codegen.online.model.GeneratorInput;
import org.openapitools.codegen.online.model.ResponseCode;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.core.io.ByteArrayResource;
import org.springframework.core.io.Resource;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.scheduling.annotation.EnableScheduling;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Service;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.server.ResponseStatusException;
Expand All @@ -45,14 +49,20 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Instant;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;

@Service
@EnableScheduling
public class GenApiService implements GenApiDelegate {

private static final Logger LOGGER = LoggerFactory.getLogger(GenApiService.class);
private static final long FILE_TTL_MS = 24 * 60 * 60 * 1000L; // 24 hours

private static List<String> clients = new ArrayList<>();
private static List<String> servers = new ArrayList<>();
private static Map<String, Generated> fileMap = new HashMap<>();
private static final Map<String, Generated> fileMap = new ConcurrentHashMap<>();

static {
List<CodegenConfig> extensions = CodegenConfigLoader.getAll();
Expand Down Expand Up @@ -80,8 +90,11 @@ public Optional<NativeWebRequest> getRequest() {
@Override
public ResponseEntity<Resource> downloadFile(String fileId) {
Generated g = fileMap.get(fileId);
System.out.println("looking for fileId " + fileId);
System.out.println("got filename " + g.getFilename());
LOGGER.debug("looking for fileId {}", fileId);
if (g == null || g.getCreatedAt().plusMillis(FILE_TTL_MS).isBefore(Instant.now())) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND, "File not found or has expired");
}
LOGGER.debug("got filename {}", g.getFilename());

File file = new File(g.getFilename());
Path path = Paths.get(file.getAbsolutePath());
Expand All @@ -96,15 +109,15 @@ public ResponseEntity<Resource> downloadFile(String fileId) {
try {
FileUtils.deleteDirectory(file.getParentFile());
} catch (IOException e) {
System.out.println("failed to delete file " + file.getAbsolutePath());
LOGGER.error("failed to delete file {}", file.getAbsolutePath());
}
return ResponseEntity
.ok()
.contentType(MediaType.valueOf("application/zip"))
.contentLength(resource.contentLength())
.header("Content-Disposition",
"attachment; filename=\"" + g.getFriendlyName() + "-generated.zip\"")
.header("Accept-Range", "bytes")
//.header("Content-Length", bytes.length)
.body(resource);
}

Expand Down Expand Up @@ -152,7 +165,7 @@ public ResponseEntity<ResponseCode> generateServerForLanguage(String framework,
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Framework is required");
}
String filename = Generator.generateServer(framework, generatorInput);
System.out.println("generated name: " + filename);
LOGGER.debug("generated name: {}", filename);

return getResponse(filename, framework + "-server");
}
Expand All @@ -173,12 +186,46 @@ private ResponseEntity<ResponseCode> getResponse(String filename, String friendl
g.setFilename(filename);
g.setFriendlyName(friendlyName);
fileMap.put(code, g);
System.out.println(code + ", " + filename);
LOGGER.debug("{}, {}", code, filename);
String link = uriBuilder.path("/api/gen/download/").path(code).toUriString();
return ResponseEntity.ok().body(new ResponseCode(code, link));
} else {
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).build();
}
}

/** @VisibleForTesting */
Generated getFileEntry(String code) {
return fileMap.get(code);
}

/** @VisibleForTesting */
void putFileEntry(String code, Generated entry) {
fileMap.put(code, entry);
}

/** @VisibleForTesting */
void removeFileEntry(String code) {
fileMap.remove(code);
}

@Scheduled(fixedDelay = 3_600_000) // run every hour
public void cleanExpiredFiles() {
Instant cutoff = Instant.now().minusMillis(FILE_TTL_MS);
fileMap.entrySet().removeIf(entry -> {
Generated g = entry.getValue();
if (g.getCreatedAt().isBefore(cutoff)) {
File file = new File(g.getFilename());
try {
FileUtils.deleteDirectory(file.getParentFile());
} catch (IOException e) {
LOGGER.warn("failed to delete expired file {}", g.getFilename());
}
LOGGER.debug("evicted expired file entry {}", entry.getKey());
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
return true;
}
return false;
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.hamcrest.Matchers.hasItem;
Comment thread
vikas0686 marked this conversation as resolved.
import static org.hamcrest.Matchers.not;
import static org.hamcrest.text.MatchesPattern.matchesPattern;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
Expand All @@ -29,6 +30,7 @@ public class GenApiControllerTest {
@Autowired
private MockMvc mockMvc;


@Test
public void clientLanguages() throws Exception {
getLanguages("clients", "java");
Expand All @@ -39,7 +41,6 @@ public void serverFrameworks() throws Exception {
getLanguages("servers", "spring");
}


public void getLanguages(String type, String expected) throws Exception {
mockMvc.perform(get("/api/gen/" + type))
.andExpect(status().isOk())
Expand Down Expand Up @@ -153,13 +154,37 @@ public void generateWithOpenAPINormalizer() throws Exception {
.contentType(MediaType.APPLICATION_JSON)
.content(withoutOpenAPINormalizer))
.andExpect(status().isOk()).andReturn().getResponse().getContentAsString();

String codeOfNotNormalized = new ObjectMapper().readValue(responseOfNotNormalized, ResponseCode.class).getCode();
Long lengthOfNotNormalized = Long.parseLong(mockMvc.perform(get("http://test.com:1234/api/gen/download/" + codeOfNotNormalized))
.andExpect(content().contentType("application/zip"))
.andExpect(status().isOk()).andReturn().getResponse().getHeader("Content-Length"));

Assert.isTrue(lengthOfNormalized <= lengthOfNotNormalized, "Using the normalizer should result in a smaller or equal file size");
}

// Fix #3: Content-Length header is present and non-zero on download response
@Test
public void downloadHasContentLengthHeader() throws Exception {
String result = mockMvc.perform(post("http://test.com:1234/api/gen/clients/java")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"openAPIUrl\": \"" + OPENAPI_URL + "\"}"))
.andExpect(status().isOk())
.andReturn().getResponse().getContentAsString();

String code = new ObjectMapper().readValue(result, ResponseCode.class).getCode();

String contentLength = mockMvc.perform(get("http://test.com:1234/api/gen/download/" + code))
.andExpect(status().isOk())
.andExpect(header().exists(HttpHeaders.CONTENT_LENGTH))
.andReturn().getResponse().getHeader(HttpHeaders.CONTENT_LENGTH);

assertTrue(Long.parseLong(contentLength) > 0, "Content-Length should be greater than 0");
}

// Fix #1: missing fileId returns 404
@Test
public void downloadMissingFileReturns404() throws Exception {
mockMvc.perform(get("http://test.com:1234/api/gen/download/nonexistent-id"))
.andExpect(status().isNotFound());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package org.openapitools.codegen.online.service;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P3: Cleanup is not guaranteed if the assertion fails, so this test can leak state into the shared GenApiService and leave temporary files behind.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator-online/src/test/java/org/openapitools/codegen/online/service/GenApiServiceTest.java, line 72:

<comment>Cleanup is not guaranteed if the assertion fails, so this test can leak state into the shared GenApiService and leave temporary files behind.</comment>

<file context>
@@ -0,0 +1,151 @@
+
+        genApiService.cleanExpiredFiles();
+
+        assertNotNull(genApiService.getFileEntry("test-deletion-fail-key"), "Entry should be retained when deletion fails");
+
+        // cleanup
</file context>


import org.junit.jupiter.api.Test;
import org.openapitools.codegen.online.model.Generated;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;

import java.time.Instant;

import static org.junit.jupiter.api.Assertions.*;

@SpringBootTest
public class GenApiServiceTest {

@Autowired
private GenApiService genApiService;

// Fix #1: TTL cleanup removes expired entries
@Test
public void cleanExpiredFilesRemovesExpiredEntry() {
Generated entry = new Generated();
entry.setFilename("/tmp/codegen-test/bundle.zip");
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
entry.setFriendlyName("test");
entry.setCreatedAt(Instant.now().minusSeconds(25 * 3600)); // 25h ago, beyond 24h TTL
genApiService.putFileEntry("test-expired-key", entry);

genApiService.cleanExpiredFiles();

assertNull(genApiService.getFileEntry("test-expired-key"), "Expired entry should have been evicted");
}

// Fix #1: recent entry is not evicted by cleanup
@Test
public void cleanExpiredFilesKeepsRecentEntry() {
Generated entry = new Generated();
entry.setFilename("/tmp/codegen-test/bundle.zip");
entry.setFriendlyName("test");
// createdAt defaults to Instant.now() — well within 24h TTL
genApiService.putFileEntry("test-recent-key", entry);

genApiService.cleanExpiredFiles();

assertNotNull(genApiService.getFileEntry("test-recent-key"), "Recent entry should not be evicted");
genApiService.removeFileEntry("test-recent-key");
}
}