check config file keys for validity, use setValue instead of direct access #296

norbusan · 2018-12-19T07:04:10Z

All possible values are already initialized in init before the config file is loaded. Check the keys of the config files against the existing keys and warn in case of mismatch.

Furthermore, instead of directly changing the values array, use the setValue function to allow for easier reimplementation of the configuration storage.

…ccess

norbusan · 2018-12-19T07:06:23Z

My recommendation is to include this into 2.2.2 milestone, to fix things like what has happened in #293

abraunegg · 2018-12-19T19:00:15Z

For some reason this change ignore's this in the config file:

# Drive ID for SharePoint Shared Library
drive_id = "b!6H_y8BWP8Uqg2X0QAy4XfxiTMFe_reZKrMZXLeBjL4g70-n16rIGQpOG_l7vVxU5"

Output with this PR:

./onedrive --confdir '~/odconfig/' --synchronize --verbose 
Loading config ...
Using Config Dir: /home/alex/odconfig/
Unknown key in config file: drive_id
Initializing the OneDrive API ...
Opening the item database ...
All operations will be performed in: /home/alex/OneDriveALT
Initializing the Synchronization Engine ...
Account Type: business
Default Drive ID: b!xA9hqR7mT0OL6Gku8YbuzTJ8oZmRnQZJu6-yDL49Zc5MccrHbuT0R4QUwIzUvbJY
Default Root ID: 01D6JQORV6Y2GOVW7725BZO354PWSELRRZ
Remaining Free Space: 1099511108257
Fetching details for OneDrive Root
OneDrive Root exists in the database
Syncing changes from OneDrive ...
Applying changes of Path ID: 01D6JQORV6Y2GOVW7725BZO354PWSELRRZ
Uploading differences of .
Processing root
The directory has not changed
Processing 4
The directory has not changed
Processing Document.docx
The file has not changed
Processing 2
The directory has not changed
Processing Document.docx
The file has not changed
Processing 1
The directory has not changed
Processing 1.txt
The file has not changed
Processing 3
The directory has not changed
Processing Presentation.pptx
The file has not changed
Uploading new items of .
Applying changes of Path ID: 01D6JQORV6Y2GOVW7725BZO354PWSELRRZ

Output with master:

./onedrive --confdir '~/odconfig/' --synchronize --verbose 
Loading config ...
Using Config Dir: /home/alex/odconfig/
Initializing the OneDrive API ...
Opening the item database ...
All operations will be performed in: /home/alex/OneDriveALT
Initializing the Synchronization Engine ...
Account Type: documentLibrary
Default Drive ID: b!6H_y8BWP8Uqg2X0QAy4XfxiTMFe_reZKrMZXLeBjL4g70-n16rIGQpOG_l7vVxU5
Default Root ID: 01LWWIJZN6Y2GOVW7725BZO354PWSELRRZ
Remaining Free Space: 27487788820266
Fetching details for OneDrive Root
OneDrive Root exists in the database
Syncing changes from OneDrive ...
Applying changes of Path ID: 01LWWIJZN6Y2GOVW7725BZO354PWSELRRZ
Uploading differences of .
Processing root
The directory has not changed
Processing 4
The directory has not changed
Processing Document.docx
The file has not changed
Processing 2
The directory has not changed
Processing Document.docx
The file has not changed
Processing 1
The directory has not changed
Processing 1.txt
The file has not changed
Processing 3
The directory has not changed
Processing Presentation.pptx
The file has not changed
Uploading new items of .
Applying changes of Path ID: 01LWWIJZN6Y2GOVW7725BZO354PWSELRRZ

Note the switch to the Office 365 Shared Library because 'drive_id' was in the config

Also, if ddrive_id is used, this is not picked up by itself, however if there is a spelling error in any of the other config options then ddrive_id is picked up:

./onedrive --confdir '~/odconfig/' --synchronize --verbose 
Loading config ...
Using Config Dir: /home/alex/odconfig/
Unknown key in config file: ssync_dir
Unknown key in config file: skkip_file
Unknown key in config file: moniitor_interval
Unknown key in config file: ddrive_id
Initializing the OneDrive API ...
Opening the item database ...
All operations will be performed in: /home/alex/OneDrive
Initializing the Synchronization Engine ...
Account Type: business
Default Drive ID: b!xA9hqR7mT0OL6Gku8YbuzTJ8oZmRnQZJu6-yDL49Zc5MccrHbuT0R4QUwIzUvbJY
Default Root ID: 01D6JQORV6Y2GOVW7725BZO354PWSELRRZ
Remaining Free Space: 1099511108257
Fetching details for OneDrive Root
OneDrive Root exists in the database

If drive_id is spelt OK, but a prior key is not, then this is picked up as being invalid:

./onedrive --confdir '~/odconfig/' --synchronize --verbose 
Loading config ...
Using Config Dir: /home/alex/odconfig/
Unknown key in config file: moniitor_interval
Unknown key in config file: drive_id
Initializing the OneDrive API ...
Opening the item database ...
All operations will be performed in: /home/alex/OneDriveALT
Initializing the Synchronization Engine ...
Account Type: business
Default Drive ID: b!xA9hqR7mT0OL6Gku8YbuzTJ8oZmRnQZJu6-yDL49Zc5MccrHbuT0R4QUwIzUvbJY
Default Root ID: 01D6JQORV6Y2GOVW7725BZO354PWSELRRZ
...

norbusan · 2018-12-19T19:06:56Z

Ahh. indeed ... because drive_id is not initially set ... I thought that config.d:init does set all possible config options:

        void init()
        {
                // Default configuration directory
                setValue("sync_dir", "~/OneDrive");
                // Configure to skip ONLY temp files (~*.doc etc) by default
                // Prior configuration was: .*|~*
                setValue("skip_file", "~*");
                // By default symlinks are not skipped (using string type
                // instead of boolean because hashmap only stores string types)
                setValue("skip_symlinks", "false");
                // Configure the monitor mode loop - the number of seconds by which
                // each sync operation is undertaken when idle under monitor mode
                setValue("monitor_interval", "45");
                // Configure the default logging directory to be /var/log/onedrive/
                setValue("log_dir", "/var/log/onedrive/");
                
                if (!load(userConfigFilePath)) {
                        log.vlog("No config file found, using defaults");
                }
        }

but that was wrong.

How can one find the definitive list of allowed config options?

abraunegg · 2018-12-19T19:14:28Z

How can one find the definitive list of allowed config options?

drive_id is the only option that is 'not' set currently via config.d:init and is called via onedrive.d:init

Looking right now at this as if drive_id is set to an empty string via config.d:init what impact this has

abraunegg · 2018-12-19T19:24:02Z

OK .. that has the desired effect where now 'drive_id' is correctly picked up.

I am thinking here as well - if there is a 'config' file issue, we don't allow the application to sync - as, if for example the 'drive_id' is spelt incorrectly, the actual account 'drive_id' would be used - which is what led to #293

Add 'drive_id' to be initialised, set to an empty string

* Issue #293 was caused by a spelling error in the configuration file. If the configuration file has errors, we should not load it or run using the application defaults as this may have undesirable consequences for users data

* Missed this edit of the file

abraunegg · 2018-12-19T20:11:04Z

I think this is good to merge now - thoughts?

norbusan · 2018-12-19T23:53:29Z

Thanks, merged!

adudek · 2018-12-20T05:51:40Z

For some reason --syncdir option is ignored when starting onedrive with latest master code. This caused all my items to be deleted when docker image was used --syncdir /onedrive/data.

Message on output showed that it will sync /home/onedrive/OneDrive folder.

lock · 2019-01-05T20:36:32Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

check config file keys for validity, use setValue instead of direct a…

12ee1f4

…ccess

norbusan requested a review from abraunegg December 19, 2018 07:04

norbusan mentioned this pull request Dec 19, 2018

Configuration file handling #297

Closed

abraunegg added this to the 2.2.2 milestone Dec 19, 2018

abraunegg added 3 commits December 20, 2018 06:26

Update config.d

6f6b584

Add 'drive_id' to be initialised, set to an empty string

exit application if there is a configuration file error

02e3109

* Issue #293 was caused by a spelling error in the configuration file. If the configuration file has errors, we should not load it or run using the application defaults as this may have undesirable consequences for users data

missed returning false on key issue

31c75d2

* Missed this edit of the file

abraunegg approved these changes Dec 19, 2018

View reviewed changes

norbusan merged commit 025a3b2 into master Dec 19, 2018

norbusan deleted the norbert/check-conf-values branch December 19, 2018 23:52

adudek mentioned this pull request Dec 20, 2018

syncdir option is ignored #301

Closed

lock bot locked and limited conversation to collaborators Jan 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

check config file keys for validity, use setValue instead of direct access #296

check config file keys for validity, use setValue instead of direct access #296

norbusan commented Dec 19, 2018

norbusan commented Dec 19, 2018

abraunegg commented Dec 19, 2018

norbusan commented Dec 19, 2018

abraunegg commented Dec 19, 2018 •

edited

Loading

abraunegg commented Dec 19, 2018 •

edited

Loading

abraunegg commented Dec 19, 2018

norbusan commented Dec 19, 2018

adudek commented Dec 20, 2018

lock bot commented Jan 5, 2019

check config file keys for validity, use setValue instead of direct access #296

check config file keys for validity, use setValue instead of direct access #296

Conversation

norbusan commented Dec 19, 2018

norbusan commented Dec 19, 2018

abraunegg commented Dec 19, 2018

norbusan commented Dec 19, 2018

abraunegg commented Dec 19, 2018 • edited Loading

abraunegg commented Dec 19, 2018 • edited Loading

abraunegg commented Dec 19, 2018

norbusan commented Dec 19, 2018

adudek commented Dec 20, 2018

lock bot commented Jan 5, 2019

abraunegg commented Dec 19, 2018 •

edited

Loading

abraunegg commented Dec 19, 2018 •

edited

Loading