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

fix redis ticket registry bean type #5577

Closed
wants to merge 1 commit into from
Closed

fix redis ticket registry bean type #5577

wants to merge 1 commit into from

Conversation

hdeadman
Copy link
Member

I am working on trying to re-create (locally with puppeteer test) cas in k8s talking to redis sentinel cluster in k8s, b/c I don't think CAS recovers from redis master changing. and this bean was failing unless I changed argument type to Topic (changing bean type to ChannelTopic also worked). Not sure why it works in existing tests, but my test just has reports and redis-ticket-registry.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov
Copy link

codecov bot commented Dec 31, 2022

Codecov Report

Merging #5577 (14260cb) into master (888f6b0) will increase coverage by 0.00783%.
The diff coverage is 83.33333%.

@@                  Coverage Diff                  @@
##                master       #5577         +/-   ##
=====================================================
+ Coverage     91.83826%   91.84609%   +0.00783%     
- Complexity       17650       17651          +1     
=====================================================
  Files             2792        2792                 
  Lines            68037       68041          +4     
  Branches          4887        4887                 
=====================================================
+ Hits             62484       62493          +9     
+ Misses            2643        2640          -3     
+ Partials          2910        2908          -2     
Flag Coverage Δ
activedirectory 7.72622% <0.00000%> (-0.00928%) ⬇️
actuatorendpoint 13.93865% <0.00000%> (+0.02417%) ⬆️
amazonwebservices 8.98429% <0.00000%> (-0.00052%) ⬇️
amqp 3.60518% <0.00000%> (-0.00021%) ⬇️
attributes 6.59014% <0.00000%> (+0.00256%) ⬆️
audits 0.67165% <0.00000%> (-0.00004%) ⬇️
authentication 8.62715% <0.00000%> (-0.03136%) ⬇️
authenticationhandler 7.13099% <0.00000%> (-0.01806%) ⬇️
authenticationmetadata 0.50264% <0.00000%> (-0.00002%) ⬇️
authenticationpolicy 3.00995% <0.00000%> (-0.00018%) ⬇️
authenticationthrottling 3.95497% <0.00000%> (-0.00023%) ⬇️
azure 4.47965% <0.00000%> (-0.01495%) ⬇️
cas 8.66536% <0.00000%> (-0.00051%) ⬇️
casconfiguration 6.09338% <0.00000%> (-0.00036%) ⬇️
cassandra 4.37530% <0.00000%> (+0.02327%) ⬆️
cipher 1.71514% <0.00000%> (-0.00009%) ⬇️
consent 4.97347% <0.00000%> (-0.01351%) ⬇️
cookie 4.35767% <0.00000%> (-0.00025%) ⬇️
couchbase 5.32032% <0.00000%> (-0.00472%) ⬇️
couchdb 10.75234% <0.00000%> (+0.03759%) ⬆️
delegation 7.70565% <0.00000%> (-0.00926%) ⬇️
duosecurity 8.33909% <0.00000%> (-0.00930%) ⬇️
dynamodb 10.92430% <0.00000%> (-0.00063%) ⬇️
events 1.20222% <0.00000%> (-0.00007%) ⬇️
expirationpolicy 8.40817% <0.00000%> (-0.00489%) ⬇️
filesystem 8.91374% <0.00000%> (+0.06415%) ⬆️
geolocation 4.98376% <0.00000%> (-0.00028%) ⬇️
git 6.67245% <0.00000%> (+0.04224%) ⬆️
groovy 12.58947% <0.00000%> (-0.00074%) ⬇️
grouper 4.22833% <0.00000%> (-0.00907%) ⬇️
hazelcast 4.50464% <0.00000%> (+0.00122%) ⬆️
hibernate 0.68341% <0.00000%> (-0.00004%) ⬇️
ignite 3.59342% <0.00000%> (-0.00021%) ⬇️
impersonation 5.17482% <0.00000%> (-0.00030%) ⬇️
influxdb 0.77453% <0.00000%> (-0.00298%) ⬇️
jdbc 13.02597% <0.00000%> (-0.01252%) ⬇️
jdbcauthentication 4.16220% <0.00000%> (-0.00023%) ⬇️
jdbcmfa 7.71447% <0.00000%> (+0.00250%) ⬆️
jmx 1.43296% <0.00000%> (+0.02050%) ⬆️
kafka 0.96706% <0.00000%> (-0.00005%) ⬇️
ldap 7.73945% <0.00000%> (-0.00634%) ⬇️
ldapattributes 1.72543% <0.00000%> (-0.00597%) ⬇️
ldapauthentication 6.10367% <0.00000%> (+0.00993%) ⬆️
ldaprepository 8.07601% <0.00000%> (-0.00047%) ⬇️
ldapservices 1.76952% <0.00000%> (+0.00871%) ⬆️
logout 0.96853% <0.00000%> (-0.00005%) ⬇️
mail 9.97634% <0.00000%> (-0.00498%) ⬇️
mariadb 7.58219% <0.00000%> (-0.00926%) ⬇️
memcached 4.68247% <0.00000%> (-0.00028%) ⬇️
metrics 1.84154% <0.00000%> (-0.00009%) ⬇️
mfa 6.82971% <0.00000%> (+0.15541%) ⬆️
mfaprovider 10.31437% <0.00000%> (-0.00060%) ⬇️
mfatrigger 8.05250% <0.00000%> (-0.00487%) ⬇️
mfatrusteddevices 5.17776% <0.00000%> (-0.00030%) ⬇️
mongodb 12.21175% <0.00000%> (-0.00071%) ⬇️
mongodbmfa 7.56456% <0.00000%> (+0.00692%) ⬆️
mssqlserver 8.35379% <0.00000%> (-0.00783%) ⬇️
mysql 13.63884% <0.00000%> (-0.00079%) ⬇️
oauth 10.03513% <0.00000%> (-0.00058%) ⬇️
oauthtoken 8.32145% <0.00000%> (-0.00048%) ⬇️
oauthweb 11.21383% <0.00000%> (-0.01388%) ⬇️
oidc 14.30608% <0.00000%> (-0.00084%) ⬇️
oracle 0.00588% <0.00000%> (ø)
passwordops 5.00434% <0.00000%> (-0.00028%) ⬇️
postgres 14.90425% <0.00000%> (-0.00969%) ⬇️
radius 6.03754% <0.00000%> (-0.00035%) ⬇️
redis 11.94721% <0.00000%> (+0.01253%) ⬆️
registeredservice 5.61279% <0.00000%> (+0.00115%) ⬆️
restfulapi 14.32666% <0.00000%> (-0.01259%) ⬇️
restfulapiauthentication 8.43462% <0.00000%> (-0.00049%) ⬇️
saml 6.09192% <0.00000%> (-0.00035%) ⬇️
saml1 6.98255% <0.00000%> (+0.00840%) ⬆️
saml2 12.63650% <0.00000%> (-0.00074%) ⬇️
saml2web 13.04361% <0.00000%> (-0.00076%) ⬇️
samllogout 7.48078% <0.00000%> (-0.00044%) ⬇️
samlmetadata 8.89758% <0.00000%> (+0.00095%) ⬆️
samlserviceprovider 6.76798% <0.00000%> (+0.01724%) ⬆️
scim 5.23655% <0.00000%> (-0.00178%) ⬇️
shell 2.13107% <0.00000%> (-0.00012%) ⬇️
simple 6.50784% <0.00000%> (+0.01432%) ⬆️
sms 4.87353% <0.00000%> (-0.00029%) ⬇️
spnego 5.86117% <0.00000%> (-0.00034%) ⬇️
syncope 5.72008% <0.00000%> (+0.00849%) ⬆️
tickets 8.16860% <0.00000%> (-0.00047%) ⬇️
uma 8.45373% <0.00000%> (-0.00048%) ⬇️
utility 8.84908% <0.00000%> (-0.00345%) ⬇️
web 6.77239% <0.00000%> (-0.00039%) ⬇️
webapp 5.63190% <83.33333%> (-0.04000%) ⬇️
webflow 8.66830% <0.00000%> (-0.00933%) ⬇️
webflowactions 14.05182% <0.00000%> (-0.00083%) ⬇️
webflowconfig 9.70003% <0.00000%> (+0.00090%) ⬆️
webflowevents 9.57952% <0.00000%> (-0.00938%) ⬇️
webflowmfaactions 10.85963% <0.00000%> (-0.00505%) ⬇️
webflowmfaconfig 8.43021% <0.00000%> (-0.00049%) ⬇️
wsfederation 9.07394% <0.00000%> (-0.01375%) ⬇️
x509 7.48519% <0.00000%> (-0.00043%) ⬇️
zookeeper 0.40711% <0.00000%> (-0.00001%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...o/cas/config/RedisTicketRegistryConfiguration.java 100.00000% <ø> (ø)
.../web/security/CasWebSecurityConfigurerAdapter.java 91.71271% <83.33333%> (-0.94266%) ⬇️
.../cas/support/events/CouchDbCasEventRepository.java 76.19048% <0.00000%> (-9.52381%) ⬇️
...che/DefaultPrincipalAttributesRepositoryCache.java 93.33333% <0.00000%> (-6.66666%) ⬇️
...support/events/dao/InfluxDbCasEventRepository.java 93.93939% <0.00000%> (-6.06060%) ⬇️
...reo/cas/util/spring/DisposableListFactoryBean.java 42.85714% <0.00000%> (ø)
...o/cas/ticket/registry/HazelcastTicketRegistry.java 84.61538% <0.00000%> (+0.96153%) ⬆️
...va/org/apereo/cas/services/GitServiceRegistry.java 89.91597% <0.00000%> (+1.68068%) ⬆️
...ordSynchronizationAuthenticationPostProcessor.java 100.00000% <0.00000%> (+4.44445%) ⬆️
...e/CreateResourceBasedRegisteredServiceWatcher.java 94.44444% <0.00000%> (+5.55555%) ⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mmoayyed
Copy link
Member

mmoayyed commented Jan 4, 2023

Have you also tested the pub/sub functionality with the bean change and confirmed that it continues to work ok?

@hdeadman
Copy link
Member Author

hdeadman commented Jan 6, 2023

The change you made last week that set Lazy to false for the whole class also fixes this issue. I suppose that is because Spring can see the actual type of the bean rather than the declared type that doesn't match the argument type? Intellij is complaining that redisTicketRegistryMessageTopic should be of type ChannelTopic but it seems to work as-is.

@hdeadman hdeadman closed this Jan 6, 2023
@mmoayyed
Copy link
Member

mmoayyed commented Jan 6, 2023

You might be correct there. We could register the topic dynamically if it starts to cause serious issues with laziness but I would prefer to keep an outer type as an interface to help with proxies. Can revisit if it shows up again.

I am curious as to why this was not caught in puppeteer tests. All should run with lazy-beans activated. I'll take a second look.

@hdeadman hdeadman deleted the master-redis branch May 10, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants