Load module parameters from @Parameter annotation

[blcham]

Many modules use @Parameter annotations but extends AbstractModule instead of AnnotatedAbstractModule. Thus they process parameters manually using loadConfiguration method.

Goal of this issue is to remove manual loading of modules wherever possible.

[palagdan]

@blcham

if some types aren't annotated with the @Parameter annotation because they need to be configured manually, should I just leave them as they are? For example repositoryManager is not annotated and is inconfigured by value of other attributes:

@Override
    public void loadConfiguration() {
        rdf4jServerURL = getEffectiveValue(P_RDF4J_SERVER_URL).asLiteral().getString();
        rdf4jRepositoryName = getEffectiveValue(P_RDF4J_REPOSITORY_NAME).asLiteral().getString();
        try {
            rdf4jIgnoreIfExists = (Objects.equals(getEffectiveValue(P_RDF4J_IGNORE_IF_EXISTS).asLiteral().getString(), "true"));
        }
        catch (NullPointerException e){
            rdf4jIgnoreIfExists = false;
        }
        repositoryManager = new RemoteRepositoryManager(rdf4jServerURL);
    }

[blcham]

Yes, leave it as they are, but if there is some parameter occurring multiple times, please mention it here so we can discuss how to extend @Parameter annotation to support it. Make an ordered list of parameters that are the biggest blockers to remove the loadConfiguration method.

[palagdan]

@blcham What do you mean by a parameter occurring multiple times? Are you referring to a situation where the same annotation @parameter is used multiple times on one field?

[palagdan]

@blcham I have a question, if I remove some configuration and add parameter annotation instead, how can I test these changes?

[blcham]

I think it is done in ApplyConstructModuleTest.executeSimple. I believe the line that I addressed should fill-in values of the object from @Parameter annotation. The actual method that is called is AnnotatedAbstractModule.loadConfiguration.

[blcham]

@palagdan

[palagdan]

@blcham

I noticed that some variables are configured by overloading the getPropertyValue method, which accepts a Property and a literal default value to use if there is no object value for the current property. For example, the isReplace variable is configured as follows:

 @Override
    public void loadConfiguration() {
        value = getEffectiveValue(SML.value);
        outputVariable = getStringPropertyValue(SM.outputVariable);
        isReplace = this.getPropertyValue(SML.replace, false);
    }

but loadConfiguration in AnnotatedAbstractModule.java calls getEffectiveValue which does not accept a default value. This is why I cannot remove the manual configuration:

RDFNode node = this.getEffectiveValue(ResourceFactory.createProperty(p.urlPrefix()+p.name()));

That is why I suggest to extend @Parameter annotation by one more attribute 'defaultValue'. Something like this:

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.FIELD)
public @interface Parameter {
    String urlPrefix() default KBSS_MODULE.uri;
    String name();
    String comment();
    Object defaultValue();
}

With this change, we could assign a default value to an attribute if this.getEffectiveValue returns null and node is empty.

[blcham]

@palagdan I agree to add defaultValue() parameter there.

[palagdan]

@blcham

I attempted to add handlers to the @Parameter annotation and use them for configuring attributes. However, I discovered that it’s more complex than simply adding some if-else statements to the loadConfiguration method in the AnnotatedAbstractModule class. Now, this method can call the appropriate getPropertyMethod in the AbstractModule class, whereas it previously handled only basic types (like int, boolean, string etc.) and couldn't set RDFNode attributes.

The main issue is that some attributes, such as path variables, cannot be configured using the simple getPropertyValue() method. For example:

 sourceFilePath = Optional.ofNullable(getEffectiveValue(SML.sourceFilePath))
                        .filter(RDFNode::isLiteral)
                        .map(RDFNode::asLiteral)
                        .map(Object::toString)
                        .map(s -> Paths.get(s))
                        .orElse(null);
selectQuery = getPropertyValue(SML.selectQuery).asResource().as(Select.class);

I have a solution, but I’m not sure if it will meet your expectations. The idea is to create a separate method loadAnnotationConfiguration() for configuring attributes with annotations, and keep loadConfiguration() for classes that need to implement a method for attributes that cannot be configured via annotations. Because it much more complex to add another handler for attributes like for this sourceFilePath or selectQuery. It solves the problem of setting everything manually and makes it much easier to configure it manually in case if it's not possible to configure with annotation, as you don’t need to create a new handler for your use case

Additionally, it is not possible to set a default value in the annotation with:

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.FIELD)
public @interface Parameter {
    String urlPrefix() default KBSS_MODULE.uri;
    String name();
    String comment();
    Object defaultValue();
}

This is because Object defaultValue(); requires a specific type at compile time.

[blcham]

@palagdan Changing status to "in progress"