Constructors should only call non-overridable methods in SonarQube tool

Question

In JersyConfiguration class i am getting sonar tool major issues like Remove this call from a constructor to the overridable "register" method. I really don't understand with what code change can resolve this issue. This is my code:

JerseyConfiguration

public class JerseyConfiguration extends ResourceConfig {

    @Bean
    @Primary
    public ObjectMapper objectMapper() {
        ObjectMapper objectMapper = new ObjectMapper();
        objectMapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
        objectMapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false);
        objectMapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES);
        objectMapper.enable(DeserializationFeature.READ_ENUMS_USING_TO_STRING);
        return objectMapper;
    }



    @Autowired
    public JerseyConfiguration() {
        register(TermsResourceImpl.class);
        register(AccessIDResourceImpl.class);
        register(CatalogResourceImpl.class);
        register(SubmitOrderResourceImpl.class);
        register(ValidateAndQuoteResourceImp.class);
        property(ServletProperties.FILTER_FORWARD_ON_404, true);
        register(DynamicLoggingFeature.class);
        register(ContextFilter.class);
        register(ServiceExceptionMapper.class);
        register(JsonParseExceptionMapper.class, 1);
        register(JsonMappingExceptionMapper.class, 1);
        register(LoggingContextJerseyFilter.class);
        register(FeatureToggleFilterBinder.class);
    }

    @Bean
    public Client jerseyClient() {
        return ClientBuilder.newClient(new ClientConfig());
    }
}

ResourceConfig overide methods

 @Override
 public ResourceConfig property(final String name, final Object value) {
     state.property(name, value);
     return this;
 }

 @Override
 public ResourceConfig register(final Class<?> componentClass) {
     invalidateCache();
     state.register(componentClass);
     return this;
 }

SonarQube issue

@SotiriosDelimanolis it may be a false positive, as it's a valid jersey code as https://blog.dejavu.sk/registering-resources-and-providers-in-jersey-2/ — user7294900, Jun 18 '20 at 12:35
Yeah I was about to add another duplicate for suppressing it. The first one properly explained why the warning occurs. — Sotirios Delimanolis, Jun 18 '20 at 12:39
I have similar code which SonarLint which doesn't find any code smell, How exactly do you check your code? — user7294900, Jun 18 '20 at 12:49
@user7294900 after deployed my code in environment we check sonar issues there its showing this issue — AbhiRam, Jun 18 '20 at 12:51
I think you should raise it in SonarQube site as false positive or at least discuss it there — user7294900, Jun 18 '20 at 13:22

score 0 · Answer 1 · answered Jun 19 '20 at 15:05

It is not a false positive, because the rule works correctly. You are calling a method in the constructor which could be changed in sub classes. This structure could cause problems. Example:

public class BrokenCode extends JerseyConfiguration {

    @Override
    public ResourceConfig register(final Class<?> componentClass) {
        // stop invaliding cache, to break the application
        // invalidateCache();

        state.register(componentClass);
        return this;
    }
}

To solve the issue the method must be marked as final. I don't know Jersey so you have the following option:

1. if making methods as `final` is impossible

In some cases adding final could break the application, because for example dynamic proxies are created by the used framework. In such cases you can only mark the issue as Won't Fix (it informs other developers that the mentioned structure doesn't match the rule, but you are fine with it)

2. if making methods as `final` is possible

There are two options, depending on which classes you can edit:

if you can edit the ResourceConfig class, then change:

@Override
public ResourceConfig register(final Class<?> componentClass) {
    invalidateCache();
    state.register(componentClass);
    return this;
}

to

@Override
public final ResourceConfig register(final Class<?> componentClass) {
    invalidateCache();
    state.register(componentClass);
    return this;
}

If invalidateCache is not private then you have to mark it as final too.

if you cannot edit the ResourceConfig class, then you can add to the JerseyConfiguration class the following method:

@Override
public final ResourceConfig register(final Class<?> componentClass) {
    super.register(componentClass)
}

Of course again if invalidateCache is not private then you have to add it too:

@Override
public final void invalidateCache() {
    super.invalidateCache()
}

The chosen "fixing" strategy should be based also on:

used framework (Jersey) guidelines
who can extend JerseyConfiguration configuration
etc.

I think it is totally fine to close it as Won't Fix also when the application:

uses structures in the used framework documentation
is developed/maintained only by your team (nobody else creates manually new instances of the JerseyConfiguration class)

score 0 · Answer 2 · answered Feb 22 '22 at 18:08

Add an explicit 'this' to the register() calls.

Example:

public class JerseyConfiguration extends ResourceConfig {

  @Autowired
  public JerseyConfiguration() {
    this.register(TermsResourceImpl.class);
    this.register(AccessIDResourceImpl.class);

    ...

  }
}

Constructors should only call non-overridable methods in SonarQube tool

JerseyConfiguration

ResourceConfig overide methods

SonarQube issue

2 Answers2

1. if making methods as final is impossible

2. if making methods as final is possible

1. if making methods as `final` is impossible

2. if making methods as `final` is possible