You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/03/14 09:39:21 UTC

[GitHub] [rocketmq] aaron-ai opened a new pull request #3987: Add new APIs for producer

aaron-ai opened a new pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987


   As we mentioned in [RIP-37 New and unified APIs](https://shimo.im/docs/m5kv92OeRRU8olqX), establish a new APIs specifications. we divide it into two independent pull requests.
   
   1. The producer part.
   2. The consumer part.
   
   this pull request is about the producer part.
   
   related issue: #3973 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] zhouxinyu commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826808776



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);

Review comment:
       We can find many fluent builder samples in JDK which use the setter pattern, such as https://docs.oracle.com/javase/7/docs/api/java/util/Locale.Builder.html




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] zhouxinyu commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826810434



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {

Review comment:
       Do we have a chance to force our developers to set the required fields when building the `Producer` or `Message`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] zhouxinyu commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826829945



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Transaction.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+/**
+ * An entity to describe an independent transaction, which follows
+ * <a href="https://en.wikipedia.org/wiki/Two-phase_commit_protocol">two-phase commit protocol</a>.
+ *
+ * <p>once {@link Transaction#commit()} or {@link Transaction#rollback()} is invoked, subsequent commit or rollback in
+ * {@link Transaction} is ignored by client.
+ *
+ * <p>Neither of {@link Transaction#commit()} and {@link Transaction#rollback()} ensures the success on account of
+ * network timeout or other issues, that's why it does not make any sense to execute {@link Transaction#commit()} or
+ * {@link Transaction#rollback()} after is has been executed once already. The suspended transaction will be
+ * solved by {@link TransactionChecker}.
+ */
+public interface Transaction {
+    /**
+     * Try to commit the transaction, which would expose the message before the transaction is closed.
+     *
+     * <p>We don't ensure this operation is successful even though no exception is thrown after invocation,
+     * <strong>actually we omit the exception on purpose because {@link TransactionChecker} is the unique right way
+     * to solve the suspended transaction rather than commit or roll-back repeatedly.</strong>
+     */
+    void commit();

Review comment:
       `TransactionChecker` is less real-time, which may result in high latency when the commit failed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls edited a comment on pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#issuecomment-1070867608


   
   [![Coverage Status](https://coveralls.io/builds/47480496/badge)](https://coveralls.io/builds/47480496)
   
   Coverage decreased (-0.008%) to 49.654% when pulling **d30f3f5101146f3f3ec32a433167289cb623ab82 on aaron-ai:producer_pr** into **0b7291b35a3a1e3517ce5403d676159c4b6500ed on apache:5.0.0-beta**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls edited a comment on pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#issuecomment-1070867608


   
   [![Coverage Status](https://coveralls.io/builds/47460561/badge)](https://coveralls.io/builds/47460561)
   
   Coverage decreased (-0.004%) to 49.665% when pulling **fc90eac42e0d4758f1b089fc89491b64f460d72a on aaron-ai:producer_pr** into **176e0d5c225a15e6c24a065325eed6eabc9cb1a2 on apache:5.0.0-beta**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r827016443



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);
+
+    /**
+     * Declare topics ahead of message sending/preparation.
+     *
+     * <p>Even though the declaration is not essential, we <strong>highly recommend</strong> to declare the topics in
+     * advance, which could help to discover potential mistakes.
+     *
+     * @param topics topics to send/prepare.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder withTopics(String... topics);
+
+    /**
+     * Set the threads count for {@link Producer#sendAsync(Message)}.
+     *
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setAsyncThreadCount(int count);
+
+    /**
+     * Set the retry policy to send message.
+     *
+     * @param retryPolicy policy to re-send message when failure encountered.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setRetryPolicy(BackoffRetryPolicy retryPolicy);
+
+    /**
+     * Set the transaction checker for producer.
+     *
+     * @param checker transaction checker.
+     * @return the produce builder instance.
+     */
+    ProducerBuilder setTransactionChecker(TransactionChecker checker);
+
+    /**
+     * Finalize the build of {@link Producer} instance and start.
+     *
+     * <p>The producer does a series of preparatory work during startup, which could help to identify more unexpected
+     * error earlier.
+     *
+     * <p>Especially, if this method is invoked more than once, different producer will be created and started.
+     *
+     * @return the producer instance.
+     * @throws TopicDoesNotExistException if there are nonexistent topic(s).
+     * @throws AuthorisationException     if no permission to communicate with server.
+     * @throws AuthenticationException    if identification could not be recognized by server.
+     * @throws NetworkTimeoutException    if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException if there is a network connection problem.
+     */
+    Producer start() throws ClientException;

Review comment:
       > It's strange if we don't have a `build` method in a builder interface.
   > 
   > If we only provide a `start` method in the producer builder, that means all the producers are started by default, so why we should have a `start`?
   
   I agree with the opinon that lack of `#build()` may be a little bit wireld, let's look at it another way. Is it unnecessary for the user to start this process? If so, then I think it's just a naming issue.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] WJL3333 commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
WJL3333 commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826585199



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/exception/AuthorisationException.java
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.rocketmq.apis.exception;
+
+public class AuthorisationException extends ClientException {

Review comment:
       can we add some doc for this exception? it seems the same as AuthenticationException from the naming




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] chenzlalvin commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
chenzlalvin commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826850071



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {

Review comment:
       Builder should throw null field check exception when call build interface.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] dongeforever commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
dongeforever commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826617165



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageId send(Message message, Transaction transaction) throws ClientException;

Review comment:
       How to send more than one message using such API?
   MessageId send(Message message, Transaction transaction)
   
   
   The old API  for the transaction is not naturally two-phase based. It's better to polish it in the new API.
   But currently, the new API is just like the old
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826595560



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);
+
+    /**
+     * Declare topics ahead of message sending/preparation.
+     *
+     * <p>Even though the declaration is not essential, we <strong>highly recommend</strong> to declare the topics in
+     * advance, which could help to discover potential mistakes.
+     *
+     * @param topics topics to send/prepare.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder withTopics(String... topics);
+
+    /**
+     * Set the threads count for {@link Producer#sendAsync(Message)}.
+     *
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setAsyncThreadCount(int count);
+
+    /**
+     * Set the retry policy to send message.
+     *
+     * @param retryPolicy policy to re-send message when failure encountered.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setRetryPolicy(BackoffRetryPolicy retryPolicy);
+
+    /**
+     * Set the transaction checker for producer.
+     *
+     * @param checker transaction checker.
+     * @return the produce builder instance.
+     */
+    ProducerBuilder setTransactionChecker(TransactionChecker checker);
+
+    /**
+     * Finalize the build of {@link Producer} instance and start.
+     *
+     * <p>The producer does a series of preparatory work during startup, which could help to identify more unexpected
+     * error earlier.
+     *
+     * <p>Especially, if this method is invoked more than once, different producer will be created and started.
+     *
+     * @return the producer instance.
+     * @throws TopicDoesNotExistException if there are nonexistent topic(s).
+     * @throws AuthorisationException     if no permission to communicate with server.
+     * @throws AuthenticationException    if identification could not be recognized by server.
+     * @throws NetworkTimeoutException    if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException if there is a network connection problem.
+     */
+    Producer start() throws ClientException;

Review comment:
       Actually `producerBuilder#start` could avoid to start producer repeatedly. What's more, a producer which is not started does not make sense.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls edited a comment on pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#issuecomment-1070867608


   
   [![Coverage Status](https://coveralls.io/builds/47479091/badge)](https://coveralls.io/builds/47479091)
   
   Coverage decreased (-0.04%) to 49.626% when pulling **23fa2c2863969e1b181139f41d8566eb6e36742c on aaron-ai:producer_pr** into **176e0d5c225a15e6c24a065325eed6eabc9cb1a2 on apache:5.0.0-beta**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] WJL3333 commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
WJL3333 commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826588141



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageView.java
##########
@@ -0,0 +1,116 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.MessageQueue;
+
+public interface MessageView {

Review comment:
       need java doc here. what is an MessageView ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] zhouxinyu commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826820281



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);
+
+    /**
+     * Declare topics ahead of message sending/preparation.
+     *
+     * <p>Even though the declaration is not essential, we <strong>highly recommend</strong> to declare the topics in
+     * advance, which could help to discover potential mistakes.
+     *
+     * @param topics topics to send/prepare.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder withTopics(String... topics);
+
+    /**
+     * Set the threads count for {@link Producer#sendAsync(Message)}.
+     *
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setAsyncThreadCount(int count);
+
+    /**
+     * Set the retry policy to send message.
+     *
+     * @param retryPolicy policy to re-send message when failure encountered.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setRetryPolicy(BackoffRetryPolicy retryPolicy);
+
+    /**
+     * Set the transaction checker for producer.
+     *
+     * @param checker transaction checker.
+     * @return the produce builder instance.
+     */
+    ProducerBuilder setTransactionChecker(TransactionChecker checker);
+
+    /**
+     * Finalize the build of {@link Producer} instance and start.
+     *
+     * <p>The producer does a series of preparatory work during startup, which could help to identify more unexpected
+     * error earlier.
+     *
+     * <p>Especially, if this method is invoked more than once, different producer will be created and started.
+     *
+     * @return the producer instance.
+     * @throws TopicDoesNotExistException if there are nonexistent topic(s).
+     * @throws AuthorisationException     if no permission to communicate with server.
+     * @throws AuthenticationException    if identification could not be recognized by server.
+     * @throws NetworkTimeoutException    if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException if there is a network connection problem.
+     */
+    Producer start() throws ClientException;

Review comment:
       It's strange if we don't have a `build` method in a builder interface.
   
   If we only provide a `start` method in the producer builder, that means all the producers are started by default, so why we should have a `start`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] zhouxinyu commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826838972



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);

