feat: Implement Detection and Prevention of Concurrent Overwritesinitial commit

[Katerina-Charakhovich]

Applicable issues

• fixes Implement Detection and Prevention of Concurrent Overwrites #53

Description of changes

An optimistic lock has been added when updating entities

Checklist

• Title of the pull request follows Conventional Commits specification

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[siarhei-fedziukovich]

i think it's optimistic lock. (Description of changes in PR)

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[siarhei-fedziukovich]

tests needed

[Katerina-Charakhovich]

Done

[siarhei-fedziukovich]

i'd add tests which compare result with some etalon. not only verifying ignored fields

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[KirylKurnosenka]

"Use "*" to skip optimistic check." - are we sure we would like to provide clients with such hack?
@siarhei-fedziukovich

[siarhei-fedziukovich]

i think it'll be useful for some automatization case. but it's not strong willing. we can remove it if you see more harm than good

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[KirylKurnosenka]

Double whitespace after =
I'd suggest to assign the value inside constructor to align with ObjectWriter

[KirylKurnosenka]

Strange formatting, I think it can be formatted like

 return dtoWithHash.hash().equals(StringUtils.unwrap(previousHash, '"'))
        ? ResponseEntity.status(HttpStatus.NOT_MODIFIED).eTag(dtoWithHash.hash()).build()
        : ResponseEntity.status(HttpStatus.OK).eTag(dtoWithHash.hash()).body(dtoWithHash.dto());

or at least .build() and .body(dtoWithHash.dto()); should have the same indentation

[KirylKurnosenka]

Can @RequestHeader(value = "If-Match") and String previousHash be on the same line?

[KirylKurnosenka]

Doesn't that fit signle line?

[ai-dial-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[siarhei-fedziukovich]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 53 - Fully compliant Compliant requirements: Detect concurrent updates to the same model and prevent overwrites. When a conflict is detected, deny the save, display an error, and prompt the user to refresh. Ensure data integrity by preventing unintended overwrites. All tests pass and the feature works across use cases and edge cases.

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Hash Calculation Consistency The hash calculation logic uses Jackson serialization with a filter to ignore certain fields. Reviewers should verify that all fields that could cause false-positive conflicts (e.g., fields that change but are not relevant to business logic) are properly excluded, and that the hash is stable across environments and versions. package com.epam.aidial.cfg.service.hashing; import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectWriter; import com.fasterxml.jackson.databind.SerializationFeature; import com.fasterxml.jackson.databind.ser.impl.SimpleBeanPropertyFilter; import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Component; import java.security.MessageDigest; import java.util.Base64; import java.util.Set; @Component @Slf4j public class HashCalculator { public static final String ANY_HASH = "*"; private static final Set<String> IGNORED_FIELDS = Set.of("createdAt", "updatedAt"); private final ObjectWriter writer; private final Base64.Encoder base64Encoder; public HashCalculator(ObjectMapper mapper) { var hashingMapper = mapper.copy() .registerModule(new JavaTimeModule()) .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) .enable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY) .enable(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS) .enable(SerializationFeature.WRITE_BIGDECIMAL_AS_PLAIN) .addMixIn(Object.class, HashFilterMixin.class); var filters = new SimpleFilterProvider() .addFilter("hashFilter", SimpleBeanPropertyFilter.serializeAllExcept(IGNORED_FIELDS)) .setFailOnUnknownId(false); hashingMapper.setFilterProvider(filters); this.writer = hashingMapper.writer(); this.base64Encoder = Base64.getUrlEncoder().withoutPadding(); } public String calculateHash(Object body) { try { byte[] json = writer.writeValueAsBytes(body); var md = MessageDigest.getInstance("SHA-256"); byte[] digest = md.digest(json); return base64Encoder.encodeToString(digest); } catch (Exception e) { log.warn("Failed to compute hash for body: {}", toLoggableJson(body), e); throw new RuntimeException("Failed to compute hash", e); } } private String toLoggableJson(Object body) { try { return writer.writeValueAsString(body); } catch (Exception e) { return "Unable to serialize"; } } } API Header Handling The new endpoints require If-None-Match and If-Match headers for GET and PUT requests, respectively. Reviewers should check that this is clearly documented for API consumers and that missing/invalid headers are handled gracefully. public ResponseEntity<ModelDto> getModel(HttpServletResponse response, @PathVariable("modelName") String modelName, @RequestHeader(value = "If-None-Match") String previousHash) { var dtoWithHash = modelFacade.getModelWithHash(modelName); return dtoWithHash.hash().equals(StringUtils.unwrap(previousHash, '"')) ? ResponseEntity.status(HttpStatus.NOT_MODIFIED).eTag(dtoWithHash.hash()).build() : ResponseEntity.status(HttpStatus.OK).eTag(dtoWithHash.hash()).body(dtoWithHash.dto()); } @PostMapping(consumes = MediaType.APPLICATION_JSON_VALUE) @ResponseStatus(HttpStatus.NO_CONTENT) public void createModel(HttpServletResponse response, @RequestBody @Valid ModelDto modelDto) { modelFacade.createModel(modelDto); } @PutMapping(path = "/{modelName}", consumes = MediaType.APPLICATION_JSON_VALUE) public ResponseEntity<Void> updateModel(HttpServletResponse response, @PathVariable("modelName") String modelName, @RequestBody @Valid ModelDto modelDto, @RequestHeader(value = "If-Match") String previousHash) { var newHash = modelFacade.updateModel(modelName, modelDto, StringUtils.unwrap(previousHash, '"')); return ResponseEntity.status(HttpStatus.NO_CONTENT).eTag(newHash).build(); } Backward Compatibility The update logic now requires a hash for optimistic locking. Reviewers should verify that all code paths (including legacy or internal calls) are updated to use the new method signature, or that the fallback (ANY_HASH) is safe and does not reintroduce the original bug. public void updateModel(String modelName, Model model) { performUpdate(modelName, model, ANY_HASH); } @Transactional public String updateModel(String modelName, Model model, String hash) { if (hash == null) { throw new IllegalArgumentException( "Hash must not be null. Use \"*\" to skip optimistic check."); } var savedModel = performUpdate(modelName, model, hash); return calculator.calculateHash(mapper.toDomain(savedModel)); } private ModelEntity performUpdate(String modelName, Model model, String hash) { modelNormalizer.normalize(model); modelValidator.validateUpdate(modelName, model); ModelEntity modelEntity = modelJpaRepository.findById(modelName) .orElseThrow(() -> new EntityNotFoundException(NOT_FOUND_MESSAGE_TEMPLATE.formatted(modelName))); assertNewModelDisplayNameAndDisplayVersion(modelEntity, model); assertNotConcurrencyOverwrite(modelEntity, hash); return modelJpaRepository.save(mapper.toEntity(model, modelEntity)); }

[siarhei-fedziukovich]

let's mark PR as breaking change and add description of it's changes (helps build release notes)
also update docs/samle/http-requests/AdminPanel.http