Skip to content

Commit

Permalink
CURATOR-665: Convey mkdirs exception to create backgorund (#453)
Browse files Browse the repository at this point in the history 
Signed-off-by: tison <[email protected]>
  • Loading branch information
@tisonkun
tisonkun authored Apr 3, 2023
1 parent 2c283bd commit d1b64d6
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 19 deletions.
8 changes: 4 additions & 4 deletions curator-client/src/main/java/org/apache/curator/utils/ZKPaths.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@

import com.google.common.base.Splitter;
import com.google.common.collect.Lists;
import java.util.Collections;
import java.util.List;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.ZooDefs;
import org.apache.zookeeper.ZooKeeper;
import org.apache.zookeeper.data.ACL;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Collections;
import java.util.List;

public class ZKPaths
{
Expand Down Expand Up @@ -228,7 +228,7 @@ public static PathAndNode getPathAndNode(String path)
/**
* Extracts the ten-digit suffix from a sequential znode path. Does not currently perform validation on the
* provided path; it will just return a string comprising the last ten characters.
*
*
* @param path the path of a sequential znodes
* @return the sequential suffix
*/
Expand Down Expand Up @@ -350,7 +350,7 @@ public static void mkdirs(ZooKeeper zookeeper, String path, boolean makeLastNode
}
zookeeper.create(subPath, new byte[0], acl, getCreateMode(asContainers));
}
catch ( KeeperException.NodeExistsException e )
catch (KeeperException.NodeExistsException ignore)
{
// ignore... someone else has created it since we checked
}
Expand Down
40 changes: 32 additions & 8 deletions curator-framework/src/main/java/org/apache/curator/framework/imps/CreateBuilderImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,32 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Predicate;
import com.google.common.collect.Iterables;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.curator.RetryLoop;
import org.apache.curator.drivers.OperationTrace;
import org.apache.curator.framework.api.*;
import org.apache.curator.framework.api.ACLBackgroundPathAndBytesable;
import org.apache.curator.framework.api.ACLCreateModeBackgroundPathAndBytesable;
import org.apache.curator.framework.api.ACLCreateModePathAndBytesable;
import org.apache.curator.framework.api.ACLCreateModeStatBackgroundPathAndBytesable;
import org.apache.curator.framework.api.ACLPathAndBytesable;
import org.apache.curator.framework.api.BackgroundCallback;
import org.apache.curator.framework.api.BackgroundPathAndBytesable;
import org.apache.curator.framework.api.CreateBackgroundModeACLable;
import org.apache.curator.framework.api.CreateBackgroundModeStatACLable;
import org.apache.curator.framework.api.CreateBuilder;
import org.apache.curator.framework.api.CreateBuilder2;
import org.apache.curator.framework.api.CreateBuilderMain;
import org.apache.curator.framework.api.CreateProtectACLCreateModePathAndBytesable;
import org.apache.curator.framework.api.CuratorEvent;
import org.apache.curator.framework.api.CuratorEventType;
import org.apache.curator.framework.api.ErrorListenerPathAndBytesable;
import org.apache.curator.framework.api.PathAndBytesable;
import org.apache.curator.framework.api.ProtectACLCreateModePathAndBytesable;
import org.apache.curator.framework.api.ProtectACLCreateModeStatPathAndBytesable;
import org.apache.curator.framework.api.UnhandledErrorListener;
import org.apache.curator.framework.api.transaction.OperationType;
import org.apache.curator.framework.api.transaction.TransactionCreateBuilder;
import org.apache.curator.framework.api.transaction.TransactionCreateBuilder2;
Expand All @@ -40,10 +63,6 @@
import org.apache.zookeeper.server.DataTree;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicBoolean;

public class CreateBuilderImpl implements CreateBuilder, CreateBuilder2, BackgroundOperation<PathAndBytes>, ErrorListenerPathAndBytesable<String>
{
Expand Down Expand Up @@ -778,6 +797,8 @@ public void performBackgroundOperation(OperationAndData<T> dummy) throws Excepti
{
if ( !client.getZookeeperClient().getRetryPolicy().allowRetry(e) )
{
final CuratorEvent event = makeCuratorEvent(client, e.code().intValue(), e.getPath(), null, e.getPath(), null);
client.processBackgroundOperation(mainOperationAndData, event);
throw e;
}
// otherwise safe to ignore as it will get retried
Expand Down Expand Up @@ -873,12 +894,15 @@ public void performBackgroundOperation(OperationAndData<PathAndBytes> op) throws
}

private void sendBackgroundResponse(int rc, String path, Object ctx, String name, Stat stat, OperationAndData<PathAndBytes> operationAndData)
{
client.processBackgroundOperation(operationAndData, makeCuratorEvent(client, rc, path, ctx, name, stat));
}

private static CuratorEvent makeCuratorEvent(CuratorFrameworkImpl client, int rc, String path, Object ctx, String name, Stat stat)
{
path = client.unfixForNamespace(path);
name = client.unfixForNamespace(name);

CuratorEvent event = new CuratorEventImpl(client, CuratorEventType.CREATE, rc, path, name, ctx, stat, null, null, null, null, null);
client.processBackgroundOperation(operationAndData, event);
return new CuratorEventImpl(client, CuratorEventType.CREATE, rc, path, name, ctx, stat, null, null, null, null, null);
}

private ACLCreateModePathAndBytesable<String> asACLCreateModePathAndBytesable()
Expand Down
78 changes: 71 additions & 7 deletions curator-x-async/src/test/java/org/apache/curator/x/async/modeled/TestModeledFramework.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,32 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import com.google.common.collect.Sets;
import java.math.BigInteger;
import java.security.NoSuchAlgorithmException;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CompletionException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
import org.apache.curator.framework.api.ACLProvider;
import org.apache.curator.framework.schema.Schema;
import org.apache.curator.framework.schema.SchemaSet;
import org.apache.curator.framework.schema.SchemaViolation;
import org.apache.curator.retry.RetryOneTime;
import org.apache.curator.x.async.AsyncCuratorFramework;
import org.apache.curator.x.async.AsyncStage;
import org.apache.curator.x.async.api.CreateOption;
import org.apache.curator.x.async.api.DeleteOption;
import org.apache.curator.x.async.modeled.models.TestModel;
import org.apache.curator.x.async.modeled.models.TestNewerModel;
import org.apache.curator.x.async.modeled.versioned.Versioned;
import org.apache.curator.x.async.modeled.versioned.VersionedModeledFramework;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.ZooDefs;
import org.apache.zookeeper.data.ACL;
Expand All @@ -46,13 +59,6 @@
import org.apache.zookeeper.server.auth.DigestAuthenticationProvider;
import org.junit.jupiter.api.Test;

import java.math.BigInteger;
import java.security.NoSuchAlgorithmException;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CountDownLatch;

public class TestModeledFramework extends TestModeledFrameworkBase
{
@Test
Expand Down Expand Up @@ -222,4 +228,62 @@ public void testAcl() throws NoSuchAlgorithmException
complete(authClient.update(new TestModel("John", "Galt", "Galt's Gulch", 42, BigInteger.valueOf(66))), (__, e) -> assertNull(e, "Should've succeeded"));
}
}

@Test
public void testExceptionHandling() throws Exception
{
final List<ACL> writeAcl = Collections.singletonList(new ACL(ZooDefs.Perms.WRITE, new Id("digest", DigestAuthenticationProvider.generateDigest("test:test"))));

// An ACLProvider is used to get the Write ACL (for the test user) for any path "/test/**".
final ACLProvider aclProvider = new ACLProvider() {
@Override
public List<ACL> getDefaultAcl() { return ZooDefs.Ids.READ_ACL_UNSAFE; }

@Override
public List<ACL> getAclForPath(String path)
{
// Any sub-path "/test/**" should only be writeable by the test user.
return path.startsWith("/test") ? writeAcl : getDefaultAcl();
}
};

try (CuratorFramework authorizedFramework = CuratorFrameworkFactory.builder()
.connectString(server.getConnectString())
.retryPolicy(new RetryOneTime(1))
.aclProvider(aclProvider)
.authorization("digest", "test:test".getBytes())
.build()) {

authorizedFramework.start();

// Create the parent path using the authorized framework, which will initially set the ACL accordingly.
authorizedFramework.create().withMode(CreateMode.PERSISTENT).forPath("/test");
}

// Now attempt to set the sub-node using an unauthorized client.
try (CuratorFramework unauthorizedFramework = CuratorFrameworkFactory.builder()
.connectString(server.getConnectString())
.retryPolicy(new RetryOneTime(1))
.aclProvider(aclProvider)
.build()) {
unauthorizedFramework.start();

// I overrode the TestModel provided path with a multi-component path under the "/test" parent path
// (which was previously created with ACL protection).
ModelSpec<TestModel> aclModelSpec = ModelSpec.builder(ZPath.parse("/test/foo/bar"), modelSpec.serializer())
.withCreateOptions(EnumSet.of(CreateOption.createParentsIfNeeded, CreateOption.createParentsAsContainers))
.build();

ModeledFramework<TestModel> noAuthClient = ModeledFramework.wrap(AsyncCuratorFramework.wrap(unauthorizedFramework), aclModelSpec);

noAuthClient.set(new TestModel("John", "Galt", "Galt's Gulch", 42, BigInteger.valueOf(66)))
.toCompletableFuture()
.get(timing.forWaiting().milliseconds(), TimeUnit.MILLISECONDS);
fail("expect to throw a NoAuth KeeperException");
}
catch (ExecutionException | CompletionException e)
{
assertTrue(e.getCause() instanceof KeeperException.NoAuthException);
}
}
}

0 comments on commit d1b64d6

Please sign in to comment.