phenopackets / phenopacket-validator

Library and tools to help validate phenopackets
BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

PhenoPacket Validator signature #1

Closed ielis closed 3 years ago

ielis commented 5 years ago

Hi @julesjacobsen ,

is the signature of the Validator#validate method a final version or just a placeholder until we figure out how should it actually look like?

I think that the Validators should be able to give the user some feedback/response if the validation fails. So changing void into boolean would not be sufficient in my opinion.

We could get the feedback by adding a second method to the interface, such as

String getValidationMessage()

at the cost of removing @FunctionalInteface and enforcing validators to maintain a state.

An alternative would be to throw an Exception, if the validation fails:

public void validate(PhenoPacket phenoPacket) throws PhenoPacketValidationException;

I would favor this approach. What do you think? Cheers, Daniel

julesjacobsen commented 5 years ago

@ielis @pnrobinson no this isn't final - just a placeholder.

I was thinking either:

public interface Validator {

    public ValidationResult validate(PhenoPacket phenoPacket);
}

or

public interface Validator<T extends Message> {

    public ValidationResult validate(T message);
}

the advantage of using the generic is that we can then have classes to check any message type from the phenopacket e.g.

public class OntologyClassValidator implements Validator<OntologyClass> {

    @Override
    public ValidationResult validate(OntologyClass ontologyClass) {
        return ontologyClass.getId().isEmpty() ? ValidationResult.fail("OntologyClass id cannot be empty") : ValidationResult.pass();
    }
}

or a method

private Validator<PhenoPacket> checkExistenceOfMetadata() {
        return phenoPacket -> {
            MetaData md = phenoPacket.getMetaData();
            if (md.equals(MetaData.getDefaultInstance())) {
                return ValidationResult.fail("Metadata is empty");
            }
            return ValidationResult.pass();
        };
    }
julesjacobsen commented 5 years ago

See 615d14eaa4b3462578d7ac0b8808d3dc1f1c307c

julesjacobsen commented 5 years ago

Validator should also be able to return a List

pnrobinson commented 3 years ago

@ielis this issue has gotten stale. I have just finished a prototype app with JSON schema that is mainly working, it would be great to do a code review of that, but I will close this issue for now.