Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0
    • Component/s: Client, Framework
    • Labels:
      None

      Description

      ZooKeeper 3.5 adds async multi/transaction APIs. Add support in Curator.

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user Randgalt opened a pull request:

        https://github.com/apache/curator/pull/77

        CURATOR-215 Support async multi/transaction APIs

        I took this opportunity to re-write the transaction APIs. The old ones were far too clever and hard to use (though pretty!). At the same time, added support for ZK 3.5's async multi() APIs. Old Curator transaction APIs have been deprecated. This uses the old transactions internal code for consistency. However, this makes the implementation a bit cumbersome. In the future, this can all be cleaned up

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/apache/curator CURATOR-215

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/curator/pull/77.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #77


        commit b8dd4b001824682acbc79f11896256479753b910
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-08T16:50:21Z

        Revert "Merge branch 'CURATOR-186' of https://github.com/xoom/curator into CURATOR-186"

        This reverts commit de23cae9e5dcb2681f2a0934c2fbb8fd7570ff0b, reversing
        changes made to 6a56c5145bf800498b4f92c0d94eaecf1941bbed.

        commit ba2b20a439a41f6da7227ab92157c11dd9152c25
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-08T18:43:18Z

        Incorporate Clirr plugin

        commit 2cb030d035f09a23b25d2ff65118764a3ee1723b
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-08T18:46:00Z

        Back to 2.8.0

        commit 1ef4df29f97a3956c65e6e2e1b08ed7a033e4fc5
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-08T18:50:34Z

        Need to skip Curator RPC for Clirr as it can't handle shaded JAR

        commit c1466e6d8312e321732f06831ee9420b1af6344a
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-08T18:57:02Z

        [maven-release-plugin] prepare release apache-curator-2.8.0

        commit 121efdb5008f6f5d8604ab0e44588f9576766a2e
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-08T18:57:12Z

        [maven-release-plugin] prepare for next development iteration

        commit 8d8fdf61e92781806d4633607e75f54223dff8fa
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-08T19:17:16Z

        Don't show RPC report

        commit 7f2098654a26e2f593801a586ce68300f54abf15
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-09T14:26:38Z

        closes #53 Moved to PR 76

        commit 3016ce2f49eb90cc01f9e3d41b94594853416965
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-09T15:40:38Z

        Merge branch 'master' into CURATOR-3.0

        commit 9fd56ce396e03347dd44e6498cd37c9f1ed16be0
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-09T17:39:14Z

        Initial work on updated transaction APIs

        commit 2fb54d1a1f12a91d9bab708b4c55e8848156c88a
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-09T19:57:28Z

        Initial implementation finished. Uses old transactions internal code for consistency. However, this makes the implementation a bit cumbersome. Old APIs are deprecated. In the future, this can all be cleaned up

        commit 22bd58a64e2f80bead93425d2387389d517a8a9b
        Author: randgalt <randgalt@apache.org>
        Date: 2015-05-09T20:01:20Z

        added doc


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user Randgalt opened a pull request: https://github.com/apache/curator/pull/77 CURATOR-215 Support async multi/transaction APIs I took this opportunity to re-write the transaction APIs. The old ones were far too clever and hard to use (though pretty!). At the same time, added support for ZK 3.5's async multi() APIs. Old Curator transaction APIs have been deprecated. This uses the old transactions internal code for consistency. However, this makes the implementation a bit cumbersome. In the future, this can all be cleaned up You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/curator CURATOR-215 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/curator/pull/77.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #77 commit b8dd4b001824682acbc79f11896256479753b910 Author: randgalt <randgalt@apache.org> Date: 2015-05-08T16:50:21Z Revert "Merge branch ' CURATOR-186 ' of https://github.com/xoom/curator into CURATOR-186 " This reverts commit de23cae9e5dcb2681f2a0934c2fbb8fd7570ff0b, reversing changes made to 6a56c5145bf800498b4f92c0d94eaecf1941bbed. commit ba2b20a439a41f6da7227ab92157c11dd9152c25 Author: randgalt <randgalt@apache.org> Date: 2015-05-08T18:43:18Z Incorporate Clirr plugin commit 2cb030d035f09a23b25d2ff65118764a3ee1723b Author: randgalt <randgalt@apache.org> Date: 2015-05-08T18:46:00Z Back to 2.8.0 commit 1ef4df29f97a3956c65e6e2e1b08ed7a033e4fc5 Author: randgalt <randgalt@apache.org> Date: 2015-05-08T18:50:34Z Need to skip Curator RPC for Clirr as it can't handle shaded JAR commit c1466e6d8312e321732f06831ee9420b1af6344a Author: randgalt <randgalt@apache.org> Date: 2015-05-08T18:57:02Z [maven-release-plugin] prepare release apache-curator-2.8.0 commit 121efdb5008f6f5d8604ab0e44588f9576766a2e Author: randgalt <randgalt@apache.org> Date: 2015-05-08T18:57:12Z [maven-release-plugin] prepare for next development iteration commit 8d8fdf61e92781806d4633607e75f54223dff8fa Author: randgalt <randgalt@apache.org> Date: 2015-05-08T19:17:16Z Don't show RPC report commit 7f2098654a26e2f593801a586ce68300f54abf15 Author: randgalt <randgalt@apache.org> Date: 2015-05-09T14:26:38Z closes #53 Moved to PR 76 commit 3016ce2f49eb90cc01f9e3d41b94594853416965 Author: randgalt <randgalt@apache.org> Date: 2015-05-09T15:40:38Z Merge branch 'master' into CURATOR-3 .0 commit 9fd56ce396e03347dd44e6498cd37c9f1ed16be0 Author: randgalt <randgalt@apache.org> Date: 2015-05-09T17:39:14Z Initial work on updated transaction APIs commit 2fb54d1a1f12a91d9bab708b4c55e8848156c88a Author: randgalt <randgalt@apache.org> Date: 2015-05-09T19:57:28Z Initial implementation finished. Uses old transactions internal code for consistency. However, this makes the implementation a bit cumbersome. Old APIs are deprecated. In the future, this can all be cleaned up commit 22bd58a64e2f80bead93425d2387389d517a8a9b Author: randgalt <randgalt@apache.org> Date: 2015-05-09T20:01:20Z added doc
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30149971

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/transaction/CuratorMultiTransactionMain.java —
        @@ -0,0 +1,45 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.api.transaction;
        +
        +import org.apache.curator.framework.CuratorFramework;
        +import java.util.List;
        +
        +public interface CuratorMultiTransactionMain
        +{
        + /**
        + * Commit the given operations as a single transaction. Create the
        + * operation instances via

        {@link CuratorFramework#transactionOp()}
        + *
        + * @param operations operations that make up the transaction.
        + * @return result details
        + * @throws Exception errors
        + */
        + List<CuratorTransactionResult> forOperations(CuratorOp... operations) throws Exception;
        +
        + /**
        + * Commit the given operations as a single transaction. Create the
        + * operation instances via {@link CuratorFramework#transactionOp()}

        + *
        + * @param operations operations that make up the transaction.
        + * @return result details
        + * @throws Exception errors
        + */
        + List<CuratorTransactionResult> forOperations(List<CuratorOp> operations) throws Exception;
        — End diff –

        Be more flexible, accept an Iterable or Collection instead of a List, maybe?

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30149971 — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/transaction/CuratorMultiTransactionMain.java — @@ -0,0 +1,45 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.api.transaction; + +import org.apache.curator.framework.CuratorFramework; +import java.util.List; + +public interface CuratorMultiTransactionMain +{ + /** + * Commit the given operations as a single transaction. Create the + * operation instances via {@link CuratorFramework#transactionOp()} + * + * @param operations operations that make up the transaction. + * @return result details + * @throws Exception errors + */ + List<CuratorTransactionResult> forOperations(CuratorOp... operations) throws Exception; + + /** + * Commit the given operations as a single transaction. Create the + * operation instances via {@link CuratorFramework#transactionOp()} + * + * @param operations operations that make up the transaction. + * @return result details + * @throws Exception errors + */ + List<CuratorTransactionResult> forOperations(List<CuratorOp> operations) throws Exception; — End diff – Be more flexible, accept an Iterable or Collection instead of a List, maybe?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30150106

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/transaction/CuratorMultiTransactionMain.java —
        @@ -0,0 +1,45 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.api.transaction;
        +
        +import org.apache.curator.framework.CuratorFramework;
        +import java.util.List;
        +
        +public interface CuratorMultiTransactionMain
        +{
        + /**
        + * Commit the given operations as a single transaction. Create the
        + * operation instances via

        {@link CuratorFramework#transactionOp()}
        + *
        + * @param operations operations that make up the transaction.
        + * @return result details
        + * @throws Exception errors
        + */
        + List<CuratorTransactionResult> forOperations(CuratorOp... operations) throws Exception;
        +
        + /**
        + * Commit the given operations as a single transaction. Create the
        + * operation instances via {@link CuratorFramework#transactionOp()}

        + *
        + * @param operations operations that make up the transaction.
        + * @return result details
        + * @throws Exception errors
        + */
        + List<CuratorTransactionResult> forOperations(List<CuratorOp> operations) throws Exception;
        — End diff –

        I think order matters here. Thus, the list.

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30150106 — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/transaction/CuratorMultiTransactionMain.java — @@ -0,0 +1,45 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.api.transaction; + +import org.apache.curator.framework.CuratorFramework; +import java.util.List; + +public interface CuratorMultiTransactionMain +{ + /** + * Commit the given operations as a single transaction. Create the + * operation instances via {@link CuratorFramework#transactionOp()} + * + * @param operations operations that make up the transaction. + * @return result details + * @throws Exception errors + */ + List<CuratorTransactionResult> forOperations(CuratorOp... operations) throws Exception; + + /** + * Commit the given operations as a single transaction. Create the + * operation instances via {@link CuratorFramework#transactionOp()} + * + * @param operations operations that make up the transaction. + * @return result details + * @throws Exception errors + */ + List<CuratorTransactionResult> forOperations(List<CuratorOp> operations) throws Exception; — End diff – I think order matters here. Thus, the list.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30150375

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java —
        @@ -0,0 +1,166 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +
        +package org.apache.curator.framework.imps;
        +
        +import com.google.common.base.Preconditions;
        +import com.google.common.collect.Lists;
        +import org.apache.curator.RetryLoop;
        +import org.apache.curator.TimeTrace;
        +import org.apache.curator.framework.CuratorFramework;
        +import org.apache.curator.framework.api.BackgroundCallback;
        +import org.apache.curator.framework.api.CuratorEvent;
        +import org.apache.curator.framework.api.CuratorEventType;
        +import org.apache.curator.framework.api.transaction.CuratorMultiTransaction;
        +import org.apache.curator.framework.api.transaction.CuratorMultiTransactionMain;
        +import org.apache.curator.framework.api.transaction.CuratorOp;
        +import org.apache.curator.framework.api.transaction.CuratorTransactionResult;
        +import org.apache.zookeeper.AsyncCallback;
        +import org.apache.zookeeper.OpResult;
        +import java.util.Arrays;
        +import java.util.List;
        +import java.util.concurrent.Callable;
        +import java.util.concurrent.Executor;
        +
        +/**
        + * @deprecated Use

        {@link CuratorFramework#transaction()}

        — End diff –

        Why are we adding a class that is immediately deprecated? Or is this a result of a move that isn't clear on the web view of the PR?

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30150375 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java — @@ -0,0 +1,166 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.curator.framework.imps; + +import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; +import org.apache.curator.RetryLoop; +import org.apache.curator.TimeTrace; +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.framework.api.BackgroundCallback; +import org.apache.curator.framework.api.CuratorEvent; +import org.apache.curator.framework.api.CuratorEventType; +import org.apache.curator.framework.api.transaction.CuratorMultiTransaction; +import org.apache.curator.framework.api.transaction.CuratorMultiTransactionMain; +import org.apache.curator.framework.api.transaction.CuratorOp; +import org.apache.curator.framework.api.transaction.CuratorTransactionResult; +import org.apache.zookeeper.AsyncCallback; +import org.apache.zookeeper.OpResult; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.Executor; + +/** + * @deprecated Use {@link CuratorFramework#transaction()} — End diff – Why are we adding a class that is immediately deprecated? Or is this a result of a move that isn't clear on the web view of the PR?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30155545

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/transaction/CuratorMultiTransactionMain.java —
        @@ -0,0 +1,45 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.api.transaction;
        +
        +import org.apache.curator.framework.CuratorFramework;
        +import java.util.List;
        +
        +public interface CuratorMultiTransactionMain
        +{
        + /**
        + * Commit the given operations as a single transaction. Create the
        + * operation instances via

        {@link CuratorFramework#transactionOp()}
        + *
        + * @param operations operations that make up the transaction.
        + * @return result details
        + * @throws Exception errors
        + */
        + List<CuratorTransactionResult> forOperations(CuratorOp... operations) throws Exception;
        +
        + /**
        + * Commit the given operations as a single transaction. Create the
        + * operation instances via {@link CuratorFramework#transactionOp()}

        + *
        + * @param operations operations that make up the transaction.
        + * @return result details
        + * @throws Exception errors
        + */
        + List<CuratorTransactionResult> forOperations(List<CuratorOp> operations) throws Exception;
        — End diff –

        If users believe that order matters, then they can use a list. If they don't care, then it might make sense for them to use a Set, or in Java8 a Stream. Well, a finite Stream. Also, would be more flexible to use `<? extends CuratorOp>`

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30155545 — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/transaction/CuratorMultiTransactionMain.java — @@ -0,0 +1,45 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.api.transaction; + +import org.apache.curator.framework.CuratorFramework; +import java.util.List; + +public interface CuratorMultiTransactionMain +{ + /** + * Commit the given operations as a single transaction. Create the + * operation instances via {@link CuratorFramework#transactionOp()} + * + * @param operations operations that make up the transaction. + * @return result details + * @throws Exception errors + */ + List<CuratorTransactionResult> forOperations(CuratorOp... operations) throws Exception; + + /** + * Commit the given operations as a single transaction. Create the + * operation instances via {@link CuratorFramework#transactionOp()} + * + * @param operations operations that make up the transaction. + * @return result details + * @throws Exception errors + */ + List<CuratorTransactionResult> forOperations(List<CuratorOp> operations) throws Exception; — End diff – If users believe that order matters, then they can use a list. If they don't care, then it might make sense for them to use a Set, or in Java8 a Stream. Well, a finite Stream. Also, would be more flexible to use `<? extends CuratorOp>`
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30155605

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorTransactionImpl.java —
        @@ -188,19 +169,45 @@ private CuratorTransactionResult makeCuratorResult(OpResult opResult, CuratorMul

        case ZooDefs.OpCode.create:

        { - OpResult.CreateResult createResult = (OpResult.CreateResult)opResult; + OpResult.CreateResult createResult = (OpResult.CreateResult)opResult; resultPath = client.unfixForNamespace(createResult.getPath()); break; }

        case ZooDefs.OpCode.setData:

        { - OpResult.SetDataResult setDataResult = (OpResult.SetDataResult)opResult; + OpResult.SetDataResult setDataResult = (OpResult.SetDataResult)opResult; resultStat = setDataResult.getStat(); break; }

        }

        • return new CuratorTransactionResult(metadata.type, metadata.forPath, resultPath, resultStat);
          + return new CuratorTransactionResult(metadata.getType(), metadata.getForPath(), resultPath, resultStat);
          + }
          +
          + private List<OpResult> doOperation(AtomicBoolean firstTime) throws Exception
          + {
          + boolean localFirstTime = firstTime.getAndSet(false);
          + if ( !localFirstTime )
          + {
          + // TODO
            • End diff –

        Do what?

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30155605 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorTransactionImpl.java — @@ -188,19 +169,45 @@ private CuratorTransactionResult makeCuratorResult(OpResult opResult, CuratorMul case ZooDefs.OpCode.create: { - OpResult.CreateResult createResult = (OpResult.CreateResult)opResult; + OpResult.CreateResult createResult = (OpResult.CreateResult)opResult; resultPath = client.unfixForNamespace(createResult.getPath()); break; } case ZooDefs.OpCode.setData: { - OpResult.SetDataResult setDataResult = (OpResult.SetDataResult)opResult; + OpResult.SetDataResult setDataResult = (OpResult.SetDataResult)opResult; resultStat = setDataResult.getStat(); break; } } return new CuratorTransactionResult(metadata.type, metadata.forPath, resultPath, resultStat); + return new CuratorTransactionResult(metadata.getType(), metadata.getForPath(), resultPath, resultStat); + } + + private List<OpResult> doOperation(AtomicBoolean firstTime) throws Exception + { + boolean localFirstTime = firstTime.getAndSet(false); + if ( !localFirstTime ) + { + // TODO End diff – Do what?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30155773

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/ExtractingCuratorOp.java —
        @@ -0,0 +1,59 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.imps;
        +
        +import com.google.common.base.Preconditions;
        +import org.apache.curator.framework.api.transaction.CuratorOp;
        +import org.apache.curator.framework.api.transaction.TypeAndPath;
        +import org.apache.zookeeper.Op;
        +
        +class ExtractingCuratorOp implements CuratorOp
        +{
        + private final CuratorMultiTransactionRecord record = new CuratorMultiTransactionRecord();
        +
        + CuratorMultiTransactionRecord getRecord()
        +

        { + return record; + }

        +
        + CuratorOp asCuratorOp()
        — End diff –

        This should not be necessary.

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30155773 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/ExtractingCuratorOp.java — @@ -0,0 +1,59 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.imps; + +import com.google.common.base.Preconditions; +import org.apache.curator.framework.api.transaction.CuratorOp; +import org.apache.curator.framework.api.transaction.TypeAndPath; +import org.apache.zookeeper.Op; + +class ExtractingCuratorOp implements CuratorOp +{ + private final CuratorMultiTransactionRecord record = new CuratorMultiTransactionRecord(); + + CuratorMultiTransactionRecord getRecord() + { + return record; + } + + CuratorOp asCuratorOp() — End diff – This should not be necessary.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30155876

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/SetDataBuilderImpl.java —
        @@ -246,7 +243,7 @@ public Stat forPath(String path, byte[] data) throws Exception
        Stat resultStat = null;
        if ( backgrounding.inBackground() )
        {

        • client.processBackgroundOperation(new OperationAndData<PathAndBytes>(this, new PathAndBytes(path, data), backgrounding.getCallback(), null, backgrounding.getContext()), null);
          + client.processBackgroundOperation(new OperationAndData<>(this, new PathAndBytes(path, data), backgrounding.getCallback(), null, backgrounding.getContext()), null);
            • End diff –

        This is not a valid JDK6 construct.

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30155876 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/SetDataBuilderImpl.java — @@ -246,7 +243,7 @@ public Stat forPath(String path, byte[] data) throws Exception Stat resultStat = null; if ( backgrounding.inBackground() ) { client.processBackgroundOperation(new OperationAndData<PathAndBytes>(this, new PathAndBytes(path, data), backgrounding.getCallback(), null, backgrounding.getContext()), null); + client.processBackgroundOperation(new OperationAndData<>(this, new PathAndBytes(path, data), backgrounding.getCallback(), null, backgrounding.getContext()), null); End diff – This is not a valid JDK6 construct.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30155941

        — Diff: curator-framework/src/site/confluence/index.confluence —
        @@ -42,11 +42,16 @@ h3. Methods

        getData() Begins an operation to get a ZNode's data. Call additional methods (watch, background or get stat) and finalize the operation by calling forPath()
        setData() Begins an operation to set a ZNode's data. Call additional methods (version or background) and finalize the operation by calling forPath()
        getChildren() Begins an operation to get a ZNode's list of children ZNodes. Call additional methods (watch, background or get stat) and finalize the operation by calling forPath()

        +<<<<<<< HEAD
        — End diff –

        Merge conflict needs to be resolved.

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30155941 — Diff: curator-framework/src/site/confluence/index.confluence — @@ -42,11 +42,16 @@ h3. Methods getData() Begins an operation to get a ZNode's data. Call additional methods (watch, background or get stat) and finalize the operation by calling forPath() setData() Begins an operation to set a ZNode's data. Call additional methods (version or background) and finalize the operation by calling forPath() getChildren() Begins an operation to get a ZNode's list of children ZNodes. Call additional methods (watch, background or get stat) and finalize the operation by calling forPath() +<<<<<<< HEAD — End diff – Merge conflict needs to be resolved.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30166221

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/transaction/CuratorMultiTransactionMain.java —
        @@ -0,0 +1,45 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.api.transaction;
        +
        +import org.apache.curator.framework.CuratorFramework;
        +import java.util.List;
        +
        +public interface CuratorMultiTransactionMain
        +{
        + /**
        + * Commit the given operations as a single transaction. Create the
        + * operation instances via

        {@link CuratorFramework#transactionOp()}
        + *
        + * @param operations operations that make up the transaction.
        + * @return result details
        + * @throws Exception errors
        + */
        + List<CuratorTransactionResult> forOperations(CuratorOp... operations) throws Exception;
        +
        + /**
        + * Commit the given operations as a single transaction. Create the
        + * operation instances via {@link CuratorFramework#transactionOp()}

        + *
        + * @param operations operations that make up the transaction.
        + * @return result details
        + * @throws Exception errors
        + */
        + List<CuratorTransactionResult> forOperations(List<CuratorOp> operations) throws Exception;
        — End diff –

        I realize now that the results are returned in the order of the operations. It's the only way to correlate. So, that suggests leaving it as a list.

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30166221 — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/transaction/CuratorMultiTransactionMain.java — @@ -0,0 +1,45 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.api.transaction; + +import org.apache.curator.framework.CuratorFramework; +import java.util.List; + +public interface CuratorMultiTransactionMain +{ + /** + * Commit the given operations as a single transaction. Create the + * operation instances via {@link CuratorFramework#transactionOp()} + * + * @param operations operations that make up the transaction. + * @return result details + * @throws Exception errors + */ + List<CuratorTransactionResult> forOperations(CuratorOp... operations) throws Exception; + + /** + * Commit the given operations as a single transaction. Create the + * operation instances via {@link CuratorFramework#transactionOp()} + * + * @param operations operations that make up the transaction. + * @return result details + * @throws Exception errors + */ + List<CuratorTransactionResult> forOperations(List<CuratorOp> operations) throws Exception; — End diff – I realize now that the results are returned in the order of the operations. It's the only way to correlate. So, that suggests leaving it as a list.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30166279

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java —
        @@ -0,0 +1,166 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +
        +package org.apache.curator.framework.imps;
        +
        +import com.google.common.base.Preconditions;
        +import com.google.common.collect.Lists;
        +import org.apache.curator.RetryLoop;
        +import org.apache.curator.TimeTrace;
        +import org.apache.curator.framework.CuratorFramework;
        +import org.apache.curator.framework.api.BackgroundCallback;
        +import org.apache.curator.framework.api.CuratorEvent;
        +import org.apache.curator.framework.api.CuratorEventType;
        +import org.apache.curator.framework.api.transaction.CuratorMultiTransaction;
        +import org.apache.curator.framework.api.transaction.CuratorMultiTransactionMain;
        +import org.apache.curator.framework.api.transaction.CuratorOp;
        +import org.apache.curator.framework.api.transaction.CuratorTransactionResult;
        +import org.apache.zookeeper.AsyncCallback;
        +import org.apache.zookeeper.OpResult;
        +import java.util.Arrays;
        +import java.util.List;
        +import java.util.concurrent.Callable;
        +import java.util.concurrent.Executor;
        +
        +/**
        + * @deprecated Use

        {@link CuratorFramework#transaction()}

        — End diff –

        The deprecated is a mistake here. I'll remove it.

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30166279 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorMultiTransactionImpl.java — @@ -0,0 +1,166 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.curator.framework.imps; + +import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; +import org.apache.curator.RetryLoop; +import org.apache.curator.TimeTrace; +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.framework.api.BackgroundCallback; +import org.apache.curator.framework.api.CuratorEvent; +import org.apache.curator.framework.api.CuratorEventType; +import org.apache.curator.framework.api.transaction.CuratorMultiTransaction; +import org.apache.curator.framework.api.transaction.CuratorMultiTransactionMain; +import org.apache.curator.framework.api.transaction.CuratorOp; +import org.apache.curator.framework.api.transaction.CuratorTransactionResult; +import org.apache.zookeeper.AsyncCallback; +import org.apache.zookeeper.OpResult; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.Executor; + +/** + * @deprecated Use {@link CuratorFramework#transaction()} — End diff – The deprecated is a mistake here. I'll remove it.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30166320

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorTransactionImpl.java —
        @@ -188,19 +169,45 @@ private CuratorTransactionResult makeCuratorResult(OpResult opResult, CuratorMul

        case ZooDefs.OpCode.create:

        { - OpResult.CreateResult createResult = (OpResult.CreateResult)opResult; + OpResult.CreateResult createResult = (OpResult.CreateResult)opResult; resultPath = client.unfixForNamespace(createResult.getPath()); break; }

        case ZooDefs.OpCode.setData:

        { - OpResult.SetDataResult setDataResult = (OpResult.SetDataResult)opResult; + OpResult.SetDataResult setDataResult = (OpResult.SetDataResult)opResult; resultStat = setDataResult.getStat(); break; }

        }

        • return new CuratorTransactionResult(metadata.type, metadata.forPath, resultPath, resultStat);
          + return new CuratorTransactionResult(metadata.getType(), metadata.getForPath(), resultPath, resultStat);
          + }
          +
          + private List<OpResult> doOperation(AtomicBoolean firstTime) throws Exception
          + {
          + boolean localFirstTime = firstTime.getAndSet(false);
          + if ( !localFirstTime )
          + {
          + // TODO
            • End diff –

        That's been in there a long time. I should just remove it.

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30166320 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/CuratorTransactionImpl.java — @@ -188,19 +169,45 @@ private CuratorTransactionResult makeCuratorResult(OpResult opResult, CuratorMul case ZooDefs.OpCode.create: { - OpResult.CreateResult createResult = (OpResult.CreateResult)opResult; + OpResult.CreateResult createResult = (OpResult.CreateResult)opResult; resultPath = client.unfixForNamespace(createResult.getPath()); break; } case ZooDefs.OpCode.setData: { - OpResult.SetDataResult setDataResult = (OpResult.SetDataResult)opResult; + OpResult.SetDataResult setDataResult = (OpResult.SetDataResult)opResult; resultStat = setDataResult.getStat(); break; } } return new CuratorTransactionResult(metadata.type, metadata.forPath, resultPath, resultStat); + return new CuratorTransactionResult(metadata.getType(), metadata.getForPath(), resultPath, resultStat); + } + + private List<OpResult> doOperation(AtomicBoolean firstTime) throws Exception + { + boolean localFirstTime = firstTime.getAndSet(false); + if ( !localFirstTime ) + { + // TODO End diff – That's been in there a long time. I should just remove it.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30166387

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/SetDataBuilderImpl.java —
        @@ -246,7 +243,7 @@ public Stat forPath(String path, byte[] data) throws Exception
        Stat resultStat = null;
        if ( backgrounding.inBackground() )
        {

        • client.processBackgroundOperation(new OperationAndData<PathAndBytes>(this, new PathAndBytes(path, data), backgrounding.getCallback(), null, backgrounding.getContext()), null);
          + client.processBackgroundOperation(new OperationAndData<>(this, new PathAndBytes(path, data), backgrounding.getCallback(), null, backgrounding.getContext()), null);
            • End diff –

        Curator 3.0 will be Java 7 as ZK 3.5 is.

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30166387 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/SetDataBuilderImpl.java — @@ -246,7 +243,7 @@ public Stat forPath(String path, byte[] data) throws Exception Stat resultStat = null; if ( backgrounding.inBackground() ) { client.processBackgroundOperation(new OperationAndData<PathAndBytes>(this, new PathAndBytes(path, data), backgrounding.getCallback(), null, backgrounding.getContext()), null); + client.processBackgroundOperation(new OperationAndData<>(this, new PathAndBytes(path, data), backgrounding.getCallback(), null, backgrounding.getContext()), null); End diff – Curator 3.0 will be Java 7 as ZK 3.5 is.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30169301

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/ExtractingCuratorOp.java —
        @@ -0,0 +1,59 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.imps;
        +
        +import com.google.common.base.Preconditions;
        +import org.apache.curator.framework.api.transaction.CuratorOp;
        +import org.apache.curator.framework.api.transaction.TypeAndPath;
        +import org.apache.zookeeper.Op;
        +
        +class ExtractingCuratorOp implements CuratorOp
        +{
        + private final CuratorMultiTransactionRecord record = new CuratorMultiTransactionRecord();
        +
        + CuratorMultiTransactionRecord getRecord()
        +

        { + return record; + }

        +
        + CuratorOp asCuratorOp()
        — End diff –

        It avoids a cast in some code (see TransactionOpImpl)

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30169301 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/ExtractingCuratorOp.java — @@ -0,0 +1,59 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.imps; + +import com.google.common.base.Preconditions; +import org.apache.curator.framework.api.transaction.CuratorOp; +import org.apache.curator.framework.api.transaction.TypeAndPath; +import org.apache.zookeeper.Op; + +class ExtractingCuratorOp implements CuratorOp +{ + private final CuratorMultiTransactionRecord record = new CuratorMultiTransactionRecord(); + + CuratorMultiTransactionRecord getRecord() + { + return record; + } + + CuratorOp asCuratorOp() — End diff – It avoids a cast in some code (see TransactionOpImpl)
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30170525

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/ExtractingCuratorOp.java —
        @@ -0,0 +1,59 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.imps;
        +
        +import com.google.common.base.Preconditions;
        +import org.apache.curator.framework.api.transaction.CuratorOp;
        +import org.apache.curator.framework.api.transaction.TypeAndPath;
        +import org.apache.zookeeper.Op;
        +
        +class ExtractingCuratorOp implements CuratorOp
        +{
        + private final CuratorMultiTransactionRecord record = new CuratorMultiTransactionRecord();
        +
        + CuratorMultiTransactionRecord getRecord()
        +

        { + return record; + }

        +
        + CuratorOp asCuratorOp()
        — End diff –

        Two notes - in Java 8, I believe the compiler can do that inferencing for you so no cast required, and today I think you can add a type witness: `new CreateBuilderImpl(client).<CuratorOp>asTransactionCreateBuilder(op, op.getRecord())`

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30170525 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/ExtractingCuratorOp.java — @@ -0,0 +1,59 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.imps; + +import com.google.common.base.Preconditions; +import org.apache.curator.framework.api.transaction.CuratorOp; +import org.apache.curator.framework.api.transaction.TypeAndPath; +import org.apache.zookeeper.Op; + +class ExtractingCuratorOp implements CuratorOp +{ + private final CuratorMultiTransactionRecord record = new CuratorMultiTransactionRecord(); + + CuratorMultiTransactionRecord getRecord() + { + return record; + } + + CuratorOp asCuratorOp() — End diff – Two notes - in Java 8, I believe the compiler can do that inferencing for you so no cast required, and today I think you can add a type witness: `new CreateBuilderImpl(client).<CuratorOp>asTransactionCreateBuilder(op, op.getRecord())`
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30170696

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/ExtractingCuratorOp.java —
        @@ -0,0 +1,59 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.imps;
        +
        +import com.google.common.base.Preconditions;
        +import org.apache.curator.framework.api.transaction.CuratorOp;
        +import org.apache.curator.framework.api.transaction.TypeAndPath;
        +import org.apache.zookeeper.Op;
        +
        +class ExtractingCuratorOp implements CuratorOp
        +{
        + private final CuratorMultiTransactionRecord record = new CuratorMultiTransactionRecord();
        +
        + CuratorMultiTransactionRecord getRecord()
        +

        { + return record; + }

        +
        + CuratorOp asCuratorOp()
        — End diff –

        Yeah, in Java 8 this is relaxed. I'll look at the second thing.

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30170696 — Diff: curator-framework/src/main/java/org/apache/curator/framework/imps/ExtractingCuratorOp.java — @@ -0,0 +1,59 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.imps; + +import com.google.common.base.Preconditions; +import org.apache.curator.framework.api.transaction.CuratorOp; +import org.apache.curator.framework.api.transaction.TypeAndPath; +import org.apache.zookeeper.Op; + +class ExtractingCuratorOp implements CuratorOp +{ + private final CuratorMultiTransactionRecord record = new CuratorMultiTransactionRecord(); + + CuratorMultiTransactionRecord getRecord() + { + return record; + } + + CuratorOp asCuratorOp() — End diff – Yeah, in Java 8 this is relaxed. I'll look at the second thing.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on a diff in the pull request:

        https://github.com/apache/curator/pull/77#discussion_r30171284

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/transaction/CuratorMultiTransactionMain.java —
        @@ -0,0 +1,45 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.api.transaction;
        +
        +import org.apache.curator.framework.CuratorFramework;
        +import java.util.List;
        +
        +public interface CuratorMultiTransactionMain
        +{
        + /**
        + * Commit the given operations as a single transaction. Create the
        + * operation instances via

        {@link CuratorFramework#transactionOp()}
        + *
        + * @param operations operations that make up the transaction.
        + * @return result details
        + * @throws Exception errors
        + */
        + List<CuratorTransactionResult> forOperations(CuratorOp... operations) throws Exception;
        +
        + /**
        + * Commit the given operations as a single transaction. Create the
        + * operation instances via {@link CuratorFramework#transactionOp()}

        + *
        + * @param operations operations that make up the transaction.
        + * @return result details
        — End diff –

        Expand the javadoc here, please. For example, I was surprised to read the code and discover that this call returns null if the operation is in the background.

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/curator/pull/77#discussion_r30171284 — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/transaction/CuratorMultiTransactionMain.java — @@ -0,0 +1,45 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.api.transaction; + +import org.apache.curator.framework.CuratorFramework; +import java.util.List; + +public interface CuratorMultiTransactionMain +{ + /** + * Commit the given operations as a single transaction. Create the + * operation instances via {@link CuratorFramework#transactionOp()} + * + * @param operations operations that make up the transaction. + * @return result details + * @throws Exception errors + */ + List<CuratorTransactionResult> forOperations(CuratorOp... operations) throws Exception; + + /** + * Commit the given operations as a single transaction. Create the + * operation instances via {@link CuratorFramework#transactionOp()} + * + * @param operations operations that make up the transaction. + * @return result details — End diff – Expand the javadoc here, please. For example, I was surprised to read the code and discover that this call returns null if the operation is in the background.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on the pull request:

        https://github.com/apache/curator/pull/77#issuecomment-101394950

        Can you add a class to curator-examples that demonstrates how to use this new API?

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on the pull request: https://github.com/apache/curator/pull/77#issuecomment-101394950 Can you add a class to curator-examples that demonstrates how to use this new API?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on the pull request:

        https://github.com/apache/curator/pull/77#issuecomment-101395257

        That's a good idea - will do

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on the pull request: https://github.com/apache/curator/pull/77#issuecomment-101395257 That's a good idea - will do
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on the pull request:

        https://github.com/apache/curator/pull/77#issuecomment-101413729

        TestCompressionInTransaction uses the old APIs. A test with the new APIs would be good to check for regressions.

        Also, I think something weird happened during the merge in CreateBuilderImpl. It's missing `debugForceFindProtectedNode` which is causing test failures.

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on the pull request: https://github.com/apache/curator/pull/77#issuecomment-101413729 TestCompressionInTransaction uses the old APIs. A test with the new APIs would be good to check for regressions. Also, I think something weird happened during the merge in CreateBuilderImpl. It's missing `debugForceFindProtectedNode` which is causing test failures.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on the pull request:

        https://github.com/apache/curator/pull/77#issuecomment-101414994

        Another thing that is unclear is whether the `TransactionOp` are reusable or if they are one-shot.

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on the pull request: https://github.com/apache/curator/pull/77#issuecomment-101414994 Another thing that is unclear is whether the `TransactionOp` are reusable or if they are one-shot.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on the pull request:

        https://github.com/apache/curator/pull/77#issuecomment-101455486

        Anything else before I merge?

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on the pull request: https://github.com/apache/curator/pull/77#issuecomment-101455486 Anything else before I merge?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user madrob commented on the pull request:

        https://github.com/apache/curator/pull/77#issuecomment-101523984

        I don't think you addressed the floating TODO, and `forOperation` should take `List<? extends CuratorOp>` instead of `List<CuratorOp>`. Other than those minor issues, the rest of this looks good. Thanks, Jordan!

        Show
        githubbot ASF GitHub Bot added a comment - Github user madrob commented on the pull request: https://github.com/apache/curator/pull/77#issuecomment-101523984 I don't think you addressed the floating TODO, and `forOperation` should take `List<? extends CuratorOp>` instead of `List<CuratorOp>`. Other than those minor issues, the rest of this looks good. Thanks, Jordan!
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on the pull request:

        https://github.com/apache/curator/pull/77#issuecomment-101641856

        I'm going to leave the TODO for future investigation. I want to know why it's there. Thanks for the great review.

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on the pull request: https://github.com/apache/curator/pull/77#issuecomment-101641856 I'm going to leave the TODO for future investigation. I want to know why it's there. Thanks for the great review.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/curator/pull/77

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/curator/pull/77

          People

          • Assignee:
            randgalt Jordan Zimmerman
            Reporter:
            randgalt Jordan Zimmerman
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development