Skip to content

fix(online): thread-safe fileMap with TTL cleanup, SLF4J logging, and Content-Length header #24011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vikas0686 wants to merge 6 commits into
base: master
Choose a base branch
from
Open

fix(online): thread-safe fileMap with TTL cleanup, SLF4J logging, and Content-Length header #24011

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
92d6c23
fix : thread safety, logging, and Content-Length in openapi-generator…
vikas0686 Jun 12, 2026
f418c78
fix : thread-safe fileMap with 24h TTL cleanup in openapi-generator-o…
vikas0686 Jun 12, 2026
3aaaa56
fix(online): enforce TTL at download time and make createdAt immutable
vikas0686 Jun 12, 2026
67631d7
fix : updated the failing test cases for GenApiControllerTest
vikas0686 Jun 12, 2026
5720305
fix: Add GenApiServiceTest with TTL cleanup and concurrent generation…
vikas0686 Jun 12, 2026
709a677
fix(online): retain fileMap entry when temp directory deletion fails
vikas0686 Jun 13, 2026
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
3 changes: 3 additions & 0 deletions ...enapi-generator-online/src/main/java/org/openapitools/codegen/online/model/Generated.java
View file
Open in desktop
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.
}
46 changes: 39 additions & 7 deletions ...generator-online/src/main/java/org/openapitools/codegen/online/service/GenApiService.java
View file
Open in desktop
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) {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
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,31 @@ 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();
}
}

@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;
});
}

}
248 changes: 246 additions & 2 deletions ...erator-online/src/test/java/org/openapitools/codegen/online/api/GenApiControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.openapitools.codegen.online.model.Generated;
import org.openapitools.codegen.online.model.ResponseCode;
import org.openapitools.codegen.online.service.GenApiService;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.http.HttpHeaders;
Expand All @@ -12,9 +14,19 @@
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.util.Assert;

import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.not;
import java.lang.reflect.Field;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

import static org.hamcrest.Matchers.*;
import static org.hamcrest.text.MatchesPattern.matchesPattern;
import static org.junit.jupiter.api.Assertions.*;
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 +41,238 @@ public class GenApiControllerTest {
@Autowired
private MockMvc mockMvc;

@Autowired
private GenApiService genApiService;

@Test
public void clientLanguages() throws Exception {
getLanguages("clients", "java");
}

@Test
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())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("$.[*]").value(hasItem(expected)));
}

@Test
public void clientOptions() throws Exception {
getOptions("clients", "java");
}

@Test
public void clientOptionsUnknown() throws Exception {
mockMvc.perform(get("/api/gen/clients/unknown"))
.andExpect(status().isNotFound());
}

@Test
public void serverOptions() throws Exception {
getOptions("servers", "spring");
}

@Test
public void serverOptionsUnknown() throws Exception {
mockMvc.perform(get("/api/gen/servers/unknown"))
.andExpect(status().isNotFound());
}

private void getOptions(String type, String name) throws Exception {
mockMvc.perform(get("/api/gen/" + type + "/" + name))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("$.sortParamsByRequiredFlag.opt").value("sortParamsByRequiredFlag"));
}

@Test
public void generateClient() throws Exception {
generateAndDownload("clients", "java");
}

@Test
public void generateServer() throws Exception {
generateAndDownload("servers", "spring");
}

private void generateAndDownload(String type, String name) throws Exception {
String result = mockMvc.perform(post("http://test.com:1234/api/gen/" + type + "/" + name)
.contentType(MediaType.APPLICATION_JSON)
.content("{\"openAPIUrl\": \"" + OPENAPI_URL + "\"}"))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("$.code").value(matchesPattern(UUID_REGEX)))
.andExpect(jsonPath("$.link").value(matchesPattern("http\\:\\/\\/test.com\\:1234\\/api\\/gen\\/download\\/" + UUID_REGEX)))
.andReturn().getResponse().getContentAsString();

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

mockMvc.perform(get("http://test.com:1234/api/gen/download/" + code))
.andExpect(content().contentType("application/zip"))
.andExpect(status().isOk())
.andExpect(header().string(HttpHeaders.CONTENT_LENGTH, not(0)));
}

@Test
public void generateWIthForwardedHeaders() throws Exception {
String result = mockMvc.perform(post("http://test.com:1234/api/gen/clients/java")
.contentType(MediaType.APPLICATION_JSON)
.header("X-Forwarded-Proto", "https")
.header("X-Forwarded-Host", "forwarded.com")
.header("X-Forwarded-Port", "5678")
.content("{\"openAPIUrl\": \"" + OPENAPI_URL + "\"}"))
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("$.code").value(matchesPattern(UUID_REGEX)))
.andExpect(jsonPath("$.link").value(matchesPattern("https\\:\\/\\/forwarded.com\\:5678\\/api\\/gen\\/download\\/" + UUID_REGEX)))
.andReturn().getResponse().getContentAsString();

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

