Allow merge requests on Views with forks (SYN-7401) (#3738)

synapse/cortex.py

@@ -4496,6 +4496,87 @@ async def _addView(self, vdef):
         await self.feedBeholder('view:add', pack, gates=[iden])
         return pack
 
+    async def delViewWithLayer(self, iden):
+        '''
+        Delete a Cortex View and its write Layer if not in use by other View stacks.
+
+        Note:
+            Any children of the View will have their parent View updated to
+            the deleted View's parent (if present). The deleted View's write Layer
+            will also be removed from any child Views which contain it in their
+            Layer stack. If the Layer is used in Views which are not children of
+            the deleted View, the Layer will be preserved, otherwise it will be
+            deleted as well.
+        '''
+        view = self.views.get(iden)
+        if view is None:
+            raise s_exc.NoSuchView(mesg=f'No such view {iden=}', iden=iden)
+
+        if view.info.get('protected'):
+            mesg = f'Cannot delete view ({iden}) that has protected set.'
+            raise s_exc.CantDelView(mesg=mesg)
+
+        layriden = view.layers[0].iden
+        pareiden = None
+        if view.parent is not None:
+            pareiden = view.parent.iden
+
+        return await self._push('view:delwithlayer', iden, layriden, newparent=pareiden)
+
+    @s_nexus.Pusher.onPush('view:delwithlayer', passitem=True)
+    async def _delViewWithLayer(self, viewiden, layriden, nexsitem, newparent=None):
+
+        if viewiden == self.view.iden:
+            raise s_exc.SynErr(mesg='Cannot delete the main view')
+
+        if (view := self.views.get(viewiden)) is not None:
+
+            await self.hive.pop(('cortex', 'views', viewiden))
+            await view.delete()
+
+            self._calcViewsByLayer()
+            await self.feedBeholder('view:del', {'iden': viewiden}, gates=[viewiden])
+            await self.auth.delAuthGate(viewiden)
+
+        if newparent is not None:
+            newview = self.views.get(newparent)
+
+        layrinuse = False
+        for view in self.viewsbylayer[layriden]:
+            if not view.isForkOf(viewiden):
+                layrinuse = True
+                continue
+
+            view.layers = [lyr for lyr in view.layers if lyr.iden != layriden]
+            await view.info.set('layers', [lyr.iden for lyr in view.layers])
+
+            if view.parent.iden == viewiden:
+                if newparent is None:
+                    view.parent = None
+                    await view.info.pop('parent')
+                else:
+                    view.parent = newview
+                    await view.info.set('parent', newparent)
+
+        if not layrinuse and (layr := self.layers.get(layriden)) is not None:
+            del self.layers[layriden]
+
+            for pdef in layr.layrinfo.get('pushs', {}).values():
+                await self.delActiveCoro(pdef.get('iden'))
+
+            for pdef in layr.layrinfo.get('pulls', {}).values():
+                await self.delActiveCoro(pdef.get('iden'))
+
+            await self.feedBeholder('layer:del', {'iden': layriden}, gates=[layriden])
+            await self.auth.delAuthGate(layriden)
+            self.dynitems.pop(layriden)
+
+            await self.hive.pop(('cortex', 'layers', layriden))
+
+            await layr.delete()
+
+            layr.deloffs = nexsitem[0]
+
     async def delView(self, iden):
         view = self.views.get(iden)
         if view is None:

synapse/lib/view.py

@@ -196,10 +196,6 @@ def hasKids(self):
     async def _setMergeRequest(self, mergeinfo):
         self.reqParentQuorum()
 
-        if self.hasKids():
-            mesg = 'Cannot add a merge request to a view with children.'
-            raise s_exc.BadState(mesg=mesg)
-
         s_schemas.reqValidMerge(mergeinfo)
         lkey = self.bidn + b'merge:req'
         self.core.slab.put(lkey, s_msgpack.en(mergeinfo), db='view:meta')
@@ -450,8 +446,7 @@ async def chunked():
             await self.core.feedBeholder('view:merge:fini', {'view': self.iden, 'merge': merge, 'merge': merge, 'votes': votes})
 
             # remove the view and top layer
-            await self.core.delView(self.iden)
-            await self.core.delLayer(self.layers[0].iden)
+            await self.core.delViewWithLayer(self.iden)
 
         except Exception as e: # pragma: no cover
             logger.exception(f'Error while merging view: {self.iden}')
@@ -1112,10 +1107,6 @@ async def setViewInfo(self, name, valu):
                 mesg = 'Circular dependency of view parents is not supported.'
                 raise s_exc.BadArg(mesg=mesg)
 
-            if parent.getMergeRequest() is not None:
-                mesg = 'You may not set the parent to a view with a pending merge request.'
-                raise s_exc.BadState(mesg=mesg)
-
             if self.parent is not None:
                 if self.parent.iden == parent.iden:
                     return valu
@@ -1299,6 +1290,9 @@ async def _insertParentFork(self, ldef, vdef, nexsitem):
         s_layer.reqValidLdef(ldef)
         s_schemas.reqValidView(vdef)
 
+        if self.getMergeRequest() is not None:
+            await self._delMergeRequest()
+
         await self.core._addLayer(ldef, nexsitem)
         await self.core._addView(vdef)
 
@@ -1355,10 +1349,6 @@ async def fork(self, ldef=None, vdef=None):
         if vdef is None:
             vdef = {}
 
-        if self.getMergeRequest() is not None:
-            mesg = 'Cannot fork a view which has a merge request.'
-            raise s_exc.BadState(mesg=mesg)
-
         ldef = await self.core.addLayer(ldef)
         layriden = ldef.get('iden')
 

synapse/tests/test_cortex.py

@@ -6292,12 +6292,14 @@ async def test_cortex_delLayerView(self):
 
             # Can't delete the default view
             await self.asyncraises(s_exc.SynErr, core.delView(core.view.iden))
+            await self.asyncraises(s_exc.SynErr, core._delViewWithLayer(core.view.iden, None, None))
 
             # Can't delete a layer in a view
             await self.asyncraises(s_exc.SynErr, core.delLayer(core.view.layers[0].iden))
 
             # Can't delete a nonexistent view
             await self.asyncraises(s_exc.NoSuchView, core.delView('XXX'))
+            await self.asyncraises(s_exc.NoSuchView, core.delViewWithLayer('XXX'))
 
             # Can't delete a nonexistent layer
             await self.asyncraises(s_exc.NoSuchLayer, core.delLayer('XXX'))
@@ -6310,6 +6312,68 @@ async def test_cortex_delLayerView(self):
             await core.delView(view2_iden)
             await self.asyncraises(s_exc.NoSuchView, core.delView(view2_iden))
 
+            layr = await core.addLayer()
+            layriden = layr['iden']
+            vdef3 = {'layers': (layriden,)}
+            view3_iden = (await core.addView(vdef3)).get('iden')
+
+            opts = {'view': view3_iden}
+            await core.callStorm('$lib.view.get().set(protected, $lib.true)', opts=opts)
+
+            await self.asyncraises(s_exc.CantDelView, core.delViewWithLayer(view3_iden))
+
+            await core.callStorm('$lib.view.get().set(protected, $lib.false)', opts=opts)
+
+            view3 = core.getView(view3_iden)
+            vdef4 = await view3.fork()
+
+            deadlayr = view3.layers[0].iden
+            view4_iden = vdef4.get('iden')
+            view4 = core.getView(view4_iden)
+
+            self.eq(view4.parent, view3)
+            self.len(2, view4.layers)
+
+            await core.auth.rootuser.setPasswd('secret')
+            host, port = await core.dmon.listen('tcp://127.0.0.1:0/')
+            layr2 = await core.callStorm('$layer=$lib.layer.add() return($layer)')
+            varz = {'iden': layriden, 'tgt': layr2.get('iden'), 'port': port}
+            opts = {'vars': varz, 'view': view3_iden}
+
+            pullq = '$layer=$lib.layer.get($iden).addPull(`tcp://root:[email protected]:{$port}/*/layer/{$tgt}`)'
+            pushq = '$layer=$lib.layer.get($iden).addPush(`tcp://root:[email protected]:{$port}/*/layer/{$tgt}`)'
+            msgs = await core.stormlist(pullq, opts=opts)
+            self.stormHasNoWarnErr(msgs)
+
+            msgs = await core.stormlist(pushq, opts=opts)
+            self.stormHasNoWarnErr(msgs)
+
+            coros = len(core.activecoros)
+
+            await core.delViewWithLayer(view3_iden)
+
+            # push/pull activecoros have been deleted
+            self.len(coros - 2, core.activecoros)
+
+            self.none(view4.parent)
+            self.len(1, view4.layers)
+            self.none(core.getLayer(deadlayr))
+
+            vdef5 = await view4.fork()
+            view5 = core.getView(vdef5.get('iden'))
+
+            usedlayr = view4.layers[0].iden
+            vdef6 = {'layers': (usedlayr,)}
+            view6 = core.getView((await core.addView(vdef6)).get('iden'))
+
+            await core.delViewWithLayer(view4_iden)
+
+            self.none(view5.parent)
+            self.len(1, view5.layers)
+
+            self.nn(core.getLayer(usedlayr))
+            self.eq([usedlayr], [lyr.iden for lyr in view6.layers])
+
     async def test_cortex_view_opts(self):
         '''
         Test that the view opts work

synapse/tests/test_lib_stormtypes.py

@@ -6704,11 +6704,10 @@ async def test_view_quorum(self):
             self.nn(await core.callStorm('return($lib.view.get().setMergeRequest())', opts=opts))
             self.eq([fork.iden], await core.callStorm(merging))
 
-            # confirm that you may not re-parent to a view with a merge request
+            # confirm that you may re-parent to a view with a merge request
             layr = await core.addLayer()
             vdef = await core.addView({'layers': (layr['iden'],)})
-            with self.raises(s_exc.BadState):
-                await core.getView(vdef['iden']).setViewInfo('parent', fork.iden)
+            await core.getView(vdef['iden']).setViewInfo('parent', fork.iden)
 
             opts = {'view': fork.iden, 'user': visi.iden}
             self.nn(await core.callStorm('return($lib.view.get().setMergeVote(approved=(false)))', opts=opts))
@@ -6758,28 +6757,42 @@ async def test_view_quorum(self):
 
             self.eq([], await core.callStorm(merging))
 
-            # test coverage for bad state for merge request
+            # merge a view with a fork
             fork00 = await core.getView().fork()
-            fork01 = await core.getView(fork00['iden']).fork()
+            midfork = core.getView(fork00['iden'])
 
-            opts = {'view': fork00['iden']}
-            with self.raises(s_exc.BadState):
-                await core.callStorm('return($lib.view.get().setMergeRequest())', opts=opts)
+            fork01 = await midfork.fork()
+            self.true(midfork.hasKids())
+
+            opts = {'view': midfork.iden}
+            await core.callStorm('return($lib.view.get().setMergeRequest())', opts=opts)
+
+            self.eq([midfork.iden], await core.callStorm(merging))
+
+            opts = {'view': midfork.iden, 'user': visi.iden}
+            await core.callStorm('return($lib.view.get().setMergeVote())', opts=opts)
+
+            self.true(await midfork.waitfini(timeout=12))
 
             self.eq([], await core.callStorm(merging))
 
-            await core.delView(fork01['iden'])
+            leaffork = core.getView(fork01['iden'])
+            self.false(leaffork.hasKids())
+
+            self.len(2, leaffork.layers)
+            self.eq(leaffork.parent, core.getView())
+
+            # test coverage for bad state for merge request
+            opts = {'view': fork01['iden']}
             await core.callStorm('return($lib.view.get().setMergeRequest())', opts=opts)
-            self.eq([fork00['iden']], await core.callStorm(merging))
+            self.eq([fork01['iden']], await core.callStorm(merging))
 
             core.getView().layers[0].readonly = True
             with self.raises(s_exc.BadState):
                 await core.callStorm('return($lib.view.get().setMergeRequest())', opts=opts)
 
             core.getView().layers[0].readonly = False
-
-            with self.raises(s_exc.BadState):
-                await core.callStorm('return($lib.view.get().fork())', opts=opts)
+            await core.callStorm('return($lib.view.get().fork())', opts=opts)
 
             # setup a new merge and make a mirror...
             forkdef = await core.getView().fork()
@@ -6789,7 +6802,7 @@ async def test_view_quorum(self):
             await core.stormlist('[ inet:ipv4=5.5.5.5 ]', opts=opts)
             await core.callStorm('return($lib.view.get().setMergeRequest())', opts=opts)
 
-            self.eq(set([fork.iden, fork00['iden']]), set(await core.callStorm(merging)))
+            self.eq(set([fork.iden, fork01['iden']]), set(await core.callStorm(merging)))
 
             # hamstring the runViewMerge method on the new view
             async def fake():

synapse/tests/test_lib_view.py

@@ -859,10 +859,20 @@ async def test_view_insert_parent_fork(self):
             msgs = await core.stormlist('auth.user.addrule visi node --gate $lib.view.get().layers.0.iden')
             self.stormHasNoWarnErr(msgs)
 
+            opts = {'vars': {'role': role.iden}}
+            quorum = await core.callStorm('return($lib.view.get().set(quorum, ({"count": 1, "roles": [$role]})))', opts=opts)
+
             forkopts = {'view': view02.iden}
+            await core.callStorm('return($lib.view.get().setMergeRequest(comment=woot))', opts=forkopts)
+
+            merging = 'return($lib.view.get().getMergingViews()) '
+            self.eq([view02.iden], await core.callStorm(merging))
+
             q = 'return($lib.view.get().insertParentFork(name=staging).iden)'
             newiden = await core.callStorm(q, opts=forkopts)
 
+            self.eq([], await core.callStorm(merging))
+
             view01 = core.getView(newiden)
 
             self.ne(view02.parent, view00)

synapse/tests/test_model_inet.py

@@ -1484,7 +1484,6 @@ async def test_url(self):
             '''
             nodes = await core.nodes(q)
             self.len(7, nodes)
-            print(nodes)
             self.eq(nodes[0].get('base'), 'http://[fedc:ba98:7654:3210:fedc:ba98:7654:3210]:80/index.html')
             self.eq(nodes[0].get('proto'), 'http')
             self.eq(nodes[0].get('path'), '/index.html')