Review comment:
       @WJL3333 Lombok doesn't support prefixed builders at the beginning, but this project resolved this issue later, please refer to https://github.com/projectlombok/lombok/issues/1805#issuecomment-465140144, there are many reasons support we use prefixed builders.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] lizhanhui merged pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
lizhanhui merged pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] lizhanhui merged pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
lizhanhui merged pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls commented on pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#issuecomment-1070867604






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826771462



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageView.java
##########
@@ -0,0 +1,116 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.MessageQueue;
+
+public interface MessageView {
+    /**
+     * Get the unique id of message.
+     *
+     * @return unique id.
+     */
+    MessageId getMessageId();
+
+    /**
+     * Get the topic of message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the {@link MessageQueue} of message.
+     *
+     * @return message queue.
+     */
+    MessageQueue getMessageQueue();
+
+    /**
+     * Get the position of message in {@link MessageQueue}.
+     */
+    long getOffset();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body, which makes the modification of return value does not
+     * affect the message itself.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();

Review comment:
       > how about ByteBuffer here which can be set as unmodifable
   
   Nice suggestion.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r825862010



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageId send(Message message, Transaction transaction) throws ClientException;
+
+    /**
+     * Send a message asynchronously.
+     *
+     * <p>This method returns immediately, the result is included in the {@link CompletableFuture};
+     *
+     * @param message message to send.
+     * @return a future that indicates the result.
+     */
+    CompletableFuture<MessageId> sendAsync(Message message);
+
+    /**
+     * Send batch messages synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * <p>All messages to send should have the same topic.
+     *
+     * @param messages batch messages to send.
+     * @return a map which indicates the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    Map<Message, MessageId> send(Collection<Message> messages) throws ClientException;

Review comment:
       Nice catch!




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] dongeforever commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
dongeforever commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r825838652



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageId send(Message message, Transaction transaction) throws ClientException;
+
+    /**
+     * Send a message asynchronously.
+     *
+     * <p>This method returns immediately, the result is included in the {@link CompletableFuture};
+     *
+     * @param message message to send.
+     * @return a future that indicates the result.
+     */
+    CompletableFuture<MessageId> sendAsync(Message message);
+
+    /**
+     * Send batch messages synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * <p>All messages to send should have the same topic.
+     *
+     * @param messages batch messages to send.
+     * @return a map which indicates the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    Map<Message, MessageId> send(Collection<Message> messages) throws ClientException;

Review comment:
       What will happen if I use a list with the same object?
   
   The return type does not match the argument.
   
   Maybe both using list is ok.

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;

Review comment:
       Where is the SendResult?
   
   The client at least needs to know the offset and messagequeue for checking!
   

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/Message.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.producer.Producer;
+
+/**
+ * Abstract message only used for {@link Producer}.
+ */
+public interface Message {
+    /**
+     * Get the topic of message, which is the first classifier for message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();
+
+    /**
+     * Get the <strong>deep copy</strong> of message properties.
+     *
+     * @return the <strong>deep copy</strong> of message properties.
+     */
+    Map<String, String> getProperties();
+
+    /**
+     * Get the tag of message, which is the second classifier besides topic.
+     *
+     * @return the tag of message.
+     */
+    Optional<String> getTag();
+
+    /**
+     * Get the key collection of message.
+     *
+     * @return <strong>the key collection</strong> of message.
+     */
+    Collection<String> getKeys();
+
+    /**
+     * Get the message group, which make sense only when topic type is fifo.
+     *
+     * @return message group, which is optional.
+     */
+    Optional<String> getMessageGroup();
+

Review comment:
       This interface need a method getMessageQueue() too.
   It may need to send message to the specified MessageQueue.
   The case cannot be covered by getMessageGroup().
   
   The priority of getMessageQueue()  is bigger than  getMessageGroup().
   
   

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageId send(Message message, Transaction transaction) throws ClientException;

Review comment:
       This is not the two-phase API!
   
   How about like this :
   try {
   TransactionMark  mark = producer.prepareSend(Message message);
   //do process
   producer.commit(mark);
   } catch(Exception e) {
     producer.rollback(mark);
   }
   
   Such style is easy to be integrated with db transaction. And the code will not be split.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai edited a comment on pull request #3987: Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai edited a comment on pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#issuecomment-1066571436


   @zhouxinyu @lizhanhui @lollipopjin @duhenglucky @RongtongJin @drpmma @chenzlalvin


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826599108



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);
+
+    /**
+     * Declare topics ahead of message sending/preparation.
+     *
+     * <p>Even though the declaration is not essential, we <strong>highly recommend</strong> to declare the topics in
+     * advance, which could help to discover potential mistakes.
+     *
+     * @param topics topics to send/prepare.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder withTopics(String... topics);
+
+    /**
+     * Set the threads count for {@link Producer#sendAsync(Message)}.
+     *
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setAsyncThreadCount(int count);
+
+    /**
+     * Set the retry policy to send message.
+     *
+     * @param retryPolicy policy to re-send message when failure encountered.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setRetryPolicy(BackoffRetryPolicy retryPolicy);

Review comment:
       Yeah, this should be considered, we think that it is more strict if we use `BackoffRetryPolicy` here before we allow users to implement their `RetryPolicy`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826774039



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageBuilder.java
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import org.apache.rocketmq.apis.producer.Producer;
+
+/**
+ * Builder to config {@link Message}.
+ */
+public interface MessageBuilder {
+    /**
+     * Set the topic for message.
+     *
+     * @param topic topic for the message.
+     * @return the message builder instance.
+     */
+    MessageBuilder setTopic(String topic);

Review comment:
       Well, setter/getter style is more common in java, `MessageBuilder#topic()` or `MessageBuilder#topic(string topicName)` may be more common in cpp.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r827020573



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/Message.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.producer.Producer;
+
+/**
+ * Abstract message only used for {@link Producer}.
+ */
+public interface Message {
+    /**
+     * Get the topic of message, which is the first classifier for message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();
+
+    /**
+     * Get the <strong>deep copy</strong> of message properties.
+     *
+     * @return the <strong>deep copy</strong> of message properties.
+     */
+    Map<String, String> getProperties();
+
+    /**
+     * Get the tag of message, which is the second classifier besides topic.
+     *
+     * @return the tag of message.
+     */
+    Optional<String> getTag();
+
+    /**
+     * Get the key collection of message.
+     *
+     * @return <strong>the key collection</strong> of message.
+     */
+    Collection<String> getKeys();
+
+    /**
+     * Get the message group, which make sense only when topic type is fifo.
+     *
+     * @return message group, which is optional.
+     */
+    Optional<String> getMessageGroup();
+
+    /**
+     * Get the expected delivery timestamp, which make sense only when topic type is delay.
+     *
+     * @return message expected delivery timestamp, which is optional.
+     */
+    Optional<Long> getDeliveryTimestamp();

Review comment:
       > Is there a corresponding setter for deliverytime ? Why is this property exposed separately?
   
   I might lose setter for `deliveryTimestamp` in a commit, thinks for reminding me.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls commented on pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#issuecomment-1070867604






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on pull request #3987: Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#issuecomment-1066571436


   @zhouxinyu @lizhanhui @duhenglucky @RongtongJin @drpmma @chenzlalvin


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r825856429



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/Message.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.producer.Producer;
+
+/**
+ * Abstract message only used for {@link Producer}.
+ */
+public interface Message {
+    /**
+     * Get the topic of message, which is the first classifier for message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();
+
+    /**
+     * Get the <strong>deep copy</strong> of message properties.
+     *
+     * @return the <strong>deep copy</strong> of message properties.
+     */
+    Map<String, String> getProperties();
+
+    /**
+     * Get the tag of message, which is the second classifier besides topic.
+     *
+     * @return the tag of message.
+     */
+    Optional<String> getTag();
+
+    /**
+     * Get the key collection of message.
+     *
+     * @return <strong>the key collection</strong> of message.
+     */
+    Collection<String> getKeys();
+
+    /**
+     * Get the message group, which make sense only when topic type is fifo.
+     *
+     * @return message group, which is optional.
+     */
+    Optional<String> getMessageGroup();
+

Review comment:
       Message here is for producer, user may build message before sending, so Message#getMessageQeue may be not appropriate, but we may consider to add it into the return value of producer sending method.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] RongtongJin commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
RongtongJin commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r825856791



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageView.java
##########
@@ -0,0 +1,116 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.MessageQueue;
+
+public interface MessageView {
+    /**
+     * Get the unique id of message.
+     *
+     * @return unique id.
+     */
+    MessageId getMessageId();
+
+    /**
+     * Get the topic of message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the {@link MessageQueue} of message.
+     *
+     * @return message queue.
+     */
+    MessageQueue getMessageQueue();
+
+    /**
+     * Get the position of message in {@link MessageQueue}.
+     */
+    long getOffset();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body, which makes the modification of return value does not
+     * affect the message itself.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();
+
+    /**
+     * Get the <strong>deep copy</strong> of message properties, which makes the modification of return value does
+     * not affect the message itself.
+     *
+     * @return the <strong>deep copy</strong> of message properties.
+     */
+    Map<String, String> getProperties();
+
+    /**
+     * Get the tag of message, which is optional.
+     *
+     * @return the tag of message, which is optional.
+     */
+    Optional<String> getTag();
+
+    /**
+     * Get the key collection of message.
+     *
+     * @return <strong>the key collection</strong> of message.
+     */
+    Collection<String> getKeys();
+
+    /**
+     * Get the born host of message.
+     *
+     * @return born host of message.
+     */
+    String getBornHost();
+
+    /**
+     * Get the born timestamp of message.
+     *
+     * @return born timestamp of message.
+     */
+    long getBornTimestamp();
+
+    /**
+     * Get the message group, which is optional and only make sense only when topic type is fifo.
+     *
+     * @return message group, which is optional.
+     */
+    Optional<String> getMessageGroup();
+
+    /**
+     * Get the expected delivery timestamp, which make sense only when topic type id delay.

Review comment:
       'when topic type is delay', not 'when topic type id delay'

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageIdVersion.java
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.rocketmq.apis.message;
+
+public enum MessageIdVersion {

Review comment:
       Why are there multiple versions of messageId?

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/Message.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.producer.Producer;
+
+/**
+ * Abstract message only used for {@link Producer}.
+ */
+public interface Message {
+    /**
+     * Get the topic of message, which is the first classifier for message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();
+
+    /**
+     * Get the <strong>deep copy</strong> of message properties.
+     *
+     * @return the <strong>deep copy</strong> of message properties.
+     */
+    Map<String, String> getProperties();
+
+    /**
+     * Get the tag of message, which is the second classifier besides topic.
+     *
+     * @return the tag of message.
+     */
+    Optional<String> getTag();
+
+    /**
+     * Get the key collection of message.
+     *
+     * @return <strong>the key collection</strong> of message.
+     */
+    Collection<String> getKeys();
+
+    /**
+     * Get the message group, which make sense only when topic type is fifo.
+     *
+     * @return message group, which is optional.
+     */
+    Optional<String> getMessageGroup();
+
+    /**
+     * Get the expected delivery timestamp, which make sense only when topic type id delay.

Review comment:
       'when topic type is delay', not 'when topic type id delay'




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r825865274



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;

Review comment:
       SendResult is not essential, because exception may be a better way to indicate the result, but offset and message queue may be considered.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r825863705



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageIdVersion.java
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.rocketmq.apis.message;
+
+public enum MessageIdVersion {

Review comment:
       Well, the old version of messageId could not meet the unique constraint, that's why we have the different version of messageId.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] zhouxinyu commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826815913



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageId send(Message message, Transaction transaction) throws ClientException;
+
+    /**
+     * Send a message asynchronously.
+     *
+     * <p>This method returns immediately, the result is included in the {@link CompletableFuture};
+     *
+     * @param message message to send.
+     * @return a future that indicates the result.
+     */
+    CompletableFuture<MessageId> sendAsync(Message message);
+
+    /**
+     * Send batch messages synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * <p>All messages to send should have the same topic.
+     *
+     * @param messages batch messages to send.
+     * @return collection indicates the message id assigned to the appointed message, which keep the same order
+     * messages collection.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    Collection<MessageId> send(Collection<Message> messages) throws ClientException;
+
+    /**
+     * Begin transaction, which follows the
+     * <a href="https://en.wikipedia.org/wiki/Two-phase_commit_protocol">two-phase commit protocol</a>.
+     *
+     * @return a transaction entity to execute commit/rollback operation.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     */
+    Transaction beginTransaction() throws ClientException;

Review comment:
       Could you please provide some code samples? @WJL3333 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r825871976



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/Message.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.producer.Producer;
+
+/**
+ * Abstract message only used for {@link Producer}.
+ */
+public interface Message {
+    /**
+     * Get the topic of message, which is the first classifier for message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();
+
+    /**
+     * Get the <strong>deep copy</strong> of message properties.
+     *
+     * @return the <strong>deep copy</strong> of message properties.
+     */
+    Map<String, String> getProperties();
+
+    /**
+     * Get the tag of message, which is the second classifier besides topic.
+     *
+     * @return the tag of message.
+     */
+    Optional<String> getTag();
+
+    /**
+     * Get the key collection of message.
+     *
+     * @return <strong>the key collection</strong> of message.
+     */
+    Collection<String> getKeys();
+
+    /**
+     * Get the message group, which make sense only when topic type is fifo.
+     *
+     * @return message group, which is optional.
+     */
+    Optional<String> getMessageGroup();
+
+    /**
+     * Get the expected delivery timestamp, which make sense only when topic type id delay.

Review comment:
       Fixed




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r827066394



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,140 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageView;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageView send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageView send(Message message, Transaction transaction) throws ClientException;

Review comment:
       > These exceptions are unchecked, which means we don't want our users to use these exceptions to control their logic, so a simple ClientException is enough.
   
   First of all, exceptions are inevitable. In fact, about the issue that whether to use checked exception or not, maybe no need to prevent users to do responding operation by the exception thrown deliberately? For example, users just want to count how many times the network timeout exception occurred.
   
   In addition, about the issue to use error codes or specific exception, actuall they can all solve the problem in theory., here's my concern
    1. It is very easy to abuse error code.
    2. Not all exceptions will have the same parameters, such as requestId, they may only exist in some exceptions.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r827066394



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,140 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageView;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageView send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageView send(Message message, Transaction transaction) throws ClientException;

Review comment:
       > These exceptions are unchecked, which means we don't want our users to use these exceptions to control their logic, so a simple ClientException is enough.
   
   First of all, exceptions are inevitable. In fact, about the issue that whether to use checked exception or not, maybe no need to prevent users to do responding operation by the exception thrown deliberately? For example, users just want to count how many times the network timeout exception occurred. .
   
   In addition, about the issue to use error codes or specific exception, actuall they can all solve the problem in theory., here's my concern
    1. It is very easy to abuse error code.
    2. Not all exceptions will have the same parameters, such as requestId, they may only exist in some exceptions.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r825870597



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageView.java
##########
@@ -0,0 +1,116 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.MessageQueue;
+
+public interface MessageView {
+    /**
+     * Get the unique id of message.
+     *
+     * @return unique id.
+     */
+    MessageId getMessageId();
+
+    /**
+     * Get the topic of message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the {@link MessageQueue} of message.
+     *
+     * @return message queue.
+     */
+    MessageQueue getMessageQueue();
+
+    /**
+     * Get the position of message in {@link MessageQueue}.
+     */
+    long getOffset();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body, which makes the modification of return value does not
+     * affect the message itself.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();
+
+    /**
+     * Get the <strong>deep copy</strong> of message properties, which makes the modification of return value does
+     * not affect the message itself.
+     *
+     * @return the <strong>deep copy</strong> of message properties.
+     */
+    Map<String, String> getProperties();
+
+    /**
+     * Get the tag of message, which is optional.
+     *
+     * @return the tag of message, which is optional.
+     */
+    Optional<String> getTag();
+
+    /**
+     * Get the key collection of message.
+     *
+     * @return <strong>the key collection</strong> of message.
+     */
+    Collection<String> getKeys();
+
+    /**
+     * Get the born host of message.
+     *
+     * @return born host of message.
+     */
+    String getBornHost();
+
+    /**
+     * Get the born timestamp of message.
+     *
+     * @return born timestamp of message.
+     */
+    long getBornTimestamp();
+
+    /**
+     * Get the message group, which is optional and only make sense only when topic type is fifo.
+     *
+     * @return message group, which is optional.
+     */
+    Optional<String> getMessageGroup();
+
+    /**
+     * Get the expected delivery timestamp, which make sense only when topic type id delay.

Review comment:
       Fixed




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r827016443



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);
+
+    /**
+     * Declare topics ahead of message sending/preparation.
+     *
+     * <p>Even though the declaration is not essential, we <strong>highly recommend</strong> to declare the topics in
+     * advance, which could help to discover potential mistakes.
+     *
+     * @param topics topics to send/prepare.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder withTopics(String... topics);
+
+    /**
+     * Set the threads count for {@link Producer#sendAsync(Message)}.
+     *
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setAsyncThreadCount(int count);
+
+    /**
+     * Set the retry policy to send message.
+     *
+     * @param retryPolicy policy to re-send message when failure encountered.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setRetryPolicy(BackoffRetryPolicy retryPolicy);
+
+    /**
+     * Set the transaction checker for producer.
+     *
+     * @param checker transaction checker.
+     * @return the produce builder instance.
+     */
+    ProducerBuilder setTransactionChecker(TransactionChecker checker);
+
+    /**
+     * Finalize the build of {@link Producer} instance and start.
+     *
+     * <p>The producer does a series of preparatory work during startup, which could help to identify more unexpected
+     * error earlier.
+     *
+     * <p>Especially, if this method is invoked more than once, different producer will be created and started.
+     *
+     * @return the producer instance.
+     * @throws TopicDoesNotExistException if there are nonexistent topic(s).
+     * @throws AuthorisationException     if no permission to communicate with server.
+     * @throws AuthenticationException    if identification could not be recognized by server.
+     * @throws NetworkTimeoutException    if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException if there is a network connection problem.
+     */
+    Producer start() throws ClientException;

Review comment:
       > It's strange if we don't have a `build` method in a builder interface.
   > 
   > If we only provide a `start` method in the producer builder, that means all the producers are started by default, so why we should have a `start`?
   
   I agree with the opinon that lack of `#build()` may be a little bit weird, let's look at it another way. Is it unnecessary for the user to start this process? If so, then I think it's just a naming issue.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] hill007299 commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
hill007299 commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r825881515



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/Message.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.producer.Producer;
+
+/**
+ * Abstract message only used for {@link Producer}.
+ */
+public interface Message {
+    /**
+     * Get the topic of message, which is the first classifier for message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();
+
+    /**
+     * Get the <strong>deep copy</strong> of message properties.
+     *
+     * @return the <strong>deep copy</strong> of message properties.
+     */
+    Map<String, String> getProperties();
+
+    /**
+     * Get the tag of message, which is the second classifier besides topic.
+     *
+     * @return the tag of message.
+     */
+    Optional<String> getTag();
+
+    /**
+     * Get the key collection of message.
+     *
+     * @return <strong>the key collection</strong> of message.
+     */
+    Collection<String> getKeys();
+
+    /**
+     * Get the message group, which make sense only when topic type is fifo.
+     *
+     * @return message group, which is optional.
+     */
+    Optional<String> getMessageGroup();
+
+    /**
+     * Get the expected delivery timestamp, which make sense only when topic type is delay.
+     *
+     * @return message expected delivery timestamp, which is optional.
+     */
+    Optional<Long> getDeliveryTimestamp();

Review comment:
       Is there a corresponding setter for deliverytime ? Why is this property exposed separately?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r827066394



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,140 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageView;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageView send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageView send(Message message, Transaction transaction) throws ClientException;

Review comment:
       > These exceptions are unchecked, which means we don't want our users to use these exceptions to control their logic, so a simple ClientException is enough.
   
   First of all, exceptions are inevitable. In fact, about the issue that whether to use checked exception or not, maybe no need to prevent users to do responding operation by the exception thrown deliberately? For example, users just want to count how many times the network timeout exception occurred.
   
   In addition, about the issue to use error codes or specific exception, actuall they can all solve the problem in theory, here's my concern
    1. It is very easy to abuse error code.
    2. Not all exceptions will have the same parameters, such as requestId, they may only exist in some exceptions.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r825859256



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageId send(Message message, Transaction transaction) throws ClientException;

Review comment:
       Well, we may send more than one message in single transaction.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826747012



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/exception/ClientException.java
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.rocketmq.apis.exception;
+
+public class ClientException extends Exception {

Review comment:
       Yes, we add `abstract` qualifier into `ClientException`, which makes it could be constructed directly, and java docs also has been added.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r827016443



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);
+
+    /**
+     * Declare topics ahead of message sending/preparation.
+     *
+     * <p>Even though the declaration is not essential, we <strong>highly recommend</strong> to declare the topics in
+     * advance, which could help to discover potential mistakes.
+     *
+     * @param topics topics to send/prepare.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder withTopics(String... topics);
+
+    /**
+     * Set the threads count for {@link Producer#sendAsync(Message)}.
+     *
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setAsyncThreadCount(int count);
+
+    /**
+     * Set the retry policy to send message.
+     *
+     * @param retryPolicy policy to re-send message when failure encountered.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setRetryPolicy(BackoffRetryPolicy retryPolicy);
+
+    /**
+     * Set the transaction checker for producer.
+     *
+     * @param checker transaction checker.
+     * @return the produce builder instance.
+     */
+    ProducerBuilder setTransactionChecker(TransactionChecker checker);
+
+    /**
+     * Finalize the build of {@link Producer} instance and start.
+     *
+     * <p>The producer does a series of preparatory work during startup, which could help to identify more unexpected
+     * error earlier.
+     *
+     * <p>Especially, if this method is invoked more than once, different producer will be created and started.
+     *
+     * @return the producer instance.
+     * @throws TopicDoesNotExistException if there are nonexistent topic(s).
+     * @throws AuthorisationException     if no permission to communicate with server.
+     * @throws AuthenticationException    if identification could not be recognized by server.
+     * @throws NetworkTimeoutException    if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException if there is a network connection problem.
+     */
+    Producer start() throws ClientException;

Review comment:
       > It's strange if we don't have a `build` method in a builder interface.
   > 
   > If we only provide a `start` method in the producer builder, that means all the producers are started by default, so why we should have a `start`?
   
   I agree with the opinon that lack of `#build()` may be a little bit weird, let's look at it another way. Is it unnecessary for the user to start producer manually? If so, then I think it's just a naming issue.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] zhouxinyu commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826910464



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,140 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageView;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageView send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageView send(Message message, Transaction transaction) throws ClientException;

Review comment:
       Tons of exceptions are thrown, it's daunting.
   
   I suggest we only declare a ClientException with managed code, request-id, message, etc.
   
   These exceptions are unchecked, which means we don't want our users to use these exceptions to control their logic, so a simple ClientException is enough.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] WJL3333 commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
WJL3333 commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826586924



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageBuilder.java
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import org.apache.rocketmq.apis.producer.Producer;
+
+/**
+ * Builder to config {@link Message}.
+ */
+public interface MessageBuilder {
+    /**
+     * Set the topic for message.
+     *
+     * @param topic topic for the message.
+     * @return the message builder instance.
+     */
+    MessageBuilder setTopic(String topic);

Review comment:
       if we remove set prefix the api will be more clean to the developer.

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageView.java
##########
@@ -0,0 +1,116 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.MessageQueue;
+
+public interface MessageView {
+    /**
+     * Get the unique id of message.
+     *
+     * @return unique id.
+     */
+    MessageId getMessageId();
+
+    /**
+     * Get the topic of message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the {@link MessageQueue} of message.
+     *
+     * @return message queue.
+     */
+    MessageQueue getMessageQueue();
+
+    /**
+     * Get the position of message in {@link MessageQueue}.
+     */
+    long getOffset();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body, which makes the modification of return value does not
+     * affect the message itself.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();

Review comment:
       deep copy seems will harm the performance. but there seems no better return type here. 

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageId send(Message message, Transaction transaction) throws ClientException;
+
+    /**
+     * Send a message asynchronously.
+     *
+     * <p>This method returns immediately, the result is included in the {@link CompletableFuture};
+     *
+     * @param message message to send.
+     * @return a future that indicates the result.
+     */
+    CompletableFuture<MessageId> sendAsync(Message message);
+
+    /**
+     * Send batch messages synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * <p>All messages to send should have the same topic.
+     *
+     * @param messages batch messages to send.
+     * @return collection indicates the message id assigned to the appointed message, which keep the same order
+     * messages collection.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    Collection<MessageId> send(Collection<Message> messages) throws ClientException;
+
+    /**
+     * Begin transaction, which follows the
+     * <a href="https://en.wikipedia.org/wiki/Two-phase_commit_protocol">two-phase commit protocol</a>.
+     *
+     * @return a transaction entity to execute commit/rollback operation.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     */
+    Transaction beginTransaction() throws ClientException;
+
+    /**
+     * Close the producer and release all related resources.
+     *
+     * <p>This method does not return until all related resource is released. Once producer is closed, <strong>it could
+     * not be started once again.</strong> we maintained an FSM (finite-state machine) to record the different states
+     * for each producer, which is similar to {@link Service.State}.
+     */
+    @SuppressWarnings("UnstableApiUsage")
+    @Override
+    void close();

Review comment:
       will close throw exception here?

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);
+
+    /**
+     * Declare topics ahead of message sending/preparation.
+     *
+     * <p>Even though the declaration is not essential, we <strong>highly recommend</strong> to declare the topics in
+     * advance, which could help to discover potential mistakes.
+     *
+     * @param topics topics to send/prepare.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder withTopics(String... topics);
+
+    /**
+     * Set the threads count for {@link Producer#sendAsync(Message)}.
+     *
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setAsyncThreadCount(int count);
+
+    /**
+     * Set the retry policy to send message.
+     *
+     * @param retryPolicy policy to re-send message when failure encountered.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setRetryPolicy(BackoffRetryPolicy retryPolicy);

Review comment:
       `RetryPolicy` interface here will be better. 

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageView.java
##########
@@ -0,0 +1,116 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.MessageQueue;
+
+public interface MessageView {

Review comment:
       need java doc here. what is an MessageView ?

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/exception/ClientException.java
##########
@@ -0,0 +1,32 @@
+/*
+ * 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.rocketmq.apis.exception;
+
+public class ClientException extends Exception {

Review comment:
       it seems this is the root Exception for all the rocketmq client exception. when we will throw this exception.
   if maintainer want to add an exception which kind of exception can extend this clientException.
   
   add java doc will be more formal

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageId.java
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.rocketmq.apis.message;
+
+/**
+ * Abstract message id, the implement must override {@link Object#toString()}, which indicates the message id using
+ * string form.
+ */
+public interface MessageId {
+    /**
+     * Get the version of message id.
+     *
+     * @return the version of message id.
+     */
+    MessageIdVersion getVersion();
+
+    /**
+     * The implementation <strong>must</strong> override this method, which indicates the message id using string form.
+     *
+     * @return string-formed string id.
+     */
+    String toString();

Review comment:
       this name is the same as the java origin Object.toString. i think we can change the method name. the origin is anti-parttern naming.

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageId send(Message message, Transaction transaction) throws ClientException;
+
+    /**
+     * Send a message asynchronously.
+     *
+     * <p>This method returns immediately, the result is included in the {@link CompletableFuture};
+     *
+     * @param message message to send.
+     * @return a future that indicates the result.
+     */
+    CompletableFuture<MessageId> sendAsync(Message message);
+
+    /**
+     * Send batch messages synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * <p>All messages to send should have the same topic.
+     *
+     * @param messages batch messages to send.
+     * @return collection indicates the message id assigned to the appointed message, which keep the same order
+     * messages collection.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    Collection<MessageId> send(Collection<Message> messages) throws ClientException;
+
+    /**
+     * Begin transaction, which follows the
+     * <a href="https://en.wikipedia.org/wiki/Two-phase_commit_protocol">two-phase commit protocol</a>.
+     *
+     * @return a transaction entity to execute commit/rollback operation.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     */
+    Transaction beginTransaction() throws ClientException;

Review comment:
       maybe an interface extand producer add some Transaction relate method will be better. 

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);

Review comment:
       same as Messagebuilder. no set prefix will make api more clean in fluent api call.

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);
+
+    /**
+     * Declare topics ahead of message sending/preparation.
+     *
+     * <p>Even though the declaration is not essential, we <strong>highly recommend</strong> to declare the topics in
+     * advance, which could help to discover potential mistakes.
+     *
+     * @param topics topics to send/prepare.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder withTopics(String... topics);
+
+    /**
+     * Set the threads count for {@link Producer#sendAsync(Message)}.
+     *
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setAsyncThreadCount(int count);
+
+    /**
+     * Set the retry policy to send message.
+     *
+     * @param retryPolicy policy to re-send message when failure encountered.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setRetryPolicy(BackoffRetryPolicy retryPolicy);
+
+    /**
+     * Set the transaction checker for producer.
+     *
+     * @param checker transaction checker.
+     * @return the produce builder instance.
+     */
+    ProducerBuilder setTransactionChecker(TransactionChecker checker);
+
+    /**
+     * Finalize the build of {@link Producer} instance and start.
+     *
+     * <p>The producer does a series of preparatory work during startup, which could help to identify more unexpected
+     * error earlier.
+     *
+     * <p>Especially, if this method is invoked more than once, different producer will be created and started.
+     *
+     * @return the producer instance.
+     * @throws TopicDoesNotExistException if there are nonexistent topic(s).
+     * @throws AuthorisationException     if no permission to communicate with server.
+     * @throws AuthenticationException    if identification could not be recognized by server.
+     * @throws NetworkTimeoutException    if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException if there is a network connection problem.
+     */
+    Producer start() throws ClientException;

Review comment:
       may be move start to the producer.
   some case will just build the producer without start.

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Transaction.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+/**
+ * An entity to describe an independent transaction, which follows
+ * <a href="https://en.wikipedia.org/wiki/Two-phase_commit_protocol">two-phase commit protocol</a>.
+ *
+ * <p>once {@link Transaction#commit()} or {@link Transaction#rollback()} is invoked, subsequent commit or rollback in
+ * {@link Transaction} is ignored by client.
+ *
+ * <p>Neither of {@link Transaction#commit()} and {@link Transaction#rollback()} ensures the success on account of
+ * network timeout or other issues, that's why it does not make any sense to execute {@link Transaction#commit()} or
+ * {@link Transaction#rollback()} after is has been executed once already. The suspended transaction will be
+ * solved by {@link TransactionChecker}.
+ */
+public interface Transaction {
+    /**
+     * Try to commit the transaction, which would expose the message before the transaction is closed.
+     *
+     * <p>We don't ensure this operation is successful even though no exception is thrown after invocation,
+     * <strong>actually we omit the exception on purpose because {@link TransactionChecker} is the unique right way
+     * to solve the suspended transaction rather than commit or roll-back repeatedly.</strong>
+     */
+    void commit();

Review comment:
       will commit throw exception need the caller process?

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageView.java
##########
@@ -0,0 +1,116 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.MessageQueue;
+
+public interface MessageView {
+    /**
+     * Get the unique id of message.
+     *
+     * @return unique id.
+     */
+    MessageId getMessageId();
+
+    /**
+     * Get the topic of message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the {@link MessageQueue} of message.
+     *
+     * @return message queue.
+     */
+    MessageQueue getMessageQueue();
+
+    /**
+     * Get the position of message in {@link MessageQueue}.
+     */
+    long getOffset();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body, which makes the modification of return value does not
+     * affect the message itself.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();

Review comment:
       how about ByteBuffer here which can be set as unmodifable




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] dongeforever commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
dongeforever commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826613463



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/Message.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.producer.Producer;
+
+/**
+ * Abstract message only used for {@link Producer}.
+ */
+public interface Message {
+    /**
+     * Get the topic of message, which is the first classifier for message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();
+
+    /**
+     * Get the <strong>deep copy</strong> of message properties.
+     *
+     * @return the <strong>deep copy</strong> of message properties.
+     */
+    Map<String, String> getProperties();
+
+    /**
+     * Get the tag of message, which is the second classifier besides topic.
+     *
+     * @return the tag of message.
+     */
+    Optional<String> getTag();
+
+    /**
+     * Get the key collection of message.
+     *
+     * @return <strong>the key collection</strong> of message.
+     */
+    Collection<String> getKeys();
+
+    /**
+     * Get the message group, which make sense only when topic type is fifo.
+     *
+     * @return message group, which is optional.
+     */
+    Optional<String> getMessageGroup();
+

Review comment:
       I repeat this:
   Except for the admin operation, the abilities of the new API should be a superset of the old API.
   
   The old API could send messages to the specified message queue?
   
   So how to do it in the new API?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826709037



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/exception/AuthorisationException.java
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.rocketmq.apis.exception;
+
+public class AuthorisationException extends ClientException {

Review comment:
       > More javadocs will be added here.
   
   done




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826743469



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageId send(Message message, Transaction transaction) throws ClientException;

Review comment:
       The code example has been added to the javadocs.

##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import java.util.Collection;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.MessageTypeDoesNotMatchException;
+import org.apache.rocketmq.apis.exception.PersistenceException;
+import org.apache.rocketmq.apis.exception.ProducerClosedAlreadyException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.exception.TransactionCheckerNotSetException;
+import org.apache.rocketmq.apis.message.Message;
+import org.apache.rocketmq.apis.message.MessageId;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException       if the topic of message does not exist.
+     * @throws AuthorisationException           if no permission to send message.
+     * @throws AuthenticationException          if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException   if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException if message type does not match with the topic.
+     * @throws NetworkTimeoutException          if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException       if there is a network connection problem.
+     * @throws PersistenceException             if encountered persistence failure from server.
+     */
+    MessageId send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     * @throws TopicDoesNotExistException        if the topic of message does not exist.
+     * @throws AuthorisationException            if no permission to send message.
+     * @throws AuthenticationException           if identification could not be recognized by server.
+     * @throws ProducerClosedAlreadyException    if producer is closed already.
+     * @throws MessageTypeDoesNotMatchException  if message type does not match with the topic.
+     * @throws NetworkTimeoutException           if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException        if there is a network connection problem.
+     * @throws PersistenceException              if encountered persistence failure from server.
+     * @throws TransactionCheckerNotSetException if {@link TransactionChecker} is not set.
+     */
+    MessageId send(Message message, Transaction transaction) throws ClientException;

Review comment:
       > How to send more than one message using such API? MessageId send(Message message, Transaction transaction)
   > 
   > The old API for the transaction is not naturally two-phase based. It's better to polish it in the new API. But currently, the new API is just like the old
   
   The code example has been added to the javadocs.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] coveralls edited a comment on pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#issuecomment-1070867608






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826595560



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/ProducerBuilder.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import org.apache.rocketmq.apis.ClientConfiguration;
+import org.apache.rocketmq.apis.retry.BackoffRetryPolicy;
+import org.apache.rocketmq.apis.exception.AuthenticationException;
+import org.apache.rocketmq.apis.exception.AuthorisationException;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.exception.NetworkConnectionException;
+import org.apache.rocketmq.apis.exception.NetworkTimeoutException;
+import org.apache.rocketmq.apis.exception.TopicDoesNotExistException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Builder to config and start {@link Producer}.
+ */
+public interface ProducerBuilder {
+    /**
+     * Set the client configuration for producer.
+     *
+     * @param clientConfiguration client's configuration.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setClientConfiguration(ClientConfiguration clientConfiguration);
+
+    /**
+     * Declare topics ahead of message sending/preparation.
+     *
+     * <p>Even though the declaration is not essential, we <strong>highly recommend</strong> to declare the topics in
+     * advance, which could help to discover potential mistakes.
+     *
+     * @param topics topics to send/prepare.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder withTopics(String... topics);
+
+    /**
+     * Set the threads count for {@link Producer#sendAsync(Message)}.
+     *
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setAsyncThreadCount(int count);
+
+    /**
+     * Set the retry policy to send message.
+     *
+     * @param retryPolicy policy to re-send message when failure encountered.
+     * @return the producer builder instance.
+     */
+    ProducerBuilder setRetryPolicy(BackoffRetryPolicy retryPolicy);
+
+    /**
+     * Set the transaction checker for producer.
+     *
+     * @param checker transaction checker.
+     * @return the produce builder instance.
+     */
+    ProducerBuilder setTransactionChecker(TransactionChecker checker);
+
+    /**
+     * Finalize the build of {@link Producer} instance and start.
+     *
+     * <p>The producer does a series of preparatory work during startup, which could help to identify more unexpected
+     * error earlier.
+     *
+     * <p>Especially, if this method is invoked more than once, different producer will be created and started.
+     *
+     * @return the producer instance.
+     * @throws TopicDoesNotExistException if there are nonexistent topic(s).
+     * @throws AuthorisationException     if no permission to communicate with server.
+     * @throws AuthenticationException    if identification could not be recognized by server.
+     * @throws NetworkTimeoutException    if encountered network timeout to communicate with server.
+     * @throws NetworkConnectionException if there is a network connection problem.
+     */
+    Producer start() throws ClientException;

Review comment:
       Actually producerBuilder#start could avoid to start producer repeatedly. What's more, a producer which is not started does not make sense.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826596447



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/exception/AuthorisationException.java
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.rocketmq.apis.exception;
+
+public class AuthorisationException extends ClientException {

Review comment:
       More javadocs will be added here.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826704059



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageId.java
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.rocketmq.apis.message;
+
+/**
+ * Abstract message id, the implement must override {@link Object#toString()}, which indicates the message id using
+ * string form.
+ */
+public interface MessageId {
+    /**
+     * Get the version of message id.
+     *
+     * @return the version of message id.
+     */
+    MessageIdVersion getVersion();
+
+    /**
+     * The implementation <strong>must</strong> override this method, which indicates the message id using string form.
+     *
+     * @return string-formed string id.
+     */
+    String toString();

Review comment:
       `#toString()` here aims to emphasize that the implementation must override this method, maybe there is a better solution?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826704059



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/MessageId.java
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.rocketmq.apis.message;
+
+/**
+ * Abstract message id, the implement must override {@link Object#toString()}, which indicates the message id using
+ * string form.
+ */
+public interface MessageId {
+    /**
+     * Get the version of message id.
+     *
+     * @return the version of message id.
+     */
+    MessageIdVersion getVersion();
+
+    /**
+     * The implementation <strong>must</strong> override this method, which indicates the message id using string form.
+     *
+     * @return string-formed string id.
+     */
+    String toString();

Review comment:
       `#toString()` here aims to emphasize that **the implementation must override this method**, maybe there is a better solution?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] zhouxinyu commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r826786327



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/message/Message.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rocketmq.apis.message;
+
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import org.apache.rocketmq.apis.producer.Producer;
+
+/**
+ * Abstract message only used for {@link Producer}.
+ */
+public interface Message {
+    /**
+     * Get the topic of message, which is the first classifier for message.
+     *
+     * @return topic of message.
+     */
+    String getTopic();
+
+    /**
+     * Get the <strong>deep copy</strong> of message body.
+     *
+     * @return the <strong>deep copy</strong> of message body.
+     */
+    byte[] getBody();
+
+    /**
+     * Get the <strong>deep copy</strong> of message properties.
+     *
+     * @return the <strong>deep copy</strong> of message properties.
+     */
+    Map<String, String> getProperties();
+
+    /**
+     * Get the tag of message, which is the second classifier besides topic.
+     *
+     * @return the tag of message.
+     */
+    Optional<String> getTag();
+
+    /**
+     * Get the key collection of message.
+     *
+     * @return <strong>the key collection</strong> of message.
+     */
+    Collection<String> getKeys();
+
+    /**
+     * Get the message group, which make sense only when topic type is fifo.
+     *
+     * @return message group, which is optional.
+     */
+    Optional<String> getMessageGroup();
+

Review comment:
       ```
   the abilities of the new API should be a superset of the old API
   ```
   We don't follow this rule, many useless or low-frequency abilities will be removed in the new APIs, the new APIs don't expand the old APIs.
   
   As for sending messages to a specific queue, we were going to add it in the future version. Consider `MessageQueue` is a major model of RocketMQ, we may need to add it in the first version. But, I am not sure it's the right choice, hope more contributors could express opinions on this topic, especially from user perspectives.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] zhouxinyu commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r828802965



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     */
+    SendReceipt send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     */
+    SendReceipt send(Message message, Transaction transaction) throws ClientException;
+
+    /**
+     * Sends a message asynchronously.
+     *
+     * <p>This method returns immediately, the result is included in the {@link CompletableFuture};
+     *
+     * @param message message to send.
+     * @return a future that indicates the result.
+     */
+    CompletableFuture<SendReceipt> sendAsync(Message message);
+
+    /**
+     * Sends batch messages synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * <p>All messages to send should have the same topic.
+     *
+     * @param messages batch messages to send.
+     * @return collection indicates the message id assigned to the appointed message, which keep the same order
+     * messages collection.
+     */
+    List<SendReceipt> send(List<Message> messages) throws ClientException;
+
+    /**
+     * Begins a transaction.
+     *
+     * <p>For example:
+     *
+     * <pre>{@code
+     * Transaction transaction = producer.beginTransaction();
+     * MessageView messageView1 = producer.send(message1, transaction);
+     * MessageView messageView2 = producer.send(message2, transaction);

Review comment:
       Return SendReceipt




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [rocketmq] aaron-ai commented on a change in pull request #3987: [RIP-37] Add new APIs for producer

Posted by GitBox <gi...@apache.org>.
aaron-ai commented on a change in pull request #3987:
URL: https://github.com/apache/rocketmq/pull/3987#discussion_r828996060



##########
File path: apis/src/main/java/org/apache/rocketmq/apis/producer/Producer.java
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.rocketmq.apis.producer;
+
+import com.google.common.util.concurrent.Service;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.io.Closeable;
+import org.apache.rocketmq.apis.exception.ClientException;
+import org.apache.rocketmq.apis.message.Message;
+
+/**
+ * Producer is a thread-safe rocketmq client which is used to publish messages.
+ *
+ * <p>On account of network timeout or other reasons, rocketmq producer only promised the at-least-once semantics.
+ * For producer, at-least-once semantics means potentially attempts are made at sending it, messages may be
+ * duplicated but not lost.
+ */
+public interface Producer extends Closeable {
+    /**
+     * Sends a message synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * @param message message to send.
+     */
+    SendReceipt send(Message message) throws ClientException;
+
+    /**
+     * Sends a transactional message synchronously.
+     *
+     * @param message     message to send.
+     * @param transaction transaction to bind.
+     * @return the message id assigned to the appointed message.
+     */
+    SendReceipt send(Message message, Transaction transaction) throws ClientException;
+
+    /**
+     * Sends a message asynchronously.
+     *
+     * <p>This method returns immediately, the result is included in the {@link CompletableFuture};
+     *
+     * @param message message to send.
+     * @return a future that indicates the result.
+     */
+    CompletableFuture<SendReceipt> sendAsync(Message message);
+
+    /**
+     * Sends batch messages synchronously.
+     *
+     * <p>This method does not return until it gets the definitive result.
+     *
+     * <p>All messages to send should have the same topic.
+     *
+     * @param messages batch messages to send.
+     * @return collection indicates the message id assigned to the appointed message, which keep the same order
+     * messages collection.
+     */
+    List<SendReceipt> send(List<Message> messages) throws ClientException;
+
+    /**
+     * Begins a transaction.
+     *
+     * <p>For example:
+     *
+     * <pre>{@code
+     * Transaction transaction = producer.beginTransaction();
+     * MessageView messageView1 = producer.send(message1, transaction);
+     * MessageView messageView2 = producer.send(message2, transaction);

Review comment:
       Done




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org