Enable SpEL expressions for @Document collection name

[dsibilio]

This pull request serves to enable SpEL expression resolution for com.microsoft.azure.spring.data.cosmosdb.core.mapping.Document annotation's collection field.

In this way it would be possible to dynamically load a collection name as follows:

• from property source: e.g.

  @Document(collection = "${dynamic.collection.name}")
  public class MyCollection 

  ---

  @Document(collection = "#{@environment.getProperty('dynamic.collection.name')}")
  public class MyCollection 

• from bean field/method invocation:

  @Document(collection = "#{@someBean.getCollectionName()}")
  public class MyCollection 

This PR would fix #391.

[dsibilio]

@saragluna @Incarnation-p-lee Is the Travis CI pipeline for integration tests currently broken? I get the following error:

Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [com.microsoft.azure.spring.data.cosmosdb.config.DocumentDBConfig]: Factory method 'getConfig' threw exception; nested exception is java.lang.IllegalArgumentException: connection string should have text!
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:185)
    at org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:622)
    ... 44 more
Caused by: java.lang.IllegalArgumentException: connection string should have text!

[saragluna]

@dsibilio I will look into this.

[saragluna]

@dsibilio I've tried the CI (check it here) and it worked. I also ran integration test against this PR locally which succeeded too. Perhaps we could add this to your branch to help debug and remove it later.

<exec osfamily="unix" executable="cat">
   <arg value="./src/test/resources/application.properties"/>
</exec>

[dsibilio]

@dsibilio I've tried the CI (check it here) and it worked. I also ran integration test against this PR locally which succeeded too. Perhaps we could add this to your branch to help debug and remove it later.

<exec osfamily="unix" executable="cat">
   <arg value="./src/test/resources/application.properties"/>
</exec>

I've added the bit that you mentioned and obtained the following result:

[INFO] Executing tasks
main:
setup:
     [echo] setup test resource
[propertyfile] Updating property file: /home/travis/build/microsoft/spring-data-cosmosdb/src/test/resources/application.properties
     [exec] #Mon, 02 Sep 2019 07:13:45 +0000
     [exec] cosmosdb.uri=https\://testdb-09-02-07-12-25.documents.azure.com\:443
     [exec] cosmosdb.key=
     [exec] cosmosdb.secondaryKey=undefined
     [exec] 
     [exec] #You can also use connection string instead of uri and key to connect to cosmos DB
     [exec] #cosmosdb.connection-string=${DOCUMENTDB_CONNECTION_STRING}
     [exec] 
     [exec] dynamic.collection.name=spel-property-collection
     [exec] # Performance test configurations
     [exec] perf.recursive.times=10
     [exec] perf.batch.size=3
     [exec] perf.acceptance.percentage=10
     [exec] 
[INFO] Executed tasks

It seems it's missing the key...?

PS. Codacy bot might have gone rogue, it's thinking some tests don't contain assertions when they actually do 😢

[saragluna]

@dsibilio I guess there's something wrong with azure-cli output when keys list command gets called. Could you please also add this to your branch or allow me to push debug commits to your branch.

[saragluna]

@dsibilio This problem is probably caused by this.

[dsibilio]

@dsibilio This problem is probably caused by this.

Thanks for investigating. Let me know if I can lend a hand somehow ;)

[kushagraThapar]

/azp run

[azure-pipelines[bot]]

No pipelines are associated with this pull request.

[kushagraThapar]

/azp run Emulator-spring-data-cosmosdb-tests

[azure-pipelines[bot]]

No pipelines are associated with this pull request.

[kushagraThapar]

/azp run

[azure-pipelines[bot]]

No pipelines are associated with this pull request.

[kushagraThapar]

/azp run

[azure-pipelines[bot]]

No pipelines are associated with this pull request.

[kushagraThapar]

@dsibilio, can you please check the logs of these two failed tests ? If you can't I will paste the exceptions here.

[dsibilio]

@dsibilio, can you please check the logs of these two failed tests ? If you can't I will paste the exceptions here.

I noticed the SpELDocumentDBAnnotationIT.testDatabaseOperationsOnDynamicallyNamedCollection test failed. Sadly I've got connection issues at the moment and can't replicate the problem locally to analyze it. You can either try and troubleshoot yourself or wait for my internet connection to get fixed 👍

