Skip to content

Code formatting while saving #382

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
stijnpotters1 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Code formatting while saving #382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d20e6be
fix: code formatting not every save, but on add
stijnpotters1 Mar 24, 2026
c2e92cf
fix: applied linting
stijnpotters1 Mar 24, 2026
ba4f81e
Apply suggestions from code review
stijnpotters1 Mar 24, 2026
272484e
fix: exception error in controller
stijnpotters1 Mar 24, 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
4 changes: 1 addition & 3 deletions src/main/frontend/app/routes/editor/editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { useShallow } from 'zustand/react/shallow'
import SidebarLayout from '~/components/sidebars-layout/sidebar-layout'
import SidebarContentClose from '~/components/sidebars-layout/sidebar-content-close'
import { SidebarSide } from '~/components/sidebars-layout/sidebar-layout-store'
import SidebarClose from '~/components/sidebars-layout/sidebar-close'
import { useTheme } from '~/hooks/use-theme'
import { useCallback, useEffect, useRef, useState } from 'react'
import { useProjectStore } from '~/stores/project-store'
Expand Down Expand Up @@ -181,8 +180,7 @@ export default function CodeEditor() {

setSaveStatus('saving')
try {
const xmlResponse = await saveConfiguration(project.name, configPath, updatedContent)
setXmlContent(xmlResponse.xmlContent)
await saveConfiguration(project.name, configPath, updatedContent)
contentCacheRef.current.set(activeTabFilePath, updatedContent)
setSaveStatus('saved')
if (savedTimerRef.current) clearTimeout(savedTimerRef.current)
Expand Down
2 changes: 1 addition & 1 deletion src/main/frontend/app/routes/studio/canvas/flow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ function FlowCanvas() {
await saveConfiguration(currentProject.name, configurationPath, updatedConfigXml)
clearConfigurationCache(currentProject.name, configurationPath)
useEditorTabStore.getState().refreshAllTabs()
if (currentProject.isGitRepository) refreshOpenDiffs(currentProject.name)
if (currentProject.isGitRepository) await refreshOpenDiffs(currentProject.name)

setSaveStatus('saved')
if (savedTimerRef.current) clearTimeout(savedTimerRef.current)
Expand Down
6 changes: 3 additions & 3 deletions src/main/frontend/app/services/configuration-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { apiFetch } from '~/utils/api'
import type { Project, XmlResponse } from '~/types/project.types'
import type { Project } from '~/types/project.types'

const configCache = new Map<string, string>()

Expand Down Expand Up @@ -32,8 +32,8 @@ export async function fetchConfiguration(projectName: string, filepath: string,
return data.content
}

export async function saveConfiguration(projectName: string, filepath: string, content: string): Promise<XmlResponse> {
return apiFetch<XmlResponse>(`/projects/${encodeURIComponent(projectName)}/configuration`, {
export async function saveConfiguration(projectName: string, filepath: string, content: string): Promise<void> {
return apiFetch<void>(`/projects/${encodeURIComponent(projectName)}/configuration`, {
method: 'PUT',
body: JSON.stringify({ filepath, content }),
})
Comment on lines +36 to 39
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

fetchConfigurationCached uses the module-level configCache, but saveConfiguration doesn't update or invalidate that cache. After saving, other screens that rely on fetchConfigurationCached can read stale XML until a manual cache clear happens. Consider updating configCache for the saved key (set it to content) or deleting that key once the PUT succeeds.

Suggested change
return apiFetch<void>(`/projects/${encodeURIComponent(projectName)}/configuration`, {
method: 'PUT',
body: JSON.stringify({ filepath, content }),
})
await apiFetch<void>(`/projects/${encodeURIComponent(projectName)}/configuration`, {
method: 'PUT',
body: JSON.stringify({ filepath, content }),
})
const key = `${projectName}:${filepath}`
configCache.set(key, content)

Copilot uses AI. Check for mistakes.
Expand Down
1 change: 0 additions & 1 deletion src/main/java/org/frankframework/flow/adapter/AdapterService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ public boolean updateAdapter(Path configurationFile, String adapterName, String
String updatedXml = XmlConfigurationUtils.convertNodeToString(configDoc);
Files.writeString(absConfigFile, updatedXml, StandardCharsets.UTF_8, StandardOpenOption.TRUNCATE_EXISTING);
return true;

} catch (AdapterNotFoundException e) {
throw e;
} catch (Exception e) {
Expand Down
14 changes: 5 additions & 9 deletions src/main/java/org/frankframework/flow/configuration/ConfigurationController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import org.frankframework.flow.project.ProjectDTO;
import org.frankframework.flow.project.ProjectNotFoundException;
import org.frankframework.flow.project.ProjectService;
import org.frankframework.flow.xml.XmlDTO;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
Expand Down Expand Up @@ -39,20 +38,17 @@ public ResponseEntity<ConfigurationDTO> getConfigurationByPath(@RequestBody Conf
}

@PutMapping("/{projectName}/configuration")
public ResponseEntity<XmlDTO> updateConfiguration(
public ResponseEntity<Void> updateConfiguration(
@RequestBody ConfigurationDTO configurationDTO)
throws ConfigurationNotFoundException, IOException, ParserConfigurationException,
SAXException, TransformerException {
String updatedContent = configurationService.updateConfiguration(
configurationDTO.filepath(), configurationDTO.content());
XmlDTO xmlDTO = new XmlDTO(updatedContent);
return ResponseEntity.ok(xmlDTO);
throws ConfigurationNotFoundException, IOException {
configurationService.updateConfiguration(configurationDTO.filepath(), configurationDTO.content());
return ResponseEntity.ok().build();
}

@PostMapping("/{projectName}/configurations/{configName}")
public ResponseEntity<ProjectDTO> addConfiguration(
@PathVariable String projectName, @PathVariable String configName)
throws ProjectNotFoundException, IOException {
throws ProjectNotFoundException, IOException, ParserConfigurationException, TransformerException, SAXException {
Comment on lines 49 to +51
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

This endpoint now declares TransformerException, but GlobalExceptionHandler does not map TransformerException to the standard ErrorResponse shape. If this exception escapes, Spring’s default error JSON will likely break the frontend apiFetch error parsing (it expects httpStatus/messages). Prefer catching/wrapping XML transform errors into ApiException (or adding a dedicated handler) to keep error responses consistent.

Copilot uses AI. Check for mistakes.
Project project = configurationService.addConfiguration(projectName, configName);
Comment on lines 48 to 52
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

addConfiguration is now declared to throw TransformerException, but there is no global @ExceptionHandler for TransformerException (unlike SAXException / ParserConfigurationException). This will likely result in inconsistent/non-JSON 500 responses if a transform fails. Consider catching TransformerException here and wrapping it in ApiException (or adding a global handler) to keep error responses consistent.

Copilot uses AI. Check for mistakes.
return ResponseEntity.ok(projectService.toDto(project));
}
Expand Down
24 changes: 12 additions & 12 deletions src/main/java/org/frankframework/flow/configuration/ConfigurationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,8 @@ public String getConfigurationContent(String filepath) throws IOException, Confi
return fileSystemStorage.readFile(filePath.toString());
}

public String updateConfiguration(String filepath, String content)
throws IOException, ConfigurationNotFoundException, ParserConfigurationException, SAXException,
TransformerException {
public boolean updateConfiguration(String filepath, String content)
throws IOException, ConfigurationNotFoundException {
Path absolutePath = fileSystemStorage.toAbsolutePath(filepath);

if (!Files.exists(absolutePath)) {
Expand All @@ -56,16 +55,13 @@ public String updateConfiguration(String filepath, String content)
if (Files.isDirectory(absolutePath)) {
throw new ConfigurationNotFoundException("Invalid file path: " + filepath);
}
Document updatedDocument = XmlConfigurationUtils.insertFlowNamespace(content);
String updatedContent = XmlConfigurationUtils.convertNodeToString(updatedDocument);

// Just write to the disk. ProjectService reads directly from disk now!
fileSystemStorage.writeFile(absolutePath.toString(), updatedContent);
return updatedContent;
fileSystemStorage.writeFile(absolutePath.toString(), content);
return true;
Comment on lines 47 to +60
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

updateConfiguration now returns boolean but always returns true, and the controller ignores the return value. This makes the API misleading and adds unnecessary branching for callers/tests. Consider changing the method to void (or return something meaningful) and removing the unconditional return true.

Copilot uses AI. Check for mistakes.
}

public Project addConfiguration(String projectName, String configurationName)
throws ProjectNotFoundException, IOException {
throws ProjectNotFoundException, IOException, TransformerException, ParserConfigurationException, SAXException {
Project project = projectService.getProject(projectName);
Comment on lines 63 to 65
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

addConfiguration() now propagates TransformerException/ParserConfigurationException/SAXException to the controller layer. GlobalExceptionHandler currently handles SAX/ParserConfiguration but not TransformerException, which can lead to inconsistent error payloads for clients. Consider catching TransformerException here and rethrowing an ApiException (or otherwise ensuring it’s handled consistently).

Copilot uses AI. Check for mistakes.

Path absProjectPath = fileSystemStorage.toAbsolutePath(project.getRootPath());
Expand All @@ -81,14 +77,16 @@ public Project addConfiguration(String projectName, String configurationName)
}

String defaultXml = loadDefaultConfigurationXml();
fileSystemStorage.writeFile(filePath.toString(), defaultXml);
Document updatedDocument = XmlConfigurationUtils.insertFlowNamespace(defaultXml);
String updatedContent = XmlConfigurationUtils.convertNodeToString(updatedDocument);
fileSystemStorage.writeFile(filePath.toString(), updatedContent);

// Returning the project handles everything, as 'toDto' will pick up the new file
return project;
}

public Project addConfigurationToFolder(String projectName, String configurationName, String folderPath)
throws IOException, ApiException {
throws IOException, ApiException, ParserConfigurationException, SAXException, TransformerException {
Project project = projectService.getProject(projectName);
Comment on lines 88 to 90
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

addConfigurationToFolder() now declares TransformerException. Unlike SAXException/ParserConfigurationException, TransformerException is not handled by GlobalExceptionHandler, so this can leak as a default Spring error response that doesn’t match the frontend’s expected error shape. Handle/wrap TransformerException (e.g., into ApiException) to keep API error responses consistent.

Copilot uses AI. Check for mistakes.

Path absProjectPath = fileSystemStorage.toAbsolutePath(project.getRootPath());
Expand All @@ -112,7 +110,9 @@ public Project addConfigurationToFolder(String projectName, String configuration
}

String defaultXml = loadDefaultConfigurationXml();
fileSystemStorage.writeFile(filePath.toString(), defaultXml);
Document updatedDocument = XmlConfigurationUtils.insertFlowNamespace(defaultXml);
String updatedContent = XmlConfigurationUtils.convertNodeToString(updatedDocument);
fileSystemStorage.writeFile(filePath.toString(), updatedContent);

return project;
}
Expand Down
13 changes: 10 additions & 3 deletions src/main/java/org/frankframework/flow/filetree/FileTreeController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.frankframework.flow.filetree;

import java.io.IOException;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.TransformerException;
import org.frankframework.flow.exception.ApiException;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
Expand All @@ -13,6 +15,7 @@
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.xml.sax.SAXException;

@RestController
@RequestMapping("/projects")
Expand Down Expand Up @@ -48,9 +51,13 @@ public FileTreeNode getDirectoryContent(@PathVariable String projectName, @Reque

@PostMapping("/{projectName}/files")
public ResponseEntity<FileTreeNode> createFile(@PathVariable String projectName, @RequestBody FileCreateDTO dto)
throws IOException, ApiException {
FileTreeNode node = fileTreeService.createFile(projectName, dto.path(), dto.name());
return ResponseEntity.status(HttpStatus.CREATED.value()).body(node);
throws IOException, ApiException, ParserConfigurationException, SAXException {
try {
FileTreeNode node = fileTreeService.createFile(projectName, dto.path(), dto.name());
return ResponseEntity.status(HttpStatus.CREATED.value()).body(node);
} catch (TransformerException e) {
throw new ApiException("Failed to create file for project '" + projectName + "' at path '" + dto.path() + "'", HttpStatus.INTERNAL_SERVER_ERROR, e);
}
}

@PostMapping("/{projectName}/folders")
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/frankframework/flow/filetree/FileTreeService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.TransformerException;
import org.frankframework.flow.configuration.ConfigurationService;
import org.frankframework.flow.exception.ApiException;
import org.frankframework.flow.filesystem.FileSystemStorage;
Expand All @@ -23,6 +25,7 @@
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.NodeList;
import org.xml.sax.SAXException;
import org.xml.sax.helpers.DefaultHandler;

@Service
Expand Down Expand Up @@ -127,10 +130,11 @@ public FileTreeNode getConfigurationsDirectoryTree(String projectName) throws IO
}

public FileTreeNode createFile(String projectName, String parentPath, String fileName)
throws IOException, ApiException {
throws IOException, ApiException, ParserConfigurationException, TransformerException, SAXException {
if (parentPath == null || parentPath.isBlank()) {
throw new IllegalArgumentException("Parent path must not be empty");
}

validateFileName(fileName);
String fullPath = parentPath.endsWith("/") ? parentPath + fileName : parentPath + "/" + fileName;
validateWithinProject(projectName, fullPath);
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/frankframework/flow/configuration/ConfigurationControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void updateConfigurationSuccessReturns200() throws Exception {
String xmlContent = "<xml>updated</xml>";

when(configurationService.updateConfiguration(filepath, xmlContent))
.thenReturn(xmlContent);
.thenReturn(true);

mockMvc.perform(
put("/api/projects/" + TEST_PROJECT_NAME + "/configuration")
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/frankframework/flow/configuration/ConfigurationServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ void updateConfiguration_Success() throws Exception {

configurationService.updateConfiguration(file.toString(), "<new/>");

assertEquals("<new/>\n", Files.readString(file, StandardCharsets.UTF_8));
verify(fileSystemStorage).writeFile(file.toString(), "<new/>\n");
assertEquals("<new/>", Files.readString(file, StandardCharsets.UTF_8));
verify(fileSystemStorage).writeFile(file.toString(), "<new/>");
}

@Test
Expand Down
7 changes: 5 additions & 2 deletions src/test/java/org/frankframework/flow/filetree/FileTreeServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import java.nio.file.Paths;
import java.util.Comparator;
import java.util.List;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.TransformerException;
import org.frankframework.flow.configuration.ConfigurationService;
import org.frankframework.flow.exception.ApiException;
import org.frankframework.flow.filesystem.FileOperations;
Expand All @@ -27,6 +29,7 @@
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.xml.sax.SAXException;

@ExtendWith(MockitoExtension.class)
public class FileTreeServiceTest {
Expand Down Expand Up @@ -337,7 +340,7 @@ void createFile_NameWithDoubleDots_ThrowsIllegalArgument() {

@Test
@DisplayName("Should create a file and return a FileTreeNode with FILE type")
void createFile_Success() throws IOException, ProjectNotFoundException, ApiException {
void createFile_Success() throws IOException, ApiException, ParserConfigurationException, TransformerException, SAXException {
stubToAbsolutePath();
stubCreateFile();

Expand All @@ -358,7 +361,7 @@ void createFile_Success() throws IOException, ProjectNotFoundException, ApiExcep
@Test
@DisplayName("Should create a file correctly when parent path already ends with a slash")
void createFile_ParentPathWithTrailingSlash_DoesNotDoubleSlash()
throws IOException, ProjectNotFoundException, ApiException {
throws IOException, ApiException, ParserConfigurationException, TransformerException, SAXException {
stubToAbsolutePath();
stubCreateFile();

Expand Down
Loading