Skip to content

Commit 5e08d73

Browse files
vbradnitskivbradnitski
authored
Narrow node api for applyPermissions #11505 (#11507)
1 parent e6707b4 commit 5e08d73

File tree

13 files changed

+92
-101
lines changed

13 files changed

+92
-101
lines changed

‎modules/core/core-api/src/main/java/com/enonic/xp/node/ApplyNodePermissionsResult.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22

33
import java.util.List;
44
import java.util.Map;
5-
import java.util.Objects;
65

76
import com.google.common.collect.ImmutableListMultimap;
87
import com.google.common.collect.ImmutableMap;
98

109
import com.enonic.xp.annotation.PublicApi;
1110
import com.enonic.xp.branch.Branch;
11+
import com.enonic.xp.security.acl.AccessControlList;
1212

1313
@PublicApi
1414
public final class ApplyNodePermissionsResult
@@ -32,18 +32,18 @@ public Map<NodeId, List<BranchResult>> getResults()
3232
return results;
3333
}
3434

35-
public Node getResult( final NodeId nodeId, final Branch branch )
35+
public BranchResult getResult( final NodeId nodeId, final Branch branch )
3636
{
3737
final List<BranchResult> results = this.results.get( nodeId );
3838

3939
return results != null ? this.results.get( nodeId )
40-
.stream().filter( br -> br.branch.equals( branch ) ).map( BranchResult::node )
41-
.filter( Objects::nonNull )
40+
.stream()
41+
.filter( br -> br.branch.equals( branch ) )
4242
.findAny()
4343
.orElse( null ) : null;
4444
}
4545

46-
public record BranchResult(Branch branch, Node node)
46+
public record BranchResult(Branch branch, NodeVersionId nodeVersionId, AccessControlList permissions)
4747
{
4848

4949
}
@@ -56,9 +56,9 @@ private Builder()
5656
{
5757
}
5858

59-
public Builder addResult( NodeId nodeId, Branch branch, Node node )
59+
public Builder addResult( NodeId nodeId, Branch branch, NodeVersionId nodeVersionId, AccessControlList permissions )
6060
{
61-
results.put( nodeId, new BranchResult( branch, node ) );
61+
results.put( nodeId, new BranchResult( branch, nodeVersionId, permissions ) );
6262
return this;
6363
}
6464

‎modules/core/core-content/src/main/java/com/enonic/xp/core/impl/content/ApplyContentPermissionsCommand.java‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
package com.enonic.xp.core.impl.content;
22

3-
import java.util.Collection;
4-
import java.util.Objects;
5-
63
import com.enonic.xp.branch.Branches;
74
import com.enonic.xp.content.ApplyContentPermissionsParams;
85
import com.enonic.xp.content.ApplyContentPermissionsResult;
@@ -35,7 +32,8 @@ ApplyContentPermissionsResult execute()
3532
final NodeId nodeId = NodeId.from( params.getContentId() );
3633

3734
final ApplyNodePermissionsParams.Builder applyNodePermissionsBuilder = ApplyNodePermissionsParams.create()
38-
.nodeId( nodeId ).permissions( params.getPermissions() )
35+
.nodeId( nodeId )
36+
.permissions( params.getPermissions() )
3937
.addPermissions( params.getAddPermissions() )
4038
.removePermissions( params.getRemovePermissions() )
4139
.scope( params.getScope() )
@@ -57,7 +55,7 @@ ApplyContentPermissionsResult execute()
5755
{
5856
if ( branchResult.branch().equals( ContextAccessor.current().getBranch() ) )
5957
{
60-
builder.addResult( ContentId.from( id ), branchResult.node() != null ? branchResult.node().getPermissions() : null );
58+
builder.addResult( ContentId.from( id ), branchResult.permissions() );
6159
break;
6260
}
6361
}
@@ -69,19 +67,19 @@ ApplyContentPermissionsResult execute()
6967
private void commitResult( final ApplyNodePermissionsResult result )
7068
{
7169
final RoutableNodeVersionIds versionIdsToCommit = result.getResults()
72-
.values()
70+
.entrySet()
7371
.stream()
74-
.flatMap( Collection::stream )
75-
.filter( branchResult -> ContentConstants.BRANCH_MASTER.equals( branchResult.branch() ) )
76-
.map( ApplyNodePermissionsResult.BranchResult::node )
77-
.filter( Objects::nonNull )
78-
.map( node -> RoutableNodeVersionId.from( node.id(), node.getNodeVersionId() ) )
72+
.flatMap( entry -> entry.getValue()
73+
.stream()
74+
.filter( br -> ContentConstants.BRANCH_MASTER.equals( br.branch() ) )
75+
.filter( br -> br.nodeVersionId() != null )
76+
.map( br -> RoutableNodeVersionId.from( entry.getKey(), br.nodeVersionId() ) ) )
7977
.collect( RoutableNodeVersionIds.collector() );
8078

8179
if ( !versionIdsToCommit.isEmpty() )
8280
{
83-
nodeService.commit( NodeCommitEntry.create().message( ContentConstants.APPLY_PERMISSIONS_COMMIT_PREFIX )
84-
.build(), versionIdsToCommit );
81+
nodeService.commit( NodeCommitEntry.create().message( ContentConstants.APPLY_PERMISSIONS_COMMIT_PREFIX ).build(),
82+
versionIdsToCommit );
8583
}
8684
}
8785