mockMvc.perform(get("http://test.com:1234/api/gen/download/" + code))
.andExpect(content().contentType("application/zip"))
.andExpect(status().isOk())
.andExpect(header().string(HttpHeaders.CONTENT_LENGTH, not(0)));
}

@Test
public void generateClientWithInvalidOpenAPIUrl() throws Exception {
final String invalidOpenAPIUrl = "https://[::1]/invalid_openapi.json";
mockMvc.perform(post("http://test.com:1234/api/gen/clients/java")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"openAPIUrl\": \"" + invalidOpenAPIUrl + "\"}"))
.andExpect(status().isBadRequest());
}

@Test
public void generateWithOpenAPINormalizer() throws Exception {
String withOpenAPINormalizer = "{\"openAPIUrl\":\"https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml\",\"openapiNormalizer\":[\"FILTER=operationId:updatePet\"],\"options\":{},\"spec\":{}}";
String withoutOpenAPINormalizer = "{\"openAPIUrl\":\"https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml\",\"options\":{},\"spec\":{}}";

String responseOfNormalized = mockMvc.perform(post("http://test.com:1234/api/gen/clients/java")
.contentType(MediaType.APPLICATION_JSON)
.content(withOpenAPINormalizer))
.andExpect(status().isOk()).andReturn().getResponse().getContentAsString();
String codeOfNormalized = new ObjectMapper().readValue(responseOfNormalized, ResponseCode.class).getCode();
Long lengthOfNormalized = Long.parseLong(mockMvc.perform(get("http://test.com:1234/api/gen/download/" + codeOfNormalized))
.andExpect(content().contentType("application/zip"))
.andExpect(status().isOk()).andReturn().getResponse().getHeader("Content-Length"));

String responseOfNotNormalized = mockMvc.perform(post("http://test.com:1234/api/gen/clients/java")
.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: downloading an expired/evicted fileId returns 404, not NPE
@Test
public void downloadExpiredFileReturns404() throws Exception {
mockMvc.perform(get("http://test.com:1234/api/gen/download/nonexistent-or-expired-id"))
.andExpect(status().isNotFound());
}

// Fix #1: fileMap uses ConcurrentHashMap (thread-safe)
@Test
public void fileMapIsConcurrentHashMap() throws Exception {
Field field = GenApiService.class.getDeclaredField("fileMap");
field.setAccessible(true);
Object map = field.get(null);
assertInstanceOf(ConcurrentHashMap.class, map, "fileMap should be a ConcurrentHashMap");
}

// Fix #1: TTL cleanup removes expired entries
@Test
public void cleanExpiredFilesRemovesOldEntries() throws Exception {
Field field = GenApiService.class.getDeclaredField("fileMap");
field.setAccessible(true);
@SuppressWarnings("unchecked")
Map<String, Generated> fileMap = (Map<String, Generated>) field.get(null);

// Insert an entry with a creation time well in the past
Generated expired = new Generated();
expired.setFilename("/tmp/nonexistent-expired.zip");
expired.setFriendlyName("test");
expired.setCreatedAt(Instant.now().minusSeconds(25 * 3600)); // 25 hours ago, beyond 24h TTL
fileMap.put("expired-test-key", expired);

genApiService.cleanExpiredFiles();

assertFalse(fileMap.containsKey("expired-test-key"), "Expired entry should have been removed by cleanExpiredFiles");
}

// Fix #1: concurrent generation does not lose entries (thread-safety smoke test)
@Test
public void concurrentGenerationDoesNotLoseEntries() throws Exception {
int threads = 5;
CountDownLatch latch = new CountDownLatch(threads);
ExecutorService executor = Executors.newFixedThreadPool(threads);
List<String> codes = new ArrayList<>();

for (int i = 0; i < threads; i++) {
executor.submit(() -> {
try {
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();
synchronized (codes) {
codes.add(new ObjectMapper().readValue(result, ResponseCode.class).getCode());
}
} catch (Exception e) {
fail("Concurrent generation failed: " + e.getMessage());
} finally {
latch.countDown();
}
});
}

latch.await();
executor.shutdown();
assertEquals(threads, codes.size(), "All concurrent generations should produce distinct download codes");
assertEquals(threads, codes.stream().distinct().count(), "All codes should be unique");
}
}

private static final String OPENAPI_URL = "https://raw.githubusercontent.com/OpenAPITools/openapi-generator/v4.3.1/modules/openapi-generator/src/test/resources/petstore.json";
private static final String UUID_REGEX = "[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[89aAbB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}";

@Autowired
private MockMvc mockMvc;

@Test
public void clientLanguages() throws Exception {
getLanguages("clients", "java");
Expand Down