Differential D29347

KAuthorized: export method to reload restrictions
ClosedPublic
Actions

Authored by dfaure on May 1 2020, 7:25 PM.

Details

Reviewers
aacid
apol
mdawson
Commits
R237:2789bb2c7b26: KAuthorized: export method to reload restrictions
Summary

This is useful for unittests. Example:

KCONFIGCORE_EXPORT void reloadUrlActionRestrictions();

void someTestMethod()
{
    KConfigGroup cg(KSharedConfig::openConfig(), "KDE URL Restrictions");
    cg.writeEntry("rule_count", 1);
    cg.writeEntry("rule_1", QStringList{"open", {}, {}, {}, "file", "", "", "false"});
    cg.sync();
    reloadUrlActionRestrictions();

    // Some test for code that uses KUrlAuthorized

    cg.deleteEntry("rule_1");
    cg.deleteEntry("rule_count");
    cg.sync();
    reloadUrlActionRestrictions();
}

This is consistent with the fact that other functions used by
KUrlAuthorized are defined here and exported internally.

Test Plan

Used this in a KIO unittest I'm writing for the future OpenUrlJob

Diff Detail

Repository
R237 KConfig
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26205
Build 26223: arc lint + arc unit
dfaure created this revision.May 1 2020, 7:25 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMay 1 2020, 7:25 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
dfaure requested review of this revision.May 1 2020, 7:25 PM
aacid added a comment.May 2 2020, 2:29 PM

So how you're going to use it from unittests? declaring the function there?

maybe better to declare it to some of the _p.h headers ?

dfaure edited the summary of this revision. (Show Details)May 2 2020, 5:03 PM

Yes declaring the function there, as in the code sample shown here.

We do the same for internally-exported variables like KSERVICE_EXPORT int ksycoca_ms_between_checks;
Qt does the same kind of things.

A _p.h header would have to be installed, which we don't normally do.

aacid accepted this revision.May 3 2020, 9:49 AM

ahhhh, for using in a different repo, ok.

This revision is now accepted and ready to land.May 3 2020, 9:49 AM
dfaure closed this revision.May 3 2020, 11:24 AM
kossebau added a subscriber: kossebau.May 3 2020, 11:36 AM
kossebau added inline comments.
src/core/kauthorized.cpp
247

Please also add a note next to this that it is for unit test, so people are not wondering about this random KCONFIGCORE_EXPORT and first have to research commit history.

mdawson added a comment.May 3 2020, 9:41 PM

One suggestion for this change:
Instead of exporting a method that takes no parameters and always loads from configuration file, why not make a new method with the implementation that takes in a given KConfigGroup. That way unit tests can pass in a KConfigGroup setup appropriately without having to create a normal configuration file in the user's home folder (ie use a temporary file). They can also configure the KConfig to not cascade/use the global configuration so they are isolated from the environment.

initUrlActionRestrictions can then remain the same, and pass in the KConfigGroup that would have been used previously.

It would look something like:

KCONFIGCORE_EXPORT void loadUrlActionRestrictions(KConfigGroup cg)
{
    // Code as before, without creating the KConfigGroup
}
void initUrlActionRestrictions()
{
    KConfigGroup cg(KSharedConfig::openConfig(), "KDE URL Restrictions");
    loadUrlActionRestrictions(cg);
}

Revision Contents
Changeset List

PathPackages
Msrc/core/kauthorized.cpp (4 lines)

Diff 81701

View Options

src/core/kauthorized.cpp

Loading...