Skip to content

Improve validation response #8985

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

Improve validation response #8985

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f25214b
Create new MetadataValidationProcessingReport & SchematronValidationR…
tylerjmchugh Aug 11, 2025
083e849
Create new gnBatchValidationReport directive
tylerjmchugh Aug 11, 2025
16b01e7
Implement new reports and directive
tylerjmchugh Aug 11, 2025
033b6a5
Remove unused imports
tylerjmchugh Aug 14, 2025
449d1b5
Move restructureReportToHavePatternRuleHierarchy to avoid calling it …
tylerjmchugh Aug 14, 2025
9d48f59
Update table styles to scroll
tylerjmchugh Aug 14, 2025
d6cccc9
Add link to record
tylerjmchugh Aug 14, 2025
f703904
Remove validation report caching
tylerjmchugh Sep 2, 2025
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
28 changes: 17 additions & 11 deletions core/src/main/java/org/fao/geonet/kernel/datamanager/base/BaseMetadataValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,6 @@
import org.springframework.context.annotation.Lazy;
import org.springframework.transaction.annotation.Transactional;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import jeeves.server.UserSession;
import jeeves.server.context.ServiceContext;

Expand Down Expand Up @@ -416,20 +411,31 @@ public Pair<Element, Boolean> doValidate(AbstractMetadata metadata, String lang)
theNSs.add(Namespace.getNamespace("geonet", "http://www.fao.org/geonetwork"));
theNSs.add(Namespace.getNamespace("svrl", "http://purl.oclc.org/dsdl/svrl"));