‎modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/NodeEvents.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.enonic.xp.event.Event;
1111
import com.enonic.xp.node.MoveNodeResult;
1212
import com.enonic.xp.node.Node;
13+
import com.enonic.xp.node.NodeId;
1314
import com.enonic.xp.node.PushNodeResult;
1415

1516
import static com.enonic.xp.event.EventConstants.NODES_FIELD;
@@ -69,9 +70,16 @@ public static Event patched( final Node patchedNode, final InternalContext inter
6970
return event( NODE_UPDATED_EVENT, patchedNode, internalContext );
7071
}
7172

72-
public static Event permissionsUpdated( final Node updatedNode, final InternalContext internalContext )
73+
public static Event permissionsUpdated( final NodeId nodeId, final InternalContext internalContext )
7374
{
74-
return event( NODE_PERMISSIONS_UPDATED, updatedNode, internalContext );
75+
final Event.Builder builder = Event.create( NODE_PERMISSIONS_UPDATED ).distributed( true );
76+
if ( internalContext.getEventMetadata() != null )
77+
{
78+
internalContext.getEventMetadata().forEach( builder::value );
79+
}
80+
builder.value( "id", nodeId );
81+
82+
return builder.build();
7583
}
7684

7785
public static Event moved( final Collection<MoveNodeResult.MovedNode> movedNodes, final InternalContext internalContext )

‎modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/ApplyNodePermissionsCommand.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ private void doApplyOnNode( final Node node, final Branch branch, final AccessCo
184184
appliedVersions.put( node.getNodeVersionId(), updatedSourceNode.nodeVersionMetadata() );
185185
}
186186

187-
results.addResult( node.id(), branch, updatedSourceNode != null ? updatedSourceNode.node() : null );
187+
results.addResult( node.id(), branch, updatedSourceNode != null ? updatedSourceNode.nodeVersionMetadata().getNodeVersionId() : null,
188+
updatedSourceNode != null ? updatedSourceNode.node().getPermissions() : null );
188189
}
189190

190191
private NodeVersionData updatePermissionsInBranch( final NodeId nodeId, final NodeVersionMetadata updatedVersionMetadata,

‎modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/ImportNodeCommand.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import com.enonic.xp.context.ContextAccessor;
77
import com.enonic.xp.context.ContextBuilder;
88
import com.enonic.xp.node.ApplyNodePermissionsParams;
9-
import com.enonic.xp.node.ApplyNodePermissionsResult;
109
import com.enonic.xp.node.BinaryAttachments;
1110
import com.enonic.xp.node.CreateNodeParams;
1211
import com.enonic.xp.node.CreateRootNodeParams;
@@ -159,7 +158,7 @@ private Node updateNode( final Node existingNode )
159158
.stream()
160159
.filter( branchResult -> ContextAccessor.current().getBranch().equals( branchResult.branch() ) )
161160
.findAny()
162-
.map( ApplyNodePermissionsResult.BranchResult::node )
161+
.map( br -> br.permissions() != null ? Node.create( updatedNode ).permissions( br.permissions() ).build() : updatedNode )
163162
.orElse( updatedNode );
164163
}
165164

‎modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/NodeServiceImpl.java‎

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.enonic.xp.repo.impl.node;
22

3-
import java.util.Collection;
43
import java.util.List;
54
import java.util.Map;
65
import java.util.Objects;
@@ -14,7 +13,6 @@
1413

1514
import com.google.common.io.ByteSource;
1615

