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.

Sign up for GitHub

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

Enh: More targeted drive prop set #4211

Open
wants to merge 6 commits into
base: synapse-3xx
Choose a base branch
from
Open

Enh: More targeted drive prop set #4211

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1cb14a0
wip
rakuy0 Mar 27, 2025
94165d5
wip
rakuy0 Mar 27, 2025
fb74cc3
back that out
rakuy0 Mar 28, 2025
a4926c2
coverage
rakuy0 Mar 31, 2025
8fba48a
more coverage
rakuy0 Mar 31, 2025
59c1e3a
add versinfo set for setDriveItemProp
rakuy0 Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion synapse/lib/cell.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1685,7 +1685,7 @@ async def addDrivePath(self, path, perm=None, reldir=s_drive.rootdir):

info = {
'name': name,
'perm': perm,
'permissions': perm,
'iden': s_common.guid(),
'created': tick,
'creator': user,
Expand Down Expand Up @@ -1715,6 +1715,17 @@ async def delDriveInfo(self, iden):
async def setDriveInfoPerm(self, iden, perm):
return self.drive.setItemPerm(iden, perm)

@s_nexus.Pusher.onPushAuto('drive:data:path:set')
async def setDriveItemProp(self, iden, path, valu):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would we want the ability to update the version with this call?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say yea. Otherwise you have to do another kinda awkward call to setItemData since there's not really a way to set just the version info. I'll add that param 👍

if isinstance(path, str):
path = (path,)
vers, item = await self.getDriveData(iden)
step = item
for p in path[:-1]:
step = step[p]
step[path[-1]] = valu
return await self.drive.setItemData(iden, vers, item)

@s_nexus.Pusher.onPushAuto('drive:set:path')
async def setDriveInfoPath(self, iden, path):

Expand Down
7 changes: 5 additions & 2 deletions synapse/lib/drive.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ def _reqInfoType(self, info, typename):

def getItemInfo(self, iden, typename=None):
info = self._getItemInfo(s_common.uhex(iden))
if not info:
Copy link
Contributor Author

@rakuy0 rakuy0 Mar 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you supply a typename and the drive doesn't have that iden, you get an unhandled python Attribute error about NoneType not having a get attribute (instead of this method just returning None like it does when the iden doesn't map to any data)

return

if typename is not None:
self._reqInfoType(info, typename)
return info
Expand Down Expand Up @@ -210,7 +213,7 @@ def setItemPerm(self, iden, perm):

def _setItemPerm(self, bidn, perm):
info = self._reqItemInfo(bidn)
info['perm'] = perm
info['permissions'] = perm
s_schemas.reqValidDriveInfo(info)
self.slab.put(LKEY_INFO + bidn, s_msgpack.en(info), db=self.dbname)
return info
Expand Down Expand Up @@ -286,7 +289,7 @@ async def addItemInfo(self, info, path=None, reldir=rootdir):
info['kids'] = 0
info['parent'] = pariden

info.setdefault('perm', {'users': {}, 'roles': {}})
info.setdefault('permissions', {'users': {}, 'roles': {}})
info.setdefault('version', (0, 0, 0))

s_schemas.reqValidDriveInfo(info)
Expand Down
4 changes: 3 additions & 1 deletion synapse/lib/schemas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
'type': 'object',
'items': {'type': 'number', 'minimum': 0, 'maximum': 3},
},
'default': {'type': 'number', 'minimum': 0, 'maximum': 3}
},
'required': ['users', 'roles'],
}
Expand Down Expand Up @@ -483,7 +484,8 @@
'parent': {'type': 'string', 'pattern': s_config.re_iden},
'type': {'type': 'string', 'pattern': re_drivename},
'name': {'type': 'string', 'pattern': re_drivename},
'perm': s_msgpack.deepcopy(easyPermSchema),
# TODO: This used to be called `perm`, so migrate this as part of the big migration from 2.x
'permissions': s_msgpack.deepcopy(easyPermSchema),
'kids': {'type': 'number', 'minimum': 0},
'created': {'type': 'number'},
'creator': {'type': 'string', 'pattern': s_config.re_iden},
Expand Down
19 changes: 17 additions & 2 deletions synapse/tests/test_lib_cell.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ async def test_cell_drive(self):

tick = s_common.now()
rootuser = cell.auth.rootuser.iden
fooser = await cell.auth.addUser('foo')
neatrole = await cell.auth.addRole('neatrole')
await fooser.grant(neatrole.iden)

with self.raises(s_exc.SchemaViolation):
versinfo = {'version': (1, 0, 0), 'updated': tick, 'updater': rootuser}
Expand Down Expand Up @@ -266,13 +269,21 @@ async def test_cell_drive(self):
info, versinfo = await cell.setDriveData(iden, versinfo, {'type': 'haha', 'size': 17, 'stuff': 15})
self.eq(versinfo, (await cell.getDriveData(iden))[0])

await cell.setDriveItemProp(iden, ('stuff',), 1234)
data = await cell.getDriveData(iden)
self.eq(data[1]['stuff'], 1234)

# This will be done by the cell in a cell storage version migration...
async def migrate_v1(info, versinfo, data):
data['woot'] = 'woot'
return data

await cell.drive.setTypeSchema('woot', testDataSchema_v1, migrate_v1)

await cell.setDriveItemProp(iden, 'stuff', 3829)
data = await cell.getDriveData(iden)
self.eq(data[1]['stuff'], 3829)

versinfo, data = await cell.getDriveData(iden, vers=(1, 0, 0))
self.eq('woot', data.get('woot'))

Expand Down Expand Up @@ -313,8 +324,12 @@ async def migrate_v1(info, versinfo, data):
baziden = pathinfo[2].get('iden')
self.eq(pathinfo, await cell.drive.getItemPath(baziden))

info = await cell.setDriveInfoPerm(baziden, {'users': {rootuser: 3}, 'roles': {}})
self.eq(3, info['perm']['users'][rootuser])
info = await cell.setDriveInfoPerm(baziden, {'users': {rootuser: s_cell.PERM_ADMIN}, 'roles': {}})
# make sure drive perms work with easy perms
self.true(cell._hasEasyPerm(info, cell.auth.rootuser, s_cell.PERM_ADMIN))
# defaults to READ
self.true(cell._hasEasyPerm(info, fooser, s_cell.PERM_READ))
self.false(cell._hasEasyPerm(info, fooser, s_cell.PERM_EDIT))

with self.raises(s_exc.NoSuchIden):
# s_drive.rootdir is all 00s... ;)
Expand Down