List<?> failedAssert = Xml.selectNodes(schemaTronReport,
"geonet:report[@geonet:required = '" + SchematronRequirement.REQUIRED + "']/svrl:schematron-output/svrl:failed-assert",
// Get all the errors
Copy link
Member

Choose a reason for hiding this comment

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

The file contains some non-used imports, I'm not sure if related to the pull request, but can you update it to remove them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in latest push

List<?> errors = Xml.selectNodes(schemaTronReport,
"geonet:report[@geonet:required = '" + SchematronRequirement.REQUIRED + "']/svrl:schematron-output/svrl:failed-assert" +
" | geonet:report[@geonet:required = '" + SchematronRequirement.REQUIRED + "']/geonet:schematronVerificationError",
theNSs);

List<?> failedSchematronVerification = Xml.selectNodes(schemaTronReport,
"geonet:report[@geonet:required = '" + SchematronRequirement.REQUIRED + "']/geonet:schematronVerificationError",
// Get all the warnings
List<?> warnings = Xml.selectNodes(schemaTronReport,
"geonet:report[@geonet:required = '" + SchematronRequirement.REPORT_ONLY + "']/svrl:schematron-output/svrl:failed-assert" +
" | geonet:report[@geonet:required = '" + SchematronRequirement.REPORT_ONLY + "']/geonet:schematronVerificationError",
theNSs);

if (failedAssert.size() > 0 || failedSchematronVerification.size() > 0) {
// If there are errors the report is not valid
if (!errors.isEmpty()) {
valid = false;
errorReport.addContent(schemaTronReport);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug(" - Schematron error: {}", Xml.getString(schemaTronReport));
}
} else if (!warnings.isEmpty()) {
LOGGER.debug(" - Schematron warning: {}", Xml.getString(schemaTronReport));
}

// Add the schematron report content if there are errors or warnings
if (!errors.isEmpty() || !warnings.isEmpty()) {
errorReport.addContent(schemaTronReport);
}
}
} catch (Exception e) {
Expand Down
141 changes: 114 additions & 27 deletions services/src/main/java/org/fao/geonet/api/processing/ValidateApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@

package org.fao.geonet.api.processing;

import java.util.ArrayList;
import java.util.List;
import java.util.*;
import com.google.common.collect.Lists;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.responses.ApiResponses;
Expand All @@ -35,8 +35,10 @@
import org.fao.geonet.ApplicationContextHolder;
import org.fao.geonet.api.ApiParams;
import org.fao.geonet.api.ApiUtils;
import org.fao.geonet.api.processing.report.MetadataValidationProcessingReport;
import org.fao.geonet.api.processing.report.SimpleMetadataProcessingReport;
import org.fao.geonet.api.processing.report.registry.IProcessingReportRegistry;
import org.fao.geonet.constants.Geonet;
import org.fao.geonet.domain.AbstractMetadata;
import org.fao.geonet.domain.MetadataValidation;
import org.fao.geonet.domain.Pair;
Expand All @@ -53,9 +55,8 @@
import org.fao.geonet.kernel.setting.Settings;
import org.fao.geonet.repository.MetadataValidationRepository;
import org.fao.geonet.kernel.search.index.BatchOpsMetadataReindexer;
import org.fao.geonet.utils.Xml;
import org.jdom.Element;
import org.jdom.Namespace;
import org.jdom.filter.ElementFilter;
import org.jdom.filter.Filter;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationContext;
Expand All @@ -72,8 +73,6 @@
import javax.management.MalformedObjectNameException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;
import java.util.ArrayDeque;
import java.util.Set;

import static org.fao.geonet.api.ApiParams.*;
import static org.fao.geonet.api.records.InspireValidationApi.API_PARAM_INSPIRE_VALIDATION_MODE;
Expand All @@ -87,6 +86,27 @@
@Controller("processValidate")
public class ValidateApi {
private static final int NUMBER_OF_SUBSEQUENT_PROCESS_MBEAN_TO_KEEP = 1;

/**
* XML element name for the active pattern in schematron reports
*/
public static final String EL_ACTIVE_PATTERN = "active-pattern";

/**
* XML element name for fired rules in schematron reports
*/
public static final String EL_FIRED_RULE = "fired-rule";

/**
* XML attribute name for context in schematron reports
*/
public static final String ATT_CONTEXT = "context";

/**
* Default context value when none is provided in schematron reports
*/
public static final String DEFAULT_CONTEXT = "??";

@Autowired
protected ApplicationContext appContext;
@Autowired
Expand Down Expand Up @@ -143,7 +163,7 @@ public void iniMBeansSlidingWindowWithEmptySlot() {
})
@ResponseStatus(HttpStatus.CREATED)
@ResponseBody
public SimpleMetadataProcessingReport validateRecords(
public MetadataValidationProcessingReport validateRecords(
@Parameter(description = API_PARAM_RECORD_UUIDS_OR_SELECTION,
required = false,
example = "")
Expand All @@ -168,8 +188,8 @@ public SimpleMetadataProcessingReport validateRecords(
) throws Exception {
UserSession userSession = ApiUtils.getUserSession(session);

SimpleMetadataProcessingReport report =
new SimpleMetadataProcessingReport();
MetadataValidationProcessingReport report =
new MetadataValidationProcessingReport();
try {
ApplicationContext applicationContext = ApplicationContextHolder.get();
ServiceContext serviceContext = ApiUtils.createServiceContext(request);
Expand Down Expand Up @@ -197,28 +217,25 @@ public SimpleMetadataProcessingReport validateRecords(
} else {
Pair<Element, String> validationPair = validator.doValidate(userSession, record.getDataInfo().getSchemaId(), Integer.toString(record.getId()), xmlSerializer.select(serviceContext, String.valueOf(record.getId())), serviceContext.getLanguage(), false);
boolean isValid = !validationPair.one().getDescendants(ErrorFinder).hasNext();
Element schemaTronReport = validationPair.one();
if (schemaTronReport != null) {
// If the schematron report is not null, we restructure it to have a pattern-rule hierarchy
restructureReportToHavePatternRuleHierarchy(schemaTronReport);
// And add any warnings to the MetadataValidationProcessingReport
report.addAllReportsMatchingRequirement(record, schemaTronReport, SchematronRequirement.REPORT_ONLY);
}
if (isValid) {
report.addMetadataInfos(record, "Is valid");
// If the record is valid, we add it to the valid metadata list
report.addValidMetadata(record);
new RecordValidationTriggeredEvent(record.getId(), ApiUtils.getUserSession(request.getSession()).getUserIdAsInt(), "1").publish(applicationContext);
} else {
report.addMetadataError(record, "(" + record.getUuid() + ") Is invalid");
Element schemaTronReport = validationPair.one();
// If the record is not valid, we add it to the invalid metadata list
if (!report.getInvalidMetadata().containsKey(record.getId())) {
report.addInvalidMetadata(record);
}
if (schemaTronReport != null) {
List<Namespace> theNSs = new ArrayList<Namespace>();
theNSs.add(Namespace.getNamespace("geonet", "http://www.fao.org/geonetwork"));
theNSs.add(Namespace.getNamespace("svrl", "http://purl.oclc.org/dsdl/svrl"));

// Extract all the know errors that exists in the report as List of Text
List<?> schemaTronReportErrors = Xml.selectNodes(schemaTronReport,
"geonet:xsderrors/geonet:error/geonet:message[normalize-space(.) != '']" +
"| geonet:schematronerrors/geonet:report[@geonet:required = '" + SchematronRequirement.REQUIRED + "']/svrl:schematron-output/svrl:failed-assert/svrl:text[normalize-space(.) != '']" +
"| geonet:schematronerrors/geonet:report[@geonet:required = '" + SchematronRequirement.REQUIRED + "']/geonet:schematronVerificationError[normalize-space(.) != '']",
theNSs);

for (Object schemaTronReportError :schemaTronReportErrors) {
// Add normalized string to the report.
report.addMetadataError(record, Xml.selectString((Element) schemaTronReportError, "normalize-space(.)", theNSs));
}
// If the schematron report is not null, add any errors to the MetadataValidationProcessingReport
report.addAllReportsMatchingRequirement(record, schemaTronReport, SchematronRequirement.REQUIRED);
}

new RecordValidationTriggeredEvent(record.getId(), ApiUtils.getUserSession(request.getSession()).getUserIdAsInt(), "0").publish(applicationContext);
Expand Down Expand Up @@ -403,4 +420,74 @@ public boolean matches(Object obj) {
return false;
}
};

/**
* Restructures a schematron validation report to create a hierarchical structure.
*
* <p>Schematron report has an odd structure where pattern elements, fired rules,
* and assertions/reports are all siblings. This method restructures the XML to create
* a more logical hierarchy where patterns contain fired rules, which in turn contain
* assertions and reports.</p>
*
* <p>The input structure looks like:
* <pre>
* <code>
* &lt;svrl:active-pattern ... />
* &lt;svrl:fired-rule ... />
* &lt;svrl:failed-assert ... />
* &lt;svrl:successful-report ... />
* </code>
* </pre>
* </p>
*
* <p>The output structure looks like:
* <pre>
* <code>
* &lt;svrl:active-pattern ... >
* &lt;svrl:fired-rule ... >
* &lt;svrl:failed-assert ... />
* &lt;svrl:successful-report ... />
* &lt;svrl:fired-rule ... >
* &lt;svrl:active-pattern>
* </code>
* </pre>
* </p>
*
* @param errorReport The schematron validation report element to restructure
*/
private static void restructureReportToHavePatternRuleHierarchy(Element errorReport) {
final Iterator patternFilter = errorReport
.getDescendants(new ElementFilter(EL_ACTIVE_PATTERN, Geonet.Namespaces.SVRL));
@SuppressWarnings("unchecked")
List<Element> patterns = Lists.newArrayList(patternFilter);
for (Element pattern : patterns) {
final Element parentElement = pattern.getParentElement();
Element currentRule = null;
@SuppressWarnings("unchecked") final List<Element> children = parentElement.getChildren();

int index = children.indexOf(pattern) + 1;
while (index < children.size() && !children.get(index).getName().equals(EL_ACTIVE_PATTERN)) {
Element next = children.get(index);
if (EL_FIRED_RULE.equals(next.getName())) {
currentRule = next;
next.detach();
pattern.addContent(next);
} else {
if (currentRule == null) {
// odd but could happen I suppose
currentRule = new Element(EL_FIRED_RULE, Geonet.Namespaces.SVRL).setAttribute(ATT_CONTEXT,
DEFAULT_CONTEXT);
pattern.addContent(currentRule);
}

next.detach();
currentRule.addContent(next);

}
}
if (pattern.getChildren().isEmpty()) {
pattern.detach();
}
}
}
}
4 changes: 2 additions & 2 deletions services/src/main/java/org/fao/geonet/api/processing/report/MetadataProcessingReport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public void addMetadataInfos(AbstractMetadata metadata, String message) {
isMetadataApproved(metadata.getId()), message);
}

private boolean isMetadataDraft(int metadataId) {
protected boolean isMetadataDraft(int metadataId) {
boolean metadataDraft = false;
try {
metadataDraft = ApplicationContextHolder.get().getBean(IMetadataUtils.class).isMetadataDraft(metadataId);
Expand All @@ -166,7 +166,7 @@ private boolean isMetadataDraft(int metadataId) {
return metadataDraft;
}

private boolean isMetadataApproved(int metadataId) {
protected boolean isMetadataApproved(int metadataId) {
boolean metadataApproved = false;
try {
metadataApproved = ApplicationContextHolder.get().getBean(IMetadataUtils.class).isMetadataApproved(metadataId);
Expand Down
Loading
Loading