mapstruct icon indicating copy to clipboard operation
mapstruct copied to clipboard

AnnotatedWith not propagating to DecoratedWith generated implementation

Open rgcv opened this issue 1 year ago • 4 comments

Expected behavior

When trying to use @AnnotatedWith to annotate a mapper implementation class with Spring's @ConditionalOnProperty on a @DecoratedWith mapper

@Mapper(componentModel = "spring")
@AnnotatedWith(value = ConditionalOnProperty.class, elements = {
    @Element(name = "name", strings = "Property"),
    @Element(name = "havingValue", strings = "")
})
@DecoratedWith(Decorator.class)
interface MyMapper { }

Mapstruct correctly annotates the Impl_ class

@Generated(/* ... */)
@Component
@ConditionalOnProperty(
    name = "Property",
    havingValue = ""
)
@Qualifier("delegate")
public class MapperImpl_ { }

I was also expecting that the Decorator's base component class would be annotated

@Generated(/* ... */)
@Component
@ConditionalOnProperty(
    name = "Property",
    havingValue = ""
)
@Primary
public class MapperImpl extends Decorator { }

Actual behavior

Base implementation component is not annotated

@Generated(/* ... */)
@Component
@Primary
public class MapperImpl extends Decorator { }

This leads to a runtime error where, if I try to inject a mapper instance, it will fail providing a bean for MapperImpl since the delegate is conditional.

2024-08-09T11:26:29.123+01:00  WARN 44711 --- [mcve] [           main] s.c.a.AnnotationConfigApplicationContext : Exception encountered during context initialization - cancelling refresh attempt: org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'mapstructAwApplicat
ion': Unsatisfied dependency expressed through field 'mapper': No qualifying bean of type 'com.example.mapstructaw.MyMapper' available: expected at least 1 bean which qualifies as autowire candidate. Dependency annotations: {@org.springframework.beans.factory.annotation.Autowired(required=true)}
2024-08-09T11:26:29.126+01:00  INFO 44711 --- [mcve] [           main] .s.b.a.l.ConditionEvaluationReportLogger :

Error starting ApplicationContext. To display the condition evaluation report re-run your application with 'debug' enabled.
2024-08-09T11:26:29.130+01:00 ERROR 44711 --- [mcve] [           main] o.s.b.d.LoggingFailureAnalysisReporter   :

***************************
APPLICATION FAILED TO START
***************************

Description:

Field mapper in com.example.mapstructaw.MapstructAwApplication required a bean of type 'com.example.mcve.MyMapper' that could not be found.

The injection point has the following annotations:
        - @org.springframework.beans.factory.annotation.Autowired(required=true)


Action:

Consider defining a bean of type 'com.example.mapstructaw.MyMapper' in your configuration.

Steps to reproduce the problem

This should be enough to reproduce the issue in a plain spring boot starter project

@Mapper(componentModel = "spring")
@AnnotatedWith(value = ConditionalOnProperty.class, elements = {
        @Element(name = "name", value = "Property"),
        @Element(name = "havingValue", value = ""),
})
@DecoratedWith(Decorator.class)
interface MyMapper { }
abstract class Decorator implements MyMapper { }
@SpringBootApplication
public class McveApplication {
    @Autowired MyMapper mapper;

    public static void main(String[] args) {
        SpringApplication.run(McveApplication.class, args);
    }
}

MapStruct Version

1.6.0.RC1

rgcv avatar Aug 09 '24 10:08 rgcv

Thanks for the detailed example @rgcv.

This is a bit tricker to do. Especially if you take into consideration that @AnnotateWith can be used to provide a name for the @Component.

e.g. if we take

@AnnotateWith( value = Component.class, elements = @AnnotateWith.Element( strings = "MyCustomMapperName" ) )

If we just propagate the @AnnotateWith to both the Impl_ and the one extending the decorator then we would have a problem as we would have 2 beans with the same name.

Perhaps a solution for this would be to allow using @AnnotateWith on your decorator class and then we propagate that to the implementation.

It does mean that you'll need to do

@AnnotatedWith(value = ConditionalOnProperty.class, elements = {
        @Element(name = "name", value = "Property"),
        @Element(name = "havingValue", value = ""),
})

twice, both on the MyMapper and also the Decorator. However, in my opinion that is OK and should intuitive.

Does that make sense to you @rgcv?

filiphr avatar Aug 11 '24 17:08 filiphr

Hey Filip,

This is a bit tricker to do. Especially if you take into consideration that @AnnotateWith can be used to provide a name for the @Component.

This is probably a non-issue since @Component is a non-repeatable annotation and (unless there is some kind of exceptional rule) it seemed like the @AnnotatedWith only added annotations, it did not squash or merge any, meaning it would generate invalid code to start with, leading to something like

@Generated(/* ... */)
@Component("MyCustomMapperName")
@Component
@Qualifier("delegate")
public class MyMapperImpl_ implements MyMapper { /* ... */ }

Perhaps a solution for this would be to allow using @AnnotateWith on your decorator class and then we propagate that to the implementation. Does that make sense to you @rgcv?

It does make sense, and it's clearly redundant, or at least seems so at a glance. However, I also like how it's explicit, allowing for some potentially more fine-grain control over which annotations are applied to which implementation classes. Ideally, if there are no major impediments with propagating the @AnnotatedWith spec to the decorator, I'd definitely prefer that. It sounds reasonable since it's a relatively closed hierarchy. But I'll take what I can get :)

On another note, I guess the redundancy is nothing a meta-annotated annotation couldn't handle. So perhaps annotating both the base interface and the decorator with

@ConditionalOnProperty(/* ... */)
public @interface ConditionalOnMyProperty { }

could mitigate that. Haven't tried, but don't see why it wouldn't work.

rgcv avatar Aug 11 '24 21:08 rgcv

This is probably a non-issue since @Component is a non-repeatable annotation and (unless there is some kind of exceptional rule) it seemed like the @AnnotatedWith only added annotations, it did not squash or merge any, meaning it would generate invalid code to start with, leading to something like

It is an issue, because we have a special handling when using @Component or another meta annotated annotation that has @Component on it. If you do that, then we are not going to add the @Component annotation.

I'll change the title to reflect that we want to add support for propagating @AnnotateWith on the decorator to the generated decorator class.

filiphr avatar Aug 12 '24 06:08 filiphr

It is an issue, because we have a special handling when using @Component or another meta annotated annotation that has @Component on it. If you do that, then we are not going to add the @Component annotation.

Caught me there, should've tested a bit more thoroughly, would probably encounter that scenario myself, sorry about that. And thanks, good to know.

I'll change the title to reflect that we want to add support for propagating @AnnotateWith on the decorator to the generated decorator class.

Sounds good to me :+1:

rgcv avatar Aug 12 '24 08:08 rgcv