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

D-Bus: Do not use timestamp optimization on "files" provider. #6343

Closed
wants to merge 1 commit into from

Conversation

aplopez
Copy link
Contributor

@aplopez aplopez commented Sep 8, 2022

Avoid requesting only the latest updates when using the "files" provider as it only updates the cache if /etc/files or /etc/group is touched.

Resolves: #6342

* Therefore the last update flag is not updated if no file was touched
* and we cannot use this optimization.
*/
if (strcasecmp(domain->provider, "files") == 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to use is_files_provider() from util.h?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Didn't know that function existed. Thanks.

@pbrezina
Copy link
Member

@aplopez Can you please write test for it?

@pbrezina
Copy link
Member

Otherwise ack to the code.

@alexey-tikhonov alexey-tikhonov added the no-backport This should go to target branch only. label Sep 16, 2022
@aplopez
Copy link
Contributor Author

aplopez commented Sep 19, 2022

Added a as requested by @pbrezina

#define TESTS_PATH "tp_" BASE_FILE_STEM
#define TEST_CONF_DB "test_responder_cache_req_conf.ldb"
#define TEST_DOM_NAME "responder_cache_req_test"
#define TEST_ID_PROVIDER "ldap"
#define TEST_ID_PROVIDER LDAP_ID_PROVIDER
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you defined both FILES_ID_PROVIDER and LDAP_ID_PROVIDER then I think you can just delete this macro and replace it with LDAP_ID_PROVIDER?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Avoid requesting only the latest updates when using the "files"
provider as it only updates the cache if /etc/files or /etc/group
is touched.

Added a test for this situation.

Resolves: SSSD#6342
@pbrezina
Copy link
Member

Pushed PR: #6343

  • master
    • f418e94 - D-Bus: Do not use timestamp optimization on "files" provider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport This should go to target branch only. Pushed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[D-Bus] ListByName() and ListByDomainAndname() return an empty list when used on the "files" provider
3 participants