17-
import com.enonic.xp.node.NodeVersionKey;
1816
import com.enonic.xp.branch.Branch;
1917
import com.enonic.xp.branch.Branches;
2018
import com.enonic.xp.context.Context;
@@ -59,6 +57,7 @@
5957
import com.enonic.xp.node.NodeService;
6058
import com.enonic.xp.node.NodeVersion;
6159
import com.enonic.xp.node.NodeVersionId;
60+
import com.enonic.xp.node.NodeVersionKey;
6261
import com.enonic.xp.node.NodeVersionQuery;
6362
import com.enonic.xp.node.NodeVersionQueryResult;
6463
import com.enonic.xp.node.Nodes;
@@ -692,42 +691,32 @@ public ApplyNodePermissionsResult applyPermissions( final ApplyNodePermissionsPa
692691
.execute();
693692

694693
final Map<NodeId, List<ApplyNodePermissionsResult.BranchResult>> resultsByNodeId = result.getResults()
695-
.values()
694+
.entrySet()
696695
.stream()
697-
.flatMap( Collection::stream )
698-
.filter( br -> br.node() != null )
699-
.collect( Collectors.groupingBy( br -> br.node().id() ) );
700-
701-
for ( Map.Entry<NodeId, List<ApplyNodePermissionsResult.BranchResult>> entry : resultsByNodeId.entrySet() )
696+
.flatMap( entry -> entry.getValue()
697+
.stream()
698+
.filter( br -> br.nodeVersionId() != null )
699+
.collect( Collectors.groupingBy( br -> entry.getKey() ) )
700+
.entrySet()
701+
.stream() )
702+
.collect( Collectors.toMap( Map.Entry::getKey, Map.Entry::getValue ) );
703+
704+
for ( final Map.Entry<NodeId, List<ApplyNodePermissionsResult.BranchResult>> entry : resultsByNodeId.entrySet() )
702705
{
703706
final List<ApplyNodePermissionsResult.BranchResult> branchResults = entry.getValue();
704707

705-
final ApplyNodePermissionsResult.BranchResult mainBranchResult = branchResults.stream()
706-
.filter( br -> ContextAccessor.current().getBranch().equals( br.branch() ) )
707-
.findFirst()
708-
.orElse( null );
709-
710-
final NodeVersionId mainBranchVersion = mainBranchResult != null ? mainBranchResult.node().getNodeVersionId() : null;
711-
712-
for ( ApplyNodePermissionsResult.BranchResult br : branchResults )
708+
for ( final ApplyNodePermissionsResult.BranchResult br : branchResults )
713709
{
714-
if ( br.node() == null )
710+
if ( br.nodeVersionId() == null )
715711
{
716712
continue;
717713
}
718714

719-
ContextBuilder.from( ContextAccessor.current() ).branch( br.branch() ).build().runWith( () -> {
720-
final InternalContext internalContext = InternalContext.from( ContextAccessor.current() );
721-
722-
if ( ( mainBranchResult != null && mainBranchResult.branch().equals( br.branch() ) ) ||
723-
!br.node().getNodeVersionId().equals( mainBranchVersion ) )
724-
{
725-
eventPublisher.publish( NodeEvents.permissionsUpdated( br.node(), internalContext ) );
726-
}
727-
else
728-
{
729-
eventPublisher.publish( NodeEvents.pushed( br.node(), internalContext ) );
730-
}
715+
final Context context = ContextBuilder.from( ContextAccessor.current() ).branch( br.branch() ).build();
716+
717+
context.runWith( () -> {
718+
eventPublisher.publish(
719+
NodeEvents.permissionsUpdated( entry.getKey(), InternalContext.from( ContextAccessor.current() ) ) );
731720
} );
732721
}
733722
}

‎modules/itest/itest-core/src/test/java/com/enonic/xp/core/node/ApplyNodePermissionsCommandTest.java‎

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,10 @@ void with_children()
100100

101101
assertEquals( 3, result.getResults().size() );
102102

103-
assertEquals( result.getResult( createdNode.id(), ContentConstants.BRANCH_DRAFT ),
104-
result.getResult( createdNode.id(), ContentConstants.BRANCH_MASTER ) );
105-
assertEquals( result.getResult( childNode.id(), ContentConstants.BRANCH_DRAFT ),
106-
result.getResult( childNode.id(), ContentConstants.BRANCH_MASTER ) );
103+
assertEquals( result.getResult( createdNode.id(), ContentConstants.BRANCH_DRAFT ).nodeVersionId(),
104+
result.getResult( createdNode.id(), ContentConstants.BRANCH_MASTER ).nodeVersionId() );
105+
assertEquals( result.getResult( childNode.id(), ContentConstants.BRANCH_DRAFT ).nodeVersionId(),
106+
result.getResult( childNode.id(), ContentConstants.BRANCH_MASTER ).nodeVersionId() );
107107

108108
assertEquals( 1, result.getResults().get( grandChildNode.id() ).size() );
109109
}
@@ -136,8 +136,8 @@ void only_children()
136136

137137
assertEquals( 2, result.getResults().size() );
138138

139-
assertEquals( result.getResult( childNode.id(), ContentConstants.BRANCH_DRAFT ),
140-
result.getResult( childNode.id(), ContentConstants.BRANCH_MASTER ) );
139+
assertEquals( result.getResult( childNode.id(), ContentConstants.BRANCH_DRAFT ).nodeVersionId(),
140+
result.getResult( childNode.id(), ContentConstants.BRANCH_MASTER ).nodeVersionId() );
141141

142142
assertEquals( 1, result.getResults().get( grandChildNode.id() ).size() );
143143
}
@@ -175,10 +175,10 @@ void modified()
175175

176176
assertEquals( 2, result.getResults().size() );
177177

178-
assertNotEquals( result.getResult( createdNode.id(), ContentConstants.BRANCH_DRAFT ),
179-
result.getResult( createdNode.id(), ContentConstants.BRANCH_MASTER ) );
180-
assertEquals( result.getResult( childNode.id(), ContentConstants.BRANCH_DRAFT ),
181-
result.getResult( childNode.id(), ContentConstants.BRANCH_MASTER ) );
178+
assertNotEquals( result.getResult( createdNode.id(), ContentConstants.BRANCH_DRAFT ).nodeVersionId(),
179+
result.getResult( createdNode.id(), ContentConstants.BRANCH_MASTER ).nodeVersionId() );
180+
assertEquals( result.getResult( childNode.id(), ContentConstants.BRANCH_DRAFT ).nodeVersionId(),
181+
result.getResult( childNode.id(), ContentConstants.BRANCH_MASTER ).nodeVersionId() );
182182
}
183183

