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

modules/openapi-generator-online/src/main/java/org/openapitools/codegen/online/model/Generated.java

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

modules/openapi-generator-online/src/main/java/org/openapitools/codegen/online/service/GenApiService.java

@@ -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;
@@ -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();
@@ -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());
@@ -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);
     }
 
@@ -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");
     }
@@ -173,12 +186,49 @@ 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 dir = new File(g.getFilename()).getParentFile();
+                if (dir.exists()) {
+                    try {
+                        FileUtils.deleteDirectory(dir);
+                    } catch (IOException | IllegalArgumentException e) {
+                        LOGGER.warn("failed to delete expired file {}, will retry on next run", g.getFilename());
+                        return false;
+                    }
+                }
+                LOGGER.debug("evicted expired file entry {}", entry.getKey());
+                return true;
+            }
+            return false;
+        });
+    }
+
 }

modules/openapi-generator-online/src/test/java/org/openapitools/codegen/online/api/GenApiControllerTest.java

@@ -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;
@@ -12,9 +14,17 @@
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.util.Assert;
 
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
 import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.not;
 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.*;
@@ -29,6 +39,7 @@ public class GenApiControllerTest {
     @Autowired
     private MockMvc mockMvc;
 
+
     @Test
     public void clientLanguages() throws Exception {
         getLanguages("clients", "java");
@@ -39,7 +50,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())
@@ -153,13 +163,31 @@ 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");
     }
+
 }

modules/openapi-generator-online/src/test/java/org/openapitools/codegen/online/service/GenApiServiceTest.java

@@ -0,0 +1,162 @@
+package org.openapitools.codegen.online.service;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.jupiter.api.Test;
+import org.openapitools.codegen.online.model.Generated;
+import org.openapitools.codegen.online.model.ResponseCode;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.http.MediaType;
+import org.springframework.test.web.servlet.MockMvc;
+
+import java.io.File;
+import java.net.URL;
+import java.nio.file.Files;
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+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.status;
+
+@SpringBootTest
+@AutoConfigureMockMvc
+public class GenApiServiceTest {
+
+    private static final String OPENAPI_URL = localPetstoreUrl();
+
+    private static String localPetstoreUrl() {
+        URL resource = GenApiServiceTest.class.getClassLoader().getResource("petstore.json");
+        if (resource == null) {
+            throw new IllegalStateException("petstore.json not found in test resources");
+        }
+        return resource.toExternalForm();
+    }
+
+    @Autowired
+    private MockMvc mockMvc;
+
+    @Autowired
+    private GenApiService genApiService;
+
+    // Fix #1: TTL cleanup removes expired entries and deletes the temp directory
+    @Test
+    public void cleanExpiredFilesRemovesExpiredEntry() throws Exception {
+        File tempDir = Files.createTempDirectory("codegen-test").toFile();
+        File bundle = new File(tempDir, "bundle.zip");
+        bundle.createNewFile();
+
+        Generated entry = new Generated();
+        entry.setFilename(bundle.getAbsolutePath());
+        entry.setFriendlyName("test");
+        entry.setCreatedAt(Instant.now().minusSeconds(25 * 3600));
+        genApiService.putFileEntry("test-expired-key", entry);
+
+        genApiService.cleanExpiredFiles();
+
+        assertNull(genApiService.getFileEntry("test-expired-key"), "Expired entry should have been evicted");
+        assertFalse(tempDir.exists(), "Temp directory should have been deleted");
+    }
+
+    // Fix: entry is retained if directory deletion fails (no orphaned files)
+    @Test
+    public void cleanExpiredFilesRetainsEntryWhenDeletionFails() throws Exception {
+        // Point filename at a path whose "parent" is a regular file — deleteDirectory will fail
+        File fakeParent = Files.createTempFile("codegen-not-a-dir", ".tmp").toFile();
+        try {
+            Generated entry = new Generated();
+            entry.setFilename(new File(fakeParent, "bundle.zip").getAbsolutePath());
+            entry.setFriendlyName("test");
+            entry.setCreatedAt(Instant.now().minusSeconds(25 * 3600));
+            genApiService.putFileEntry("test-deletion-fail-key", entry);
+
+            genApiService.cleanExpiredFiles();
+
+            assertNotNull(genApiService.getFileEntry("test-deletion-fail-key"), "Entry should be retained when deletion fails");
+        } finally {
+            fakeParent.delete();
+            genApiService.removeFileEntry("test-deletion-fail-key");
+        }
+    }
+
+    // Fix #1: recent entry is not evicted by cleanup
+    @Test
+    public void cleanExpiredFilesKeepsRecentEntry() throws Exception {
+        File tempDir = Files.createTempDirectory("codegen-test").toFile();
+        try {
+            Generated entry = new Generated();
+            entry.setFilename(new File(tempDir, "bundle.zip").getAbsolutePath());
+            entry.setFriendlyName("test");
+            genApiService.putFileEntry("test-recent-key", entry);
+
+            genApiService.cleanExpiredFiles();
+
+            assertNotNull(genApiService.getFileEntry("test-recent-key"), "Recent entry should not be evicted");
+        } finally {
+            genApiService.removeFileEntry("test-recent-key");
+            tempDir.delete();
+        }
+    }
+
+    // Fix #1: missing fileId returns 404
+    @Test
+    public void downloadMissingFileReturns404() throws Exception {
+        mockMvc.perform(get("/api/gen/download/nonexistent-id"))
+                .andExpect(status().isNotFound());
+    }
+
+    // Fix #1: existing entry past TTL returns 404 (TTL enforced at request time)
+    @Test
+    public void downloadExpiredEntryReturns404() throws Exception {
+        String result = mockMvc.perform(post("/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();
+
+        genApiService.getFileEntry(code).setCreatedAt(Instant.now().minusSeconds(25 * 3600));
+
+        mockMvc.perform(get("/api/gen/download/" + code))
+                .andExpect(status().isNotFound());
+    }
+
+    // Fix #1: concurrent generation does not lose entries (thread-safety)
+    @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("/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 succeed");
+        assertEquals(threads, codes.stream().distinct().count(), "All codes should be unique");
+    }
+}