[kushagraThapar]

@dsibilio, can you please check the logs of these two failed tests ? If you can't I will paste the exceptions here.

I noticed the SpELDocumentDBAnnotationIT.testDatabaseOperationsOnDynamicallyNamedCollection test failed. Sadly I've got connection issues at the moment and can't replicate the problem locally to analyze it. You can either try and troubleshoot yourself or wait for my internet connection to get fixed 👍

Thanks for checking, it's good that you can view the logs. I will try to fix it if I get some time. I am fully packed today, but can help tomorrow. Or if you get good connection, then please fix it.

[dsibilio]

@dsibilio, can you please check the logs of these two failed tests ? If you can't I will paste the exceptions here.

I noticed the SpELDocumentDBAnnotationIT.testDatabaseOperationsOnDynamicallyNamedCollection test failed. Sadly I've got connection issues at the moment and can't replicate the problem locally to analyze it. You can either try and troubleshoot yourself or wait for my internet connection to get fixed +1

Thanks for checking, it's good that you can view the logs. I will try to fix it if I get some time. I am fully packed today, but can help tomorrow. Or if you get good connection, then please fix it.

I'm living on the edge, USB tethering my way to production ahah

I fixed the test, it was using a wrong ObjectMapper (one without fail_on_unknown_fields set to false). That test should pass now, if you run the azp we can check if all tests pass :+1:

[kushagraThapar]

/azp run

[ericher4worximity]

@kushagraThapar I am trying this fix on the version 3.7.0 but encounter a problem. The @Document collection SPeL evaluation (CosmosEntityInformation.getContainerName line 165) tries to resolve the expression by calling the static method ExpressionResolver.resolveExpression. here is the problem: the static variable embeddedValueResolver is still null! Consequently, the expression is not resolved Burt returned as is. Result: the application fails to boot.

For debug purpose, I replaced the SPeL expression by a simple string value ("abc") and put a breakpoint in both the constructor of ExpressionResolver and on the static resolveExpression. The breakpoint in resolveExpression kicks in well before the one on the constructor.

Is there something I miss in my code to make it work as intended? Thanks for your support Eric

[kushagraThapar]

@kushagraThapar I am trying this fix on the version 3.7.0 but encounter a problem. The @document collection SPeL evaluation (CosmosEntityInformation.getContainerName line 165) tries to resolve the expression by calling the static method ExpressionResolver.resolveExpression. here is the problem: the static variable embeddedValueResolver is still null! Consequently, the expression is not resolved Burt returned as is. Result: the application fails to boot.

For debug purpose, I replaced the SPeL expression by a simple string value ("abc") and put a breakpoint in both the constructor of ExpressionResolver and on the static resolveExpression. The breakpoint in resolveExpression kicks in well before the one on the constructor.

Is there something I miss in my code to make it work as intended? Thanks for your support Eric

@ericher4worximity - This would mean that the expressionResolver bean is not getting created successfully. Can you please debug creation of this bean ?

public ExpressionResolver(BeanFactory beanFactory) {
        if (beanFactory instanceof ConfigurableBeanFactory) {
            setEmbeddedValueResolver(new EmbeddedValueResolver((ConfigurableBeanFactory) beanFactory));
        }
    }

@dsibilio - Can you please help us here ? I can also miss any important piece here, as you wrote this code :)

[dsibilio]

If the embeddedValueResolver is still null when the resolveExpression method is called, it means the method was called before the bean was initialized. @ericher4worximity, you could try again by adding @DependsOn("expressionResolver") on top of a Spring configuration class that is certainly processed before the static method is invoked.

[ericher4worximity]

@kushagraThapar I did put a breakpoint in the constructor of ExpressionResolver. It is eventually being called but after the SPeL expression has been evaluated. I did find a workaround: by having a @Bean that depends on ExpressionResolver (it does not even use it!). It seems it forces Spring to create it before the SPeL is being evaluated. I'll try to simplify my code so I can be shared.

[ericher4worximity]

@dsibilio Adding the @DependsOn on the Application main class works. It's a better workaround that what I came up to, I agree... but a workaround nonetheless :)

Here the code (in Kotlin)

@DependsOn("expressionResolver")
@SpringBootApplication
class Application

fun main(args: Array<String>) {
    runApplication<Application>(*args)
}