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

Seperate OneDriveWebhook from OnedriveAPI instance #2607

Merged
merged 4 commits into from
Feb 20, 2024

Conversation

JC-comp
Copy link
Contributor

@JC-comp JC-comp commented Jan 27, 2024

This pull request separates the OneDriveWebhook from OnedriveAPI, as part of #2530.

Changes

  • Create a seperate webhook module
  • Move OneDriveWebhook out of OneDriveApi
  • Replace global oneDriveApiInstance in main.d with oneDriveWebhook
  • Remove singleton pattern for OneDriveWebhook

@abraunegg abraunegg added this to the v2.5.0 milestone Feb 11, 2024
@abraunegg
Copy link
Owner

@JC-comp
This PR fails to compile:

ldc2  -w -g -O -J. -d-debug -gc -L-lcurl -L-lsqlite3  -L-ldl src/main.d src/config.d src/log.d src/util.d src/qxor.d src/curlEngine.d src/onedrive.d src/webhook.d src/sync.d src/itemdb.d src/sqlite.d src/clientSideFiltering.d src/monitor.d src/arsd/cgi.d -ofonedrive
src/arsd/cgi.d(3921): Error: static assert:  "Not implemented in your compiler version!"
src/webhook.d(112):        instantiated from here: serveEmbeddedHttp!(handle, Cgi, 5000000L)

So in your testing and building, you must test each PR separately, by itself, compiling against the minimum LDC version, using a script similar to the following (assuming you already know how to install 'ldc' manually, and can install v1.20.1):

#!/bin/bash
  
PR=2607

rm -rf ./onedrive-pr${PR}
git clone https://github.com/abraunegg/onedrive.git onedrive-pr${PR}
cd onedrive-pr${PR}
git fetch origin pull/${PR}/head:pr${PR}
git checkout pr${PR}

# MIN LDC Version to compile
# MIN Version for ARM / Compiling with LDC
#source ~/dlang/ldc-1.18.0/activate
source ~/dlang/ldc-1.20.1/activate


./configure --enable-debug --enable-notifications; make clean; make;
deactivate
./onedrive --version

Post building, then start running all your tests for which the PR is resolving and/or fixing.

@JC-comp
Copy link
Contributor Author

JC-comp commented Feb 11, 2024

Update

  • Ensure backward compatibility with ldc v1.20.1 and test the compilation.

@abraunegg
Copy link
Owner

@JC-comp
Have you tested this PR against actually using the feature ?

@abraunegg
Copy link
Owner

@Lyncredible
Would it be possible for you to run this specific PR through your Webhook testing please?

@JC-comp
Copy link
Contributor Author

JC-comp commented Feb 12, 2024

Have you tested this PR against actually using the feature ?

@abraunegg
I've tested a cycle of webhook startup, creation, event handling, deletion, and shutdown with no issues.

Reading configuration file: /home/alan/.config/onedrive/config
Configuration file successfully loaded
Using IPv4 and IPv6 (if configured) for all network operations
Configuring Global Azure AD Endpoints
OneDrive synchronisation interval (seconds): 7200
Initialising filesystem inotify monitoring ...
Performing initial syncronisation to ensure consistent local state ...
Started webhook server
Initializing subscription for updates ...
Webhook: handled validation request
Created new subscription cf469518-5f04-40ab-b825-400a2b8906ed with expiration: 2024-02-11T06:46:56.7640272Z
Starting a sync with Microsoft OneDrive
Performing a database consistency and integrity check on locally stored data
Got termination signal, performing clean up
Waiting for any existing upload|download process to complete
Shutting down DB connection and merging temporary data
Stopped webhook server
Deleted subscription

@abraunegg
Copy link
Owner

@JC-comp
Is there a subscription event being handled there ? dont see it ?

@JC-comp
Copy link
Contributor Author

JC-comp commented Feb 12, 2024

@JC-comp
Is there a subscription event being handled there ? dont see it ?

Line 9-11

Initializing subscription for updates ...
Webhook: handled validation request
Created new subscription cf469518-5f04-40ab-b825-400a2b8906ed with expiration: 2024-02-11T06:46:56.7640272Z

@Lyncredible
Copy link
Contributor

@JC-comp
Is there an actual event being handled, such as an updated file?

@abraunegg
I can try this upcoming weekend. Sorry for not being able to contribute, but it has been hectic lately.

@abraunegg
Copy link
Owner

@JC-comp
Is there a subscription event being handled there ? dont see it ?

Line 9-11

Initializing subscription for updates ...
Webhook: handled validation request
Created new subscription cf469518-5f04-40ab-b825-400a2b8906ed with expiration: 2024-02-11T06:46:56.7640272Z

This is the initialising .. not any online activity being handled.

@abraunegg
Copy link
Owner

@Lyncredible

I can try this upcoming weekend. Sorry for not being able to contribute, but it has been hectic lately.

No problem - any testing you can do is greatly appreciated

@JC-comp
Copy link
Contributor Author

JC-comp commented Feb 15, 2024

This is the initialising .. not any online activity being handled.

@abraunegg
The following are the logs from testing the handling of the webhook events.

...
Sync with Microsoft OneDrive is complete

[M] Total number of local file changed: 2
[M] New local file added: ./11
[M] New local file added: ./22
New items to upload to OneDrive: 2
Total New Data to Upload:        0 Bytes
Uploading new file ./11 ... done.
Uploading new file ./22 ... done.
Webhook: sent refresh signal #1
Received 1 refresh signals from the webhook
Fetching /delta response from the OneDrive API for Drive ID: af2b5e52bea96776
Processing API Response Bundle: 1 - Quantity of 'changes|items' in this bundle to process: 3
Finished processing /delta JSON response from the OneDrive API
Processing 2 applicable changes and items received from Microsoft OneDrive
Processing OneDrive JSON item batch [1/1] to ensure consistent local state
[M] Local item deleted: ./11
The operating system sent a deletion notification. Trying to delete the item as requested
Deleting item from OneDrive: ./11
[M] Local item deleted: ./22
The operating system sent a deletion notification. Trying to delete the item as requested
Deleting item from OneDrive: ./22
Webhook: sent refresh signal #2
Received 1 refresh signals from the webhook
Fetching /delta response from the OneDrive API for Drive ID: af2b5e52bea96776
Processing API Response Bundle: 1 - Quantity of 'changes|items' in this bundle to process: 3
Finished processing /delta JSON response from the OneDrive API
No additional changes or items that can be applied were discovered while processing the data received from Microsoft OneDrive
[M] Total number of local file changed: 2
[M] New local file added: ./11
[M] New local file added: ./22
New items to upload to OneDrive: 2
Total New Data to Upload:        0 Bytes
Uploading new file ./11 ... done.
Uploading new file ./22 ... done.
Webhook: sent refresh signal #3
Received 1 refresh signals from the webhook
Fetching /delta response from the OneDrive API for Drive ID: af2b5e52bea96776
Processing API Response Bundle: 1 - Quantity of 'changes|items' in this bundle to process: 3
Finished processing /delta JSON response from the OneDrive API
Processing 2 applicable changes and items received from Microsoft OneDrive
Processing OneDrive JSON item batch [1/1] to ensure consistent local state
^CGot termination signal, performing clean up
Waiting for any existing upload|download process to complete
Shutting down DB connection and merging temporary data
Stopped webhook server

@abraunegg
Copy link
Owner

@Lyncredible
Any update from yourself on testing this code change?

@Lyncredible
Copy link
Contributor

@abraunegg I gave it a try just now. It seems to be working fine

...
Webhook: sent refresh signal #1
Webhook: sent refresh signal #2
Processing 2 applicable changes and items received from Microsoft OneDrive .
Downloading file Temp/test.txt ... done
Sync with Microsoft OneDrive is complete
Received 2 refresh signals from the webhook
Fetching items from the OneDrive API for Drive ID: <redacted> ..
No additional changes or items that can be applied were discovered while processing the data received from Microsoft OneDrive
Webhook: sent refresh signal #3
Received 1 refresh signals from the webhook
Fetching items from the OneDrive API for Drive ID: <redacted> ..
Processing 2 applicable changes and items received from Microsoft OneDrive .
Downloading file Temp/text2.txt ... done
Webhook: sent refresh signal #4
Received 1 refresh signals from the webhook
Fetching items from the OneDrive API for Drive ID: <redacted> ..
Processing 1 applicable changes and items received from Microsoft OneDrive .
Trying to delete file Temp/test.txt
Deleting file Temp/test.txt
Trying to delete file Temp/text2.txt
Deleting file Temp/text2.txt
The operating system sent a deletion notification. Trying to delete the item as requested
The operating system sent a deletion notification. Trying to delete the item as requested
...

@abraunegg
Copy link
Owner

@Lyncredible
Thanks for the confirmation - will merge this into 'alpha-5'

@abraunegg abraunegg merged commit 03386c1 into abraunegg:onedrive-v2.5.0-alpha-5 Feb 20, 2024
@abraunegg
Copy link
Owner

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

Repository owner locked and limited conversation to collaborators Feb 28, 2024
@JC-comp JC-comp deleted the webhook branch March 5, 2024 03:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants