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

Update enforce statement for parent id database query (Issue #912) #913

Merged
merged 3 commits into from
May 9, 2020

Conversation

abraunegg
Copy link
Owner

  • Remove redundant enforce check as if the parent is not in the database, the previous statement ensures the parent is in the database. If the parent is a shared folder, the parent ID will never be in the database as we are never provided it.

* Remove redundant enforce check as if the parent is not in the database, the previous statement ensures the parent is in the database. If the parent is a shared folder, the parent ID will never be in the database as we are never provided it.
@abraunegg abraunegg requested a review from norbusan May 8, 2020 23:57
@norbusan
Copy link
Collaborator

norbusan commented May 9, 2020

Seems fine to me and understandably, but why was it there in the beginning, and could it have consequences for other usage patterns/combinations so that different failures occur? I.e., if it is not a shared folder, can it still be not in the database?

@abraunegg
Copy link
Owner Author

@norbusan
The original code was this:

...
// Perform the database lookup - is the parent in the database?
if (!itemdb.selectByPath(dirName(path), parent.driveId, parent)) {
	// parent is not in the database
	log.vdebug("Parent path is not in the database - need to add it");
	uploadCreateDir(dirName(path));
}
// still enforce check of parent path. if the above was triggered, the below will generate a sync retry and will now be sucessful
enforce(itemdb.selectByPath(dirName(path), parent.driveId, parent), "The parent item id is not in the database");

JSONValue driveItem = [
		"name": JSONValue(baseName(path)),
		"folder": parseJSON("{}")
];
...

The same query is being used above, to check if the parent path is in the database, and if not, then add it.

Now - could leave the enforce in, but do this:

// Perform the database lookup - is the parent in the database?
if (!itemdb.selectByPath(dirName(path), parent.driveId, parent)) {
	// parent is not in the database
	log.vdebug("Parent path is not in the database - need to add it");
	uploadCreateDir(dirName(path));
}
// Is the parent a 'folder' from another user? ie - is this a 'shared folder' that has been shared with us?
if (defaultDriveId == parent.driveId){
	// enforce check of parent path. if the above was triggered, the below will generate a sync retry and will now be sucessful
	enforce(itemdb.selectByPath(dirName(path), parent.driveId, parent), "The parent item id is not in the database");
} else {
	log.vdebug("Parent drive ID is not our drive ID - parent most likely a shared folder");
}

JSONValue driveItem = [
		"name": JSONValue(baseName(path)),
		"folder": parseJSON("{}")
];

This has the same net effect.

@norbusan
Copy link
Collaborator

norbusan commented May 9, 2020

Indeed indeed, that is what I meant. Thanks.

norbusan
norbusan previously approved these changes May 9, 2020
* Update fix to leave enforce in place, but with conditional check on parent drive id.
@abraunegg abraunegg changed the title Remove redundant enforce statement (Issue #912) Update enforce statement for parent id database query (Issue #912) May 9, 2020
@abraunegg abraunegg linked an issue May 9, 2020 that may be closed by this pull request
@abraunegg abraunegg merged commit 2529b99 into master May 9, 2020
@abraunegg abraunegg deleted the Issue-#912 branch May 9, 2020 19:42
@abraunegg abraunegg added this to the v2.4.2 milestone May 25, 2020
@github-actions
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators May 28, 2021
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.

Fails to create a new sub-folder in OneDrive inside a shared folder
2 participants