Merge pull request #1773 from hapifhir/2024-10-gg-sql-validation

refactor sql view validation to fix NPE
This commit is contained in:
Grahame Grieve 2024-10-08 15:27:28 +08:00 committed by GitHub
commit 8fdad60b84
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 38 additions and 29 deletions

View File

@ -3,16 +3,17 @@ package org.hl7.fhir.r5.utils.sql;
import java.util.List;
import org.hl7.fhir.r5.model.Base;
import org.hl7.fhir.r5.utils.sql.Validator.TrueFalseOrUnknown;
public interface Storage {
boolean supportsArrays();
boolean supportsComplexTypes();
TrueFalseOrUnknown supportsArrays();
TrueFalseOrUnknown supportsComplexTypes();
Store createStore(String name, List<Column> columns);
void addRow(Store store, List<Cell> cells);
void finish(Store store);
boolean needsName();
TrueFalseOrUnknown needsName();
String getKeyForSourceResource(Base res);
String getKeyForTargetResource(Base res);
}

View File

@ -4,6 +4,7 @@ import java.util.List;
import org.hl7.fhir.exceptions.FHIRException;
import org.hl7.fhir.r5.model.Base;
import org.hl7.fhir.r5.utils.sql.Validator.TrueFalseOrUnknown;
import org.hl7.fhir.utilities.json.model.JsonArray;
import org.hl7.fhir.utilities.json.model.JsonBoolean;
import org.hl7.fhir.utilities.json.model.JsonElement;
@ -18,8 +19,8 @@ public class StorageJson implements Storage {
private JsonArray rows;
@Override
public boolean supportsArrays() {
return true;
public TrueFalseOrUnknown supportsArrays() {
return TrueFalseOrUnknown.TRUE;
}
@Override
@ -77,13 +78,13 @@ public class StorageJson implements Storage {
}
@Override
public boolean supportsComplexTypes() {
return true;
public TrueFalseOrUnknown supportsComplexTypes() {
return TrueFalseOrUnknown.TRUE;
}
@Override
public boolean needsName() {
return false;
public TrueFalseOrUnknown needsName() {
return TrueFalseOrUnknown.FALSE;
}
@Override

View File

@ -7,6 +7,7 @@ import java.util.List;
import org.hl7.fhir.exceptions.FHIRException;
import org.hl7.fhir.r5.model.Base;
import org.hl7.fhir.r5.utils.sql.Validator.TrueFalseOrUnknown;
import org.hl7.fhir.utilities.CommaSeparatedStringBuilder;
public class StorageSqlite3 implements Storage {
@ -119,18 +120,18 @@ public class StorageSqlite3 implements Storage {
}
@Override
public boolean supportsArrays() {
return false;
public TrueFalseOrUnknown supportsArrays() {
return TrueFalseOrUnknown.FALSE;
}
@Override
public boolean supportsComplexTypes() {
return false;
public TrueFalseOrUnknown supportsComplexTypes() {
return TrueFalseOrUnknown.FALSE;
}
@Override
public boolean needsName() {
return true;
public TrueFalseOrUnknown needsName() {
return TrueFalseOrUnknown.TRUE;
}
@Override

View File

@ -32,6 +32,7 @@ import org.hl7.fhir.r5.model.UnsignedIntType;
import org.hl7.fhir.r5.model.UriType;
import org.hl7.fhir.r5.model.UrlType;
import org.hl7.fhir.r5.model.UuidType;
import org.hl7.fhir.r5.utils.sql.Validator.TrueFalseOrUnknown;
import org.hl7.fhir.r5.fhirpath.ExpressionNode.CollectionStatus;
import org.hl7.fhir.r5.fhirpath.FHIRPathEngine.IssueMessage;
import org.hl7.fhir.utilities.Utilities;
@ -49,25 +50,29 @@ import org.hl7.fhir.utilities.validation.ValidationMessage.Source;
public class Validator {
public enum TrueFalseOrUnknown {
TRUE, FALSE, UNKNOWN
}
private IWorkerContext context;
private FHIRPathEngine fpe;
private List<String> prohibitedNames = new ArrayList<String>();
private List<ValidationMessage> issues = new ArrayList<ValidationMessage>();
private Boolean arrays;
private Boolean complexTypes;
private Boolean needsName;
private TrueFalseOrUnknown supportsArrays;
private TrueFalseOrUnknown supportsComplexTypes;
private TrueFalseOrUnknown supportsNeedsName;
private String resourceName;
private String name;
public Validator(IWorkerContext context, FHIRPathEngine fpe, List<String> prohibitedNames, Boolean arrays, Boolean complexTypes, Boolean needsName) {
public Validator(IWorkerContext context, FHIRPathEngine fpe, List<String> prohibitedNames, TrueFalseOrUnknown supportsArrays, TrueFalseOrUnknown supportsComplexTypes, TrueFalseOrUnknown supportsNeedsName) {
super();
this.context = context;
this.fpe = fpe;
this.prohibitedNames = prohibitedNames;
this.arrays = arrays;
this.complexTypes = complexTypes;
this.needsName = needsName;
this.supportsArrays = supportsArrays;
this.supportsComplexTypes = supportsComplexTypes;
this.supportsNeedsName = supportsNeedsName;
}
public String getResourceName() {
@ -80,9 +85,9 @@ public class Validator {
JsonElement nameJ = viewDefinition.get("name");
if (nameJ == null) {
if (needsName == null) {
if (supportsNeedsName == null) {
hint(path, viewDefinition, "No name provided. A name is required in many contexts where a ViewDefinition is used");
} else if (needsName) {
} else if (supportsNeedsName == TrueFalseOrUnknown.TRUE) {
error(path, viewDefinition, "No name provided", IssueType.REQUIRED);
}
} else if (!(nameJ instanceof JsonString)) {
@ -334,9 +339,9 @@ public class Validator {
hint(path, column, "collection is true, but the path statement(s) can only return single values for the column '"+columnName+"'");
}
} else {
if (arrays == null) {
if (supportsArrays == TrueFalseOrUnknown.UNKNOWN) {
warning(path, expression, "The column '"+columnName+"' appears to be a collection based on it's path. Collections are not supported in all execution contexts");
} else if (!arrays) {
} else if (supportsArrays == TrueFalseOrUnknown.FALSE) {
warning(path, expression, "The column '"+columnName+"' appears to be a collection based on it's path, but this is not allowed in the current execution context");
}
if (td.getCollectionStatus() != CollectionStatus.SINGLETON) {
@ -373,9 +378,9 @@ public class Validator {
String type = types.iterator().next();
boolean ok = false;
if (!isSimpleType(type) && !"null".equals(type)) {
if (complexTypes) {
if (supportsComplexTypes == TrueFalseOrUnknown.UNKNOWN) {
warning(path, expression, "Column is a complex type. This is not supported in some Runners");
} else if (!complexTypes) {
} else if (supportsComplexTypes == TrueFalseOrUnknown.FALSE) {
error(path, expression, "Column is a complex type but this is not allowed in this context", IssueType.BUSINESSRULE);
} else {
ok = true;

View File

@ -157,6 +157,7 @@ import org.hl7.fhir.r5.utils.ToolingExtensions;
import org.hl7.fhir.r5.utils.XVerExtensionManager;
import org.hl7.fhir.r5.utils.XVerExtensionManager.XVerExtensionStatus;
import org.hl7.fhir.r5.utils.sql.Validator;
import org.hl7.fhir.r5.utils.sql.Validator.TrueFalseOrUnknown;
import org.hl7.fhir.r5.utils.validation.BundleValidationRule;
import org.hl7.fhir.r5.utils.validation.IMessagingServices;
import org.hl7.fhir.r5.utils.validation.IResourceValidator;
@ -5936,7 +5937,7 @@ public class InstanceValidator extends BaseValidator implements IResourceValidat
} else if ("http://hl7.org/fhir/uv/sql-on-fhir/StructureDefinition/ViewDefinition".equals(element.getProperty().getStructure().getUrl())) {
if (element.getNativeObject() != null && element.getNativeObject() instanceof JsonObject) {
JsonObject json = (JsonObject) element.getNativeObject();
Validator sqlv = new Validator(context, fpe, new ArrayList<>(), null, null, null);
Validator sqlv = new Validator(context, fpe, new ArrayList<>(), TrueFalseOrUnknown.UNKNOWN, TrueFalseOrUnknown.UNKNOWN, TrueFalseOrUnknown.UNKNOWN);
sqlv.checkViewDefinition(stack.getLiteralPath(), json);
errors.addAll(sqlv.getIssues());
ok = sqlv.isOk() && ok;