184184
@Test
@@ -213,10 +213,10 @@ void switched()
213213

214214
assertEquals( 2, result.getResults().size() );
215215

216-
assertNotEquals( result.getResult( createdNode.id(), ContentConstants.BRANCH_DRAFT ),
217-
result.getResult( createdNode.id(), ContentConstants.BRANCH_MASTER ) );
218-
assertNotEquals( result.getResult( childNode.id(), ContentConstants.BRANCH_DRAFT ),
219-
result.getResult( childNode.id(), ContentConstants.BRANCH_MASTER ) );
216+
assertNotEquals( result.getResult( createdNode.id(), ContentConstants.BRANCH_DRAFT ).nodeVersionId(),
217+
result.getResult( createdNode.id(), ContentConstants.BRANCH_MASTER ).nodeVersionId() );
218+
assertNotEquals( result.getResult( childNode.id(), ContentConstants.BRANCH_DRAFT ).nodeVersionId(),
219+
result.getResult( childNode.id(), ContentConstants.BRANCH_MASTER ).nodeVersionId() );
220220
}
221221

222222
@Test
@@ -248,10 +248,10 @@ void from_master_to_draft()
248248

249249
assertEquals( 2, result.getResults().size() );
250250

251-
assertEquals( result.getResult( createdNode.id(), ContentConstants.BRANCH_DRAFT ),
252-
result.getResult( createdNode.id(), ContentConstants.BRANCH_MASTER ) );
253-
assertEquals( result.getResult( childNode.id(), ContentConstants.BRANCH_DRAFT ),
254-
result.getResult( childNode.id(), ContentConstants.BRANCH_MASTER ) );
251+
assertEquals( result.getResult( createdNode.id(), ContentConstants.BRANCH_DRAFT ).nodeVersionId(),
252+
result.getResult( createdNode.id(), ContentConstants.BRANCH_MASTER ).nodeVersionId() );
253+
assertEquals( result.getResult( childNode.id(), ContentConstants.BRANCH_DRAFT ).nodeVersionId(),
254+
result.getResult( childNode.id(), ContentConstants.BRANCH_MASTER ).nodeVersionId() );
255255
}
256256

257257
@Test
@@ -309,7 +309,7 @@ void has_no_write_permissions()
309309
verify( listener, times( 1 ) ).notEnoughRights( 1 );
310310

311311
assertEquals( 1, result.getResults().size() );
312-
assertNull( result.getResult( createdNode.id(), ContentConstants.BRANCH_DRAFT ) );
312+
assertNull( result.getResult( createdNode.id(), ContentConstants.BRANCH_DRAFT ).permissions() );
313313
}
314314

315315
@Test
@@ -341,12 +341,10 @@ void add_and_remove()
341341
.build() );
342342

343343
assertEquals( 1, result.getResults().size() );
344-
assertFalse( result.getResults().get( createdNode.id() ).get( 0 ).node().getPermissions().isAllowedFor( principal, READ ) );
344+
assertFalse( result.getResults().get( createdNode.id() ).get( 0 ).permissions().isAllowedFor( principal, READ ) );
345345
assertTrue( result.getResults()
346346
.get( createdNode.id() )
347-
.get( 0 )
348-
.node()
349-
.getPermissions()
347+
.get( 0 ).permissions()
350348
.isAllowedFor( principal, MODIFY, DELETE, CREATE, PUBLISH, WRITE_PERMISSIONS, READ_PERMISSIONS ) );
351349

352350
}
@@ -386,13 +384,10 @@ void add_to_existing()
386384
.build() );
387385

388386
assertEquals( 1, result.getResults().size() );
389-
assertTrue(
390-
result.getResults().get( createdNode.id() ).get( 0 ).node().getPermissions().isAllowedFor( principal, READ, MODIFY ) );
387+
assertTrue( result.getResults().get( createdNode.id() ).get( 0 ).permissions().isAllowedFor( principal, READ, MODIFY ) );
391388
assertFalse( result.getResults()
392389
.get( createdNode.id() )
393-
.get( 0 )
394-
.node()
395-
.getPermissions()
390+
.get( 0 ).permissions()
396391
.isAllowedFor( principal, DELETE, CREATE, PUBLISH, WRITE_PERMISSIONS, READ_PERMISSIONS ) );
397392

398393
}
@@ -426,7 +421,7 @@ void remove_all_on_empty()
426421
.build() );
427422

428423
assertEquals( 1, result.getResults().size() );
429-
assertFalse( result.getResults().get( createdNode.id() ).get( 0 ).node().getPermissions().contains( principal ) );
424+
assertFalse( result.getResults().get( createdNode.id() ).get( 0 ).permissions().contains( principal ) );
430425

431426
}
432427

@@ -446,7 +441,7 @@ void set_empty_permissions()
446441
.build() );
447442

448443
assertEquals( 1, result.getResults().size() );
449-
assertTrue( result.getResults().get( createdNode.id() ).get( 0 ).node().getPermissions().isEmpty() );
444+
assertTrue( result.getResults().get( createdNode.id() ).get( 0 ).permissions().isEmpty() );
450445
}
451446

452447
private void applyPermissionsWithOverwrite()

‎modules/itest/itest-core/src/test/java/com/enonic/xp/core/node/NodeServiceImplTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,9 +514,8 @@ void applyPermissions_shouldPublishCorrectEvents()
514514

515515
List<Event> publishedEvents = captor.getAllValues();
516516

517-
assertEquals( 1,
517+
assertEquals( 2,
518518
publishedEvents.stream().filter( event -> event.getType().equals( NodeEvents.NODE_PERMISSIONS_UPDATED ) ).count() );
519-
assertEquals( 1, publishedEvents.stream().filter( event -> event.getType().equals( NodeEvents.NODE_PUSHED_EVENT ) ).count() );
520519
}
521520

522521
@Test

‎modules/lib/lib-content/src/main/java/com/enonic/xp/lib/content/CreateContentHandler.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,9 @@ private CreateContentParams createParams()
7272
displayName( this.displayName ).
7373
requireValid( this.requireValid ).
7474
type( contentTypeName ).
75-
contentData( createPropertyTree( data, contentTypeName ) ).
76-
extraDatas( createExtraDatas( x, contentTypeName ) ).page( createPage( page ) ).
75+
contentData( createPropertyTree( data, contentTypeName ) ).extraDatas( createExtraDatas( x, contentTypeName ) )
76+
.page( createPage( page ) )
77+
.
7778
language( language != null ? Locale.forLanguageTag( language ) : null ).
7879
childOrder( childOrder != null ? ChildOrder.from( childOrder ) : null ).
7980
refresh( this.refresh ).

0 commit comments

Comments
 (0)