Skip to content
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

Override CaseChecker configuration #1289

Closed
jeangael-domine opened this issue Jul 9, 2021 · 3 comments
Closed

Override CaseChecker configuration #1289

jeangael-domine opened this issue Jul 9, 2021 · 3 comments

Comments

@jeangael-domine
Copy link

Hi,

After getting the Kotlin & Java upgrade I encounter an issue with the override of the CaseChecker defined in the reference.conf of the core module.
I redefine two properties to change the case check in a reference.conf inside my ruleset module like this:

CaseChecker.propertyNames.allow = [${CaseChecker.cases.camelCase}]
CaseChecker.queryParameterNames.allow = [${CaseChecker.cases.camelCase}]

To make it work, in the build.gradle.kts, I added my ruleset dependency before the core one to ensure typesafe loaded mine first:

dependencies {
    val jadlerVersion = "1.3.0"

    implementation(project(":zally-ruleset-afkl"))
    implementation(project(":zally-core"))

It was working fine until the upgrade though I have no idea what exactly changed the behavior.

I know it is not a Zally's issue but I was wondering if anyone had the same need as me and if so how they manage to handle it.

Thank you for your help

Jean-Gaël or JG

@jeangael-domine
Copy link
Author

Hi

I found a workaround by adding the two lines in the rules-config file inside the server module but I'd rather be able to leave it in the ruleset module if possible.

@vadeg
Copy link
Contributor

vadeg commented Jul 12, 2021

Hi @jeangael-domine ,

It is hard to say what has been changed. Java / Kotlin migration didn't touch configuration modules. The configuration Zally is implemented using typesafe config. The problem here that zally-core and your module use the same mechanism for default settings - define reference.conf file. The library doesn't guarantee in which order reference.conf files would be merged and resolved. Perhaps, after the upgrade the resolution order has been changed and now the reference.conf from zally-core merged after the reference.conf from your module. I would say you were lucky enough since this moment by using undocumented librarys' feature 😀

The workaround you have found is actually one of the solutions proposed by the config library. The final word is always from the final config.

I think in general we should review our current configuration setup especially in cases with custom rule sets which reuse parts of existing configuration. The same will happen with two existing rule sets (zally-ruleset-zally and zally-ruleset-zalando) if they will have an overlapping configuration.

@jeangael-domine
Copy link
Author

Hi,

Thanks for your quick reply @vadeg

What made me think it was not completely sheer luck that it was working is that if I reset to the commit preceding the Java/Kotlin upgrade, everything was loading in the right order.
But I could not figure out what such an upgrade would impact this behavior.

Anyway it is true that I was not really confident about not having bugs in the future with my solution of changing the order of declaration in the build file.

I'll stick to this solution then even if it would have made more sense to me to be able to place the conf in the ruleset file and not the server. But the lib does not make it possible :)

Thanks for your explanations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants