Harden validation error recovery

This commit is contained in:
nessar committed 2026-08-17 14:18:16 +02:00
1 parent d16239881a
commit ce53bab698
11 files changed
+130 -7

No files matched your search

@@ -19,10 +19,14 @@ import org.slf4j.LoggerFactory;
import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.dao.OptimisticLockingFailureException;
import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity; import org.springframework.http.ResponseEntity;
import org.springframework.http.converter.HttpMessageNotReadableException;
import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.MissingServletRequestParameterException;
import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice; import org.springframework.web.bind.annotation.RestControllerAdvice;
import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException;
import org.springframework.web.multipart.MaxUploadSizeExceededException; import org.springframework.web.multipart.MaxUploadSizeExceededException;
import org.springframework.web.multipart.support.MissingServletRequestPartException;
@RestControllerAdvice(basePackages = "fr.nessar.lmnporganizer.rental.adapter.in.web") @RestControllerAdvice(basePackages = "fr.nessar.lmnporganizer.rental.adapter.in.web")
public class RentalApiExceptionHandler { public class RentalApiExceptionHandler {
@@ -93,6 +97,27 @@ ResponseEntity<ValidationApiError> invalidRequest(MethodArgumentNotValidExceptio
.toList())); .toList()));
} }
@ExceptionHandler(HttpMessageNotReadableException.class)
ResponseEntity<ValidationApiError> unreadableRequest() {
return validation("request", "malformed");
}
@ExceptionHandler(MethodArgumentTypeMismatchException.class)
ResponseEntity<ValidationApiError> invalidParameter(MethodArgumentTypeMismatchException exception) {
return validation(exception.getName(), "invalid");
}
@ExceptionHandler(MissingServletRequestParameterException.class)
ResponseEntity<ValidationApiError> missingParameter(
MissingServletRequestParameterException exception) {
return validation(exception.getParameterName(), "required");
}
@ExceptionHandler(MissingServletRequestPartException.class)
ResponseEntity<ValidationApiError> missingPart(MissingServletRequestPartException exception) {
return validation(exception.getRequestPartName(), "required");
}
@ExceptionHandler(Exception.class) @ExceptionHandler(Exception.class)
ResponseEntity<ApiError> unexpected(Exception exception) { ResponseEntity<ApiError> unexpected(Exception exception) {
LOGGER.error("Unhandled rental API exception", exception); LOGGER.error("Unhandled rental API exception", exception);
@@ -107,6 +132,13 @@ private static ResponseEntity<ApiError> error(HttpStatus status, String message)
return ResponseEntity.status(status).body(new ApiError(message)); return ResponseEntity.status(status).body(new ApiError(message));
} }
private static ResponseEntity<ValidationApiError> validation(String field, String code) {
return ResponseEntity.badRequest()
.body(
new ValidationApiError(
"validation_failed", java.util.List.of(new FieldValidationError(field, code))));
}
record ApiError(String message) {} record ApiError(String message) {}
record CodedApiError(String code, String message) {} record CodedApiError(String code, String message) {}
@@ -2,6 +2,7 @@ package fr.nessar.lmnporganizer.rental.adapter.in.web;
import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
@@ -31,6 +32,7 @@ import fr.nessar.lmnporganizer.rental.application.apartment.ApartmentCatalogServ
import fr.nessar.lmnporganizer.rental.application.landlord.LandlordAccountService; import fr.nessar.lmnporganizer.rental.application.landlord.LandlordAccountService;
import fr.nessar.lmnporganizer.rental.application.lease.AmendLeaseService; import fr.nessar.lmnporganizer.rental.application.lease.AmendLeaseService;
import fr.nessar.lmnporganizer.rental.application.lease.GenerateLeaseDocumentService; import fr.nessar.lmnporganizer.rental.application.lease.GenerateLeaseDocumentService;
import fr.nessar.lmnporganizer.rental.application.lease.LeaseClauseCatalog;
import fr.nessar.lmnporganizer.rental.application.lease.LeaseDraftService; import fr.nessar.lmnporganizer.rental.application.lease.LeaseDraftService;
import fr.nessar.lmnporganizer.rental.application.rent.AllocateRentPaymentService; import fr.nessar.lmnporganizer.rental.application.rent.AllocateRentPaymentService;
import fr.nessar.lmnporganizer.rental.application.rent.GeneratePaymentReceiptService; import fr.nessar.lmnporganizer.rental.application.rent.GeneratePaymentReceiptService;
@@ -126,6 +128,7 @@ private static final RentPaymentRefundId REFUND_ID = new RentPaymentRefundId("re
@MockitoBean private LeaseDraftService leaseDrafts; @MockitoBean private LeaseDraftService leaseDrafts;
@MockitoBean private AmendLeaseService amendments; @MockitoBean private AmendLeaseService amendments;
@MockitoBean private GenerateLeaseDocumentService leaseDocuments; @MockitoBean private GenerateLeaseDocumentService leaseDocuments;
@MockitoBean private LeaseClauseCatalog leaseClauseCatalog;
@MockitoBean private RentPaymentHistoryService paymentHistory; @MockitoBean private RentPaymentHistoryService paymentHistory;
@MockitoBean private RecordRentPaymentService paymentRecording; @MockitoBean private RecordRentPaymentService paymentRecording;
@MockitoBean private AllocateRentPaymentService paymentAllocation; @MockitoBean private AllocateRentPaymentService paymentAllocation;
@@ -136,9 +139,17 @@ private static final RentPaymentRefundId REFUND_ID = new RentPaymentRefundId("re
@BeforeEach @BeforeEach
void authenticateLandlordAndFixTime() { void authenticateLandlordAndFixTime() {
Instant now = Instant.parse("2026-03-01T12:00:00Z");
when(accounts.identify(any())).thenReturn(landlord()); when(accounts.identify(any())).thenReturn(landlord());
when(clock.instant()).thenReturn(Instant.parse("2026-03-01T12:00:00Z")); when(clock.instant()).thenReturn(now);
when(clock.getZone()).thenReturn(ZoneOffset.UTC); when(clock.getZone()).thenReturn(ZoneOffset.UTC);
when(clock.withZone(any())).thenReturn(Clock.fixed(now, ZoneOffset.UTC));
LeaseClauseCatalog clauses =
LeaseClauseCatalog.load(getClass().getResourceAsStream("/lease-clauses.yml"));
when(leaseClauseCatalog.available(anyString(), any()))
.thenAnswer(
invocation ->
clauses.available(invocation.getArgument(0), invocation.getArgument(1)));
} }
@Test @Test
@@ -161,6 +172,25 @@ void protectedRentalMutationsRejectMissingCsrfTokens() throws Exception {
.andExpect(status().isForbidden()); .andExpect(status().isForbidden());
} }
@Test
void malformedJsonIsAStableClientValidationFailure() throws Exception {
mockMvc
.perform(
post("/api/apartments")
.with(asLandlord())
.with(csrf())
.contentType(MediaType.APPLICATION_JSON)
.content(
"""
{"name":"Studio","address":"12 rue A","purchasePrice":"not-a-number",
"description":"","privateAreas":[]}
"""))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.code").value("validation_failed"))
.andExpect(jsonPath("$.errors[0].field").value("request"))
.andExpect(jsonPath("$.errors[0].code").value("malformed"));
}
@Test @Test
void oidcClaimsScopeApartmentCreationAndLookupAndResponsesAreMapped() throws Exception { void oidcClaimsScopeApartmentCreationAndLookupAndResponsesAreMapped() throws Exception {
Apartment apartment = Apartment apartment =
@@ -413,7 +443,11 @@ void leaseDraftRequestsRejectAnImpossibleAnnualRevisionDay() throws Exception {
"referenceYear":2026,"seriesId":"001515333"}}} "referenceYear":2026,"seriesId":"001515333"}}}
""")) """))
.andExpect(status().isBadRequest()) .andExpect(status().isBadRequest())
.andExpect(jsonPath("$.message").value("Le jour annuel de révision IRL est invalide.")); .andExpect(jsonPath("$.code").value("validation_failed"))
.andExpect(
jsonPath("$.errors[0].field")
.value("legalTerms.rentRevision.annualRevisionDay"))
.andExpect(jsonPath("$.errors[0].code").value("invalid_date"));
} }
@Test @Test
@@ -16,6 +16,7 @@ import fr.nessar.lmnporganizer.rental.adapter.in.web.common.RentalApiExceptionHa
import fr.nessar.lmnporganizer.rental.application.landlord.LandlordAccountService; import fr.nessar.lmnporganizer.rental.application.landlord.LandlordAccountService;
import fr.nessar.lmnporganizer.rental.application.lease.AmendLeaseService; import fr.nessar.lmnporganizer.rental.application.lease.AmendLeaseService;
import fr.nessar.lmnporganizer.rental.application.lease.GenerateLeaseDocumentService; import fr.nessar.lmnporganizer.rental.application.lease.GenerateLeaseDocumentService;
import fr.nessar.lmnporganizer.rental.application.lease.LeaseClauseCatalog;
import fr.nessar.lmnporganizer.rental.application.lease.LeaseDraftService; import fr.nessar.lmnporganizer.rental.application.lease.LeaseDraftService;
import fr.nessar.lmnporganizer.rental.domain.landlord.ExternalIdentity; import fr.nessar.lmnporganizer.rental.domain.landlord.ExternalIdentity;
import fr.nessar.lmnporganizer.rental.domain.landlord.Landlord; import fr.nessar.lmnporganizer.rental.domain.landlord.Landlord;
@@ -43,6 +44,7 @@ class LeaseAmendmentWebTest {
@MockitoBean LeaseDraftService drafts; @MockitoBean LeaseDraftService drafts;
@MockitoBean AmendLeaseService amendments; @MockitoBean AmendLeaseService amendments;
@MockitoBean GenerateLeaseDocumentService documents; @MockitoBean GenerateLeaseDocumentService documents;
@MockitoBean LeaseClauseCatalog clauseCatalog;
@MockitoBean Clock clock; @MockitoBean Clock clock;
@BeforeEach @BeforeEach
@@ -131,7 +131,7 @@ export class LeaseClauseEditor {
this.emit( this.emit(
selected ? [...clauses, { catalogId: id, parameters: {}, customText: '' }] : clauses, selected ? [...clauses, { catalogId: id, parameters: {}, customText: '' }] : clauses,
); );
this.fieldChanged.emit('specialClauses[' + index + '].catalogId'); this.fieldChanged.emit(selected ? 'specialClauses[' + index + '].catalogId' : 'specialClauses[');
} }
parameterValue(catalogId: string, parameterName: string): string { parameterValue(catalogId: string, parameterName: string): string {
@@ -178,6 +178,7 @@ export class LeaseClauseEditor {
removeClause(index: number): void { removeClause(index: number): void {
this.emit(this.specialClauses().filter((_, clauseIndex) => clauseIndex !== index)); this.emit(this.specialClauses().filter((_, clauseIndex) => clauseIndex !== index));
this.fieldChanged.emit('specialClauses[');
} }
private load(): void { private load(): void {
@@ -110,4 +110,23 @@ describe('LeaseEndNoticeCard', () => {
}); });
expect(downloadEvidence).toHaveBeenCalledWith('lease-1', 'notice-1', 1, 'preuve.jpg'); expect(downloadEvidence).toHaveBeenCalledWith('lease-1', 'notice-1', 1, 'preuve.jpg');
}); });
it('clears a stale evidence error when its file or reason is corrected', () => {
const fixture = createFixture();
const input = document.createElement('input');
const evidence = new File(['new'], 'preuve.txt', { type: 'text/plain' });
Object.defineProperty(input, 'files', { value: { item: () => evidence } });
fixture.componentInstance.evidenceError.set('Sélectionnez une preuve à conserver.');
fixture.componentInstance.selectEvidence({ target: input } as unknown as Event);
expect(fixture.componentInstance.evidenceError()).toBe('');
fixture.componentInstance.evidenceError.set(
'Indiquez la raison de cette nouvelle version de preuve.',
);
const reason = document.createElement('input');
reason.value = 'Version lisible';
fixture.componentInstance.updateEvidenceReason({ target: reason } as unknown as Event);
expect(fixture.componentInstance.evidenceError()).toBe('');
});
}); });
@@ -114,12 +114,14 @@ export class LeaseEndNoticeCard {
this.evidence.set( this.evidence.set(
input instanceof HTMLInputElement ? (input.files?.item(0) ?? null) : null, input instanceof HTMLInputElement ? (input.files?.item(0) ?? null) : null,
); );
this.evidenceError.set('');
if (input instanceof HTMLInputElement) input.value = ''; if (input instanceof HTMLInputElement) input.value = '';
} }
updateEvidenceReason(event: Event): void { updateEvidenceReason(event: Event): void {
const input = event.target; const input = event.target;
this.evidenceReason.set(input instanceof HTMLInputElement ? input.value : ''); this.evidenceReason.set(input instanceof HTMLInputElement ? input.value : '');
this.evidenceError.set('');
} }
submitWithdrawal(): void { submitWithdrawal(): void {
@@ -202,6 +202,7 @@ export class LeaseForm {
this.guarantors.update((guarantors) => this.guarantors.update((guarantors) =>
guarantors.filter((_, itemIndex) => itemIndex !== index), guarantors.filter((_, itemIndex) => itemIndex !== index),
); );
this.fieldWasChanged('guarantors[');
} }
updateGuarantor(index: number, guarantor: Guarantor): void { updateGuarantor(index: number, guarantor: Guarantor): void {
@@ -103,6 +103,8 @@ describe('PrivateAreaEditor', () => {
]); ]);
const changed = vi.fn(); const changed = vi.fn();
fixture.componentInstance.areasChanged.subscribe(changed); fixture.componentInstance.areasChanged.subscribe(changed);
const fieldChanged = vi.fn();
fixture.componentInstance.fieldChanged.subscribe(fieldChanged);
const family = document.createElement('select'); const family = document.createElement('select');
family.append(new Option('Annexe', 'ANNEXE')); family.append(new Option('Annexe', 'ANNEXE'));
family.value = 'ANNEXE'; family.value = 'ANNEXE';
@@ -122,5 +124,6 @@ describe('PrivateAreaEditor', () => {
); );
expect(changed.mock.calls[1][0][0]).toEqual(expect.objectContaining({ label: 'Cave' })); expect(changed.mock.calls[1][0][0]).toEqual(expect.objectContaining({ label: 'Cave' }));
expect(changed.mock.calls[2][0]).toEqual([]); expect(changed.mock.calls[2][0]).toEqual([]);
expect(fieldChanged).toHaveBeenCalledWith('privateAreas[');
}); });
}); });
@@ -104,6 +104,7 @@ export class PrivateAreaEditor {
removeArea(id: string): void { removeArea(id: string): void {
this.areasChanged.emit(this.areas().filter((area) => area.id !== id)); this.areasChanged.emit(this.areas().filter((area) => area.id !== id));
this.fieldChanged.emit('privateAreas[');
} }
validate(): boolean { validate(): boolean {
@@ -108,8 +108,10 @@ export class WorkspaceResource<T extends { id?: string }> {
clearSaveFieldError(key: string, field: string): void { clearSaveFieldError(key: string, field: string): void {
const fieldErrors = this.saveFieldErrors()[key]; const fieldErrors = this.saveFieldErrors()[key];
if (!fieldErrors?.some((error) => error.field === field)) return; const matches = (candidate: string) =>
const remaining = fieldErrors.filter((error) => error.field !== field); field.endsWith('[') ? candidate.startsWith(field) : candidate === field;
if (!fieldErrors?.some((error) => matches(error.field))) return;
const remaining = fieldErrors.filter((error) => !matches(error.field));
this.saveFieldErrors.update((errors) => this.saveFieldErrors.update((errors) =>
remaining.length ? { ...errors, [key]: remaining } : withoutRecordKey(errors, key), remaining.length ? { ...errors, [key]: remaining } : withoutRecordKey(errors, key),
); );
@@ -328,6 +328,30 @@ describe('WorkspaceService', () => {
); );
}); });
it('clears indexed apartment errors after a structural collection change', () => {
apartments.updateApartment.mockReturnValueOnce(
throwError(() => ({
status: 400,
error: {
code: 'validation_failed',
errors: [
{ field: 'privateAreas[0].label', code: 'not_blank' },
{ field: 'privateAreas[1].family', code: 'not_null' },
{ field: 'name', code: 'not_blank' },
],
},
})),
);
const service = TestBed.inject(WorkspaceService);
service.saveApartment(apartment, apartment.id);
service.clearApartmentSaveFieldError('privateAreas[', apartment.id);
expect(service.apartmentSaveFieldErrors(apartment.id)).toEqual([
{ field: 'name', code: 'not_blank' },
]);
});
it('removes a confirmed deletion locally before reconciliation finishes', () => { it('removes a confirmed deletion locally before reconciliation finishes', () => {
const deleteResponse = new Subject<void>(); const deleteResponse = new Subject<void>();
const refreshResponse = new Subject<(typeof apartment)[]>(); const refreshResponse = new Subject<(typeof apartment)[]>();
@@ -529,12 +553,14 @@ describe('WorkspaceService', () => {
throwError(() => ({ error: { message: 'Format refusé' } })), throwError(() => ({ error: { message: 'Format refusé' } })),
); );
service.uploadTenantAttachment('tenant-1', new File(['id'], 'id.pdf')); service.uploadTenantAttachment('tenant-1', new File(['id'], 'id.pdf'));
expect(service.tenantAttachmentErrors()['tenant-1']).toBe('Format refusé'); expect(service.tenantAttachmentErrors()['tenant-1']).toBe(
'Impossible d’envoyer cette pièce jointe. Vérifiez le fichier et sa taille maximale.',
);
tenants.uploadAttachment.mockReturnValueOnce(throwError(() => new Error('offline'))); tenants.uploadAttachment.mockReturnValueOnce(throwError(() => new Error('offline')));
service.uploadTenantAttachment('tenant-1', new File(['id'], 'id.pdf')); service.uploadTenantAttachment('tenant-1', new File(['id'], 'id.pdf'));
expect(service.tenantAttachmentErrors()['tenant-1']).toBe( expect(service.tenantAttachmentErrors()['tenant-1']).toBe(
'Impossible d’envoyer cette pièce jointe.', 'Impossible d’envoyer cette pièce jointe. Vérifiez le fichier et sa taille maximale.',
); );
}); });