Extract the access to the user config in its own helper class #926
Conversation
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
christianlupus
force-pushed
the
dev/config-abstraction
branch
from
b0b5b38
to
00ecbd2
Compare
Codecov Report
@@ Coverage Diff @@
## master #926 +/- ##
==========================================
+ Coverage 20.20% 22.42% +2.21%
==========================================
Files 20 21 +1
Lines 1440 1476 +36
==========================================
+ Hits 291 331 +40
+ Misses 1149 1145 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
lib/Helper/UserConfigHelper.php
Outdated
| * @return integer The timestamp of the last index rebuild | ||
| */ | ||
| public function getLastIndexUpdate(): int { | ||
| $rawValue = $this->getRawValue('last_index_update'); |
There was a problem hiding this comment.
Is there a sensible way to get rid of those magic strings?
There was a problem hiding this comment.
That was exactly the reason for introducing this class. Until now, these magic strings were distributed in the complete code. Now, they are at least condensed into a single file/class.
One could at most introduce constants of the class to use them. But the effect would be similar. The thing is that the server provides just a string based key-value interface. We could as well store one JSON object there. (Now easier as all is contained in the class)
| * | ||
| * @return string The name of the folder within the users files | ||
| */ | ||
| public function getFolderName(): string { |
There was a problem hiding this comment.
Should we think of having multiple cookbooks here? As discussed in #340
You probably just want to provide the same functionality as the current code but in a nicer package?
There was a problem hiding this comment.
Yes, exactly. For now, I wanted to create something like a DBAL.
Extending this and returning an array of strings (and storing appropriately), is just a small change in this class plus the corresponding upper layers. These upper layers I wanted to generate some abstractions as well. So the switch from one to many cookbooks should be a smaller one.
When separating file system access from business logic, we can work towards the discussed results in #340 in the classes there.
Review by @seyfeb Co-authored-by: Sebastian Fey <info@sebastianfey.de> Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
christianlupus
force-pushed
the
dev/config-abstraction
branch
from
00e73bd
to
8a1242d
Compare
This should encapsulate the access to the user settings.