Skip to content

unresolvable circular references updating from 5.0.3 #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
efenderbosch opened this issue Jan 17, 2020 · 15 comments
Closed

unresolvable circular references updating from 5.0.3 #8

efenderbosch opened this issue Jan 17, 2020 · 15 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@efenderbosch
Copy link

efenderbosch commented Jan 17, 2020

It looks like there are some new bean factories trying to inject instances of AmazonDynamoDB, DynamoDBMapperConfig, etc.

Is there a way to turn them off and simply use the beans we already have configured?

Caused by: org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'dynamoDB-DynamoDBMapper': Unsatisfied dependency expressed through constructor parameter 1; nested exception is org.springframework.beans.factory.BeanCurrentlyInCreationException: Error creating bean with name 'dynamoDBMapperConfig': Requested bean is currently in creation: Is there an unresolvable circular reference?

Looks like this popped up in v5.1.0, so before your fork.

@efenderbosch
Copy link
Author

I was able to hack around this.

https://gist.github.com/efenderbosch/a59b12d2101a1e742e3ee864a39cd288

Pretty ugly. Any suggestions on how to do this without such hackery?

@boostchicken
Copy link
Owner

boostchicken commented Jan 21, 2020

Hey @efenderbosch , I have not had to do this. Using spring boot My config looks like this.

Do you mind sharing yours? I would note that I am wiring in my stuff on the annotation at the top. You can ignore the Region stuff if you have no reason to override it

@EnableDynamoDBRepositories(basePackages = "blah.repository.dynamodb", amazonDynamoDBRef = "amazonDynamoDB", dynamoDBMapperConfigRef = "dynamoDBMapperConfig")
public class Config{


    @Bean
    public AmazonDynamoDB amazonDynamoDB()
    {
        Region region = Regions.getCurrentRegion();
        if(region == null) {
            region = RegionUtils.getRegion(this.region);
        }
        return AmazonDynamoDBClientBuilder.standard().withRegion(region.getName()).build();
    }

    @Bean
    public DynamoDBMapperConfig dynamoDBMapperConfig() {
        DynamoDBMapperConfig.Builder builder = new DynamoDBMapperConfig.Builder();
        builder.setPaginationLoadingStrategy(DynamoDBMapperConfig.PaginationLoadingStrategy.LAZY_LOADING);
        builder.setTypeConverterFactory(DynamoDBTypeConverterFactory.standard());
        return builder.build();
    }
}

@boostchicken boostchicken self-assigned this Jan 21, 2020
@efenderbosch
Copy link
Author

We have two separate Configuration classes:

@Configuration
@ConditionalOnDynamoDBEnabled
@EnableConfigurationProperties(DynamoDBProperties.class)
public class DynamoDBConfiguration {

    private static final Logger LOG = LoggerFactory.getLogger(DynamoDBConfiguration.class);

    @Bean
    public AmazonDynamoDB amazonDynamoDB(DynamoDBProperties props, AWSCredentialsProvider credentialsProvider) {
        AmazonDynamoDBClientBuilder builder = AmazonDynamoDBClientBuilder.standard().
                withCredentials(credentialsProvider);

        String endpoint = props.getEndpoint();
        if (endpoint != null) {
            EndpointConfiguration endpointConfiguration = new EndpointConfiguration(endpoint, props.getRegion());
            builder.setEndpointConfiguration(endpointConfiguration);
        }

        DynamoDBProperties.RetryPolicyProperties retryPolicyProperties = props.getRetryPolicy();
        if (retryPolicyProperties != null) {
            BackoffStrategy backoffStrategy = new SDKDefaultBackoffStrategy(
                    retryPolicyProperties.getBaseDelay(),
                    retryPolicyProperties.getThrottledBaseDelay(),
                    retryPolicyProperties.getMaxBackoff());

            RetryPolicy retryPolicy = new RetryPolicy(
                    retryPolicyProperties.getRetryCondition(),
                    backoffStrategy,
                    retryPolicyProperties.getMaxErrorRetry(),
                    retryPolicyProperties.isHonorMaxErrorRetryInClientConfig());

            ClientConfiguration clientConfiguration = new ClientConfiguration();
            clientConfiguration.setRetryPolicy(retryPolicy);
            builder.withClientConfiguration(clientConfiguration);
        }

        return builder.build();
    }

    @Bean
    public DynamoDBMapperConfig dynamoDBMapperConfig(DynamoDBProperties props, Environment env) {
        Builder builder = new Builder().withTypeConverterFactory(DynamoDBTypeConverterFactory.standard());

        String profile = "test";

        String[] profiles = env.getActiveProfiles();
        if (profiles.length == 1) {
            profile = profiles[0];
        }

        String tableNamePrefix = props.getTableNameOverridePrefix();
        if (tableNamePrefix == null) {
            tableNamePrefix = profile;
        }

        if (!"prod".equals(tableNamePrefix)) {
            LOG.info("Using tableNamePrefix: '{}'", tableNamePrefix);
            builder.withTableNameOverride(TableNameOverride.withTableNamePrefix(tableNamePrefix + '_'));
        }

        DynamoDBMapperConfig.PaginationLoadingStrategy paginationLoadingStrategy = props.getPaginationLoadingStrategy();
        if (paginationLoadingStrategy != null) {
            builder.withPaginationLoadingStrategy(paginationLoadingStrategy);
        }

        return builder.build();
    }

    @Bean
    public IDynamoDBMapper dynamoDBMapper(AmazonDynamoDB dynamoDB,
                                          DynamoDBMapperConfig config,
                                          Optional<AttributeTransformer> attributeTransformer,
                                          AWSCredentialsProvider awsCredentialsProvider,
                                          Optional<AmazonS3> s3) {
        DynamoDBMapper dynamoDBMapper = new DynamoDBMapper(dynamoDB,
                config,
                attributeTransformer.orElse(null),
                awsCredentialsProvider);
        s3.ifPresent(value -> dynamoDBMapper.getS3ClientCache().useClient(value));

        return dynamoDBMapper;
    }

    @Bean
    @Primary
    @Profile("localstack")
    public IDynamoDBMapper localStackDynamoDBMapper(AmazonDynamoDB dynamoDB,
                                                    DynamoDBMapperConfig config,
                                                    Optional<AttributeTransformer> attributeTransformer,
                                                    AWSCredentialsProvider awsCredentialsProvider,
                                                    Optional<AmazonS3> s3) {
        DynamoDBMapper dynamoDBMapper = new DynamoDBMapper(dynamoDB,
                config,
                attributeTransformer.orElse(null),
                awsCredentialsProvider);
        s3.ifPresent(value -> dynamoDBMapper.getS3ClientCache().useClient(value));

        return dynamoDBMapper;
    }
}
@Configuration
@ConditionalOnDynamoDBEnabled
public class DynamoDBSpringDataConfiguration {

    @Bean
    @Primary
    @Order(HIGHEST_PRECEDENCE)
    @SuppressFBWarnings(
            value = "BC_UNCONFIRMED_CAST",
            justification = "https://github.com/aws/aws-sdk-java/issues/1802")
    public DynamoDBOperations dynamoDBOperations(AmazonDynamoDB dynamoDB,
                                                 DynamoDBMapperConfig dbMapperConfig,
                                                 IDynamoDBMapper dbMapper) {
        return new DynamoDBTemplate(dynamoDB, (DynamoDBMapper) dbMapper, dbMapperConfig);
    }
}

@boostchicken
Copy link
Owner

boostchicken commented Jan 25, 2020

Hey dude, taking some time to dig through this, doing things a bit differnt than usual for sure. The issue in DynamoDBRepositoryConfigExtension

public void registerBeansForRoot(BeanDefinitionRegistry registry,
RepositoryConfigurationSource configurationSource) {
super.registerBeansForRoot(registry, configurationSource);
// Store for later to be used by #postProcess, too
this.registry = registry;
this.dynamoDBMapperConfigName = getBeanNameWithModulePrefix("DynamoDBMapperConfig");
Optional dynamoDBMapperConfigRef = configurationSource.getAttribute("dynamoDBMapperConfigRef");
if (!dynamoDBMapperConfigRef.isPresent()) {
BeanDefinitionBuilder dynamoDBMapperConfigBuiilder = BeanDefinitionBuilder
.genericBeanDefinition(DynamoDBMapperConfigFactory.class);
registry.registerBeanDefinition(this.dynamoDBMapperConfigName,
dynamoDBMapperConfigBuiilder.getBeanDefinition());
}
this.dynamoDBMapperName = getBeanNameWithModulePrefix("DynamoDBMapper");
BeanDefinitionBuilder dynamoDBMapperBuilder = BeanDefinitionBuilder
.genericBeanDefinition(DynamoDBMapperFactory.class);
registry.registerBeanDefinition(this.dynamoDBMapperName, dynamoDBMapperBuilder.getBeanDefinition());
}

I believe you need to pass in the Ref via the @enable annotation like I have above.

@EnableDynamoDBRepositories(basePackages = "blah.repository.dynamodb", amazonDynamoDBRef = "amazonDynamoDB", dynamoDBMapperConfigRef = "dynamoDBMapperConfig")

@boostchicken
Copy link
Owner

On second thought it looks like the code changed so it always makes a mapper bean instead of taking one from your config it always makes its own regardless. Let me think of an elegant way to get around this

@boostchicken
Copy link
Owner

One way to get around it might be to post-process the DynamoDBMapper bean instead of creating it to make your mods.

@boostchicken
Copy link
Owner

boostchicken commented Jan 25, 2020

@efenderbosch
#10

Added a new property for you to specify a mapper ref.

@EnableDynamoDBRepositories(basePackages = "blah.repository.dynamodb", amazonDynamoDBRef = "amazonDynamoDB", dynamoDBMapperConfigRef = "dynamoDBMapperConfig", dynamoDBMapperRef= "blahhhhh" )

Give it a shot and let me know if it works, published in 5.2.2-SNAPSHOT on ossrh

@boostchicken boostchicken added the bug Something isn't working label Jan 25, 2020
@boostchicken boostchicken added this to the 5.2.2 milestone Jan 25, 2020
@frommeyerc
Copy link

Hi,
we just stumbled upon the same (a similar) issue. I tracked that beast down and here it is:

The DynamoDBMapperFactory is not very nice to it's BeanFactory. In order for the BeanFactory to retrieve the type of the object created by the FactoryBean it already requires to get it's dependencies injected via Constructor injection.

This results in an issue when e.g. while building a custom AmazonDynamoDB object a dependency bean is searched and not found early enough so that the BeanFactory would like to know what it could get from the DynamoDBMapperFactory.

The documentation of the FactoryBean interface already suggests a solution to this issue. I've implemented that and got rid of the issue. Here is my new class:

package org.socialsignin.spring.data.dynamodb.repository.config;

import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.BeanFactoryAware;
import org.springframework.beans.factory.FactoryBean;

import com.amazonaws.services.dynamodbv2.AmazonDynamoDB;
import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper;
import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperConfig;

public class DynamoDBMapperFactory implements FactoryBean<DynamoDBMapper>, BeanFactoryAware {

    private BeanFactory beanFactory;
    
	@Override
	public DynamoDBMapper getObject() throws Exception {
	    AmazonDynamoDB amazonDynamoDB = beanFactory.getBean(AmazonDynamoDB.class);
	    DynamoDBMapperConfig dynamoDBMapperConfig = beanFactory.getBean(DynamoDBMapperConfig.class);
		return new DynamoDBMapper(amazonDynamoDB, dynamoDBMapperConfig);
	}

	@Override
	public Class<?> getObjectType() {
		return DynamoDBMapper.class;
	}

    @Override
    public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
        this.beanFactory = beanFactory;
    }

}

@boostchicken
Copy link
Owner

Wanna send a PR @frommeyerc

I think your solution is ideal but some black magic voodoo for custom repos uses some deprecated CDI interface

@frommeyerc
Copy link

I'll do, just need to figure out how that works with github.

frommeyerc pushed a commit to frommeyerc/spring-data-dynamodb that referenced this issue Jan 28, 2020
@frommeyerc
Copy link

Is there a chance for a Release containing the fix anytime soon?

@boostchicken
Copy link
Owner

@frommeyerc I'll get a snapshot out tonight. Do you need a full blown release? I could ship out 5.2.2 this weekend.

@frommeyerc
Copy link

@boostchicken I already tested with a locally build version. In order to use the lib in Production I'll need a released Version.

boostchicken added a commit that referenced this issue Feb 25, 2020
* Read Consistency Setting

* #8

Custom Mapper Spec

* #8

Custom Mapper CDI fix

* Fix symlink to CONTRIBUTING.md (#13)

* #8 Don't use autowired or constructor injection with FactoryBeans (#12)

* [maven-release-plugin] prepare release v5.2.2

* [maven-release-plugin] prepare for next development iteration

Co-authored-by: Christian Frommeyer <[email protected]>
@boostchicken
Copy link
Owner

boostchicken commented Feb 25, 2020

@efenderbosch @frommeyerc this os released and should be on central soon

boostchicken added a commit that referenced this issue Feb 25, 2020
* Read Consistency Setting

* #8

Custom Mapper Spec

* #8

Custom Mapper CDI fix

* Fix symlink to CONTRIBUTING.md (#13)

* #8 Don't use autowired or constructor injection with FactoryBeans (#12)

* [maven-release-plugin] prepare release v5.2.2

* [maven-release-plugin] prepare for next development iteration

* 5.2.2 docs fix

* Hibernate Validator CVE fix
GHSA-m8p2-495h-ccmh

* Documentation updates

* Create FUNDING.yml

* Fixed a Type (#17)

Co-authored-by: thedevluffy <[email protected]>

* Documentation updates

* 5.2.3 prep

* [maven-release-plugin] prepare release v5.2.3

* [maven-release-plugin] prepare for next development iteration

Co-authored-by: Christian Frommeyer <[email protected]>
Co-authored-by: thedevluffy <[email protected]>
@efenderbosch
Copy link
Author

@brettcooper

boostchicken added a commit that referenced this issue Jun 17, 2020
* v5.2.3 (#18)

* Read Consistency Setting

* #8

Custom Mapper Spec

* #8

Custom Mapper CDI fix

* Fix symlink to CONTRIBUTING.md (#13)

* #8 Don't use autowired or constructor injection with FactoryBeans (#12)

* [maven-release-plugin] prepare release v5.2.2

* [maven-release-plugin] prepare for next development iteration

* 5.2.2 docs fix

* Hibernate Validator CVE fix
GHSA-m8p2-495h-ccmh

* Documentation updates

* Create FUNDING.yml

* Fixed a Type (#17)

Co-authored-by: thedevluffy <[email protected]>

* Documentation updates

* 5.2.3 prep

* [maven-release-plugin] prepare release v5.2.3

* [maven-release-plugin] prepare for next development iteration

Co-authored-by: Christian Frommeyer <[email protected]>
Co-authored-by: thedevluffy <[email protected]>

* ✨ Allow CONTAINS and NOT_CONTAINS is repositories

* 🎨 cleanup

* 🔧 check for collection

* 🔧 check for collection

* ✏️ cleanup

* 🎨 cleanup imports

* Remove funding changes

Co-authored-by: John D <[email protected]>
Co-authored-by: Christian Frommeyer <[email protected]>
Co-authored-by: thedevluffy <[email protected]>
Co-authored-by: John Dorman <[email protected]>
Co-authored-by: Hannes Angst <[email protected]>
boostchicken added a commit that referenced this issue Jun 17, 2020
* Update FUNDING.yml

* Introduce string set handling (#34)

* v5.2.3 (#18)

* Read Consistency Setting

* #8

Custom Mapper Spec

* #8

Custom Mapper CDI fix

* Fix symlink to CONTRIBUTING.md (#13)

* #8 Don't use autowired or constructor injection with FactoryBeans (#12)

* [maven-release-plugin] prepare release v5.2.2

* [maven-release-plugin] prepare for next development iteration

* 5.2.2 docs fix

* Hibernate Validator CVE fix
GHSA-m8p2-495h-ccmh

* Documentation updates

* Create FUNDING.yml

* Fixed a Type (#17)

Co-authored-by: thedevluffy <[email protected]>

* Documentation updates

* 5.2.3 prep

* [maven-release-plugin] prepare release v5.2.3

* [maven-release-plugin] prepare for next development iteration

Co-authored-by: Christian Frommeyer <[email protected]>
Co-authored-by: thedevluffy <[email protected]>

* ✨ Allow CONTAINS and NOT_CONTAINS is repositories

* 🎨 cleanup

* 🔧 check for collection

* 🔧 check for collection

* ✏️ cleanup

* 🎨 cleanup imports

* Remove funding changes

Co-authored-by: John D <[email protected]>
Co-authored-by: Christian Frommeyer <[email protected]>
Co-authored-by: thedevluffy <[email protected]>
Co-authored-by: John Dorman <[email protected]>
Co-authored-by: Hannes Angst <[email protected]>

* Updating changelog for #33

* Updating for release

* [maven-release-plugin] prepare release v5.2.5

* [maven-release-plugin] prepare for next development iteration

Co-authored-by: Hannes Angst <[email protected]>
Co-authored-by: Christian Frommeyer <[email protected]>
Co-authored-by: thedevluffy <[email protected]>
Co-authored-by: Hannes Angst <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants