fix: add validation for cron in page source during import to prevent malformed cron expressions

[vikrantgravitee]

Issue

https://gravitee.atlassian.net/browse/APIM-9446

Description

This commit adds validation logic for the cron expression field in page sources (e.g., GitHub fetcher). Previously, malformed cron expressions (e.g., with only 5 fields) were silently accepted during import or upgrade, leading to inconsistencies in behavior.

Issue:

APIM-9446.mov

Fix:

Fix_APIM-9446.mov

Additional context

---

📚  View the storybook of this branch here

[cypress]

APIM UI Tests    Run #5301

Run Properties:   Passed #5301  •  358c63b0d8: fix: add validation for cron in page source during import to prevent malformed c...

Project	APIM UI Tests
Branch Review	APIM-9446-fix-cron-check-api-import
Run status	Passed #5301
Run duration	06m 51s
Commit	358c63b0d8: fix: add validation for cron in page source during import to prevent malformed c...
Committer	vikrant
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	8
Skipped	0
Passing	82
View all changes introduced in this branch ↗︎

[jourdiw]

Should this be run whenever a page is created or modified?
I wonder if there is a more central page validation service to add this to? 🤔

[jourdiw]

Maybe this could throw a gravitee exception and add the exception to the signature?
I believe we try to avoid throwing RuntimeExceptions 🤔

[jourdiw]

There are two things that I suggest improving in this method:

1. Not to throw an exception, catch it, then rethrow it
2. Using exceptions in io.gravitee.rest.api.service.exceptions; instead of io.gravitee.repository.exceptions; to have the desired impact when the user calls the resource

By throwing a TechnicalException that is specific to repository errors, the user will receive a 500 error if their CRON is invalid. This should probably be a 400. You can create a new exception PageCronExpressionInvalidException (or however you want to name it) that extends AbstractManagementException and returns HttpStatusCode.BAD_REQUEST_400 for getHttpCode.

A suggested refactoring:

Suggested change

	public static void validateFetchConfig(String jsonConfig) throws TechnicalException {
	ObjectMapper mapper = new ObjectMapper();
	try {
	JsonNode root = mapper.readTree(jsonConfig);
	boolean autoFetch = root.path("autoFetch").asBoolean(false);
	String fetchCron = root.path("fetchCron").asText();
	if (autoFetch) {
	if (fetchCron == null || fetchCron.isEmpty()) {
	throw new TechnicalException("fetchCron is required when autoFetch is true.");
	}
	}
	if (fetchCron != null && !fetchCron.isEmpty() && !"null".equals(fetchCron)) {
	CronExpression.parse(fetchCron);
	}
	} catch (IllegalArgumentException e) {
	throw new TechnicalException("Invalid cron expression: " + e.getMessage(), e);
	} catch (Exception e) {
	throw new TechnicalException("Failed to parse configuration JSON: " + e.getMessage(), e);
	}
	}
	public static void validateFetchConfig(String jsonConfig) throws PageCronExpressionInvalidException {
	ObjectMapper mapper = new ObjectMapper();
	JsonNode root;
	try {
	root = mapper.readTree(jsonConfig);
	} catch (IOException e) {
	throw new PageCronExpressionInvalidException("Invalid JSON format: " + e.getMessage(), e);
	}
	boolean autoFetch = root.path("autoFetch").asBoolean(false);
	String fetchCron = root.path("fetchCron").asText(null); // default to null
	if (autoFetch && (fetchCron == null || fetchCron.trim().isEmpty() || "null".equalsIgnoreCase(fetchCron))) {
	throw new PageCronExpressionInvalidException("fetchCron is required when autoFetch is true.");
	}
	if (fetchCron != null && !fetchCron.trim().isEmpty() && !"null".equalsIgnoreCase(fetchCron)) {
	try {
	CronExpression.parse(fetchCron);
	} catch (IllegalArgumentException e) {
	throw new PageCronExpressionInvalidException("Invalid cron expression: " + e.getMessage(), e);
	}
	}
	}

[jourdiw]

You can probably make a cleaner solution that what I proposed...