feedback-to-code/mozzarella huggingface dataset:
Repository: provectus/kafka-ui
Issue: Improvement for the pagination "Go to page" field logic
Labels: type/bug
good first issue
scope/frontend
status/accepted
status/confirmed

**Describe the bug**
Not allowed to decrease the "Go to page" value from 1, but possible to delete the value

**Set up**
https://www.kafka-ui.provectus.io/


**Steps to Reproduce**
<!-- We'd like you to provide an example setup (via docker-compose, helm, etc.) 
to reproduce the problem, especially with a complex setups. -->
Steps to reproduce the behavior:

1. Navigate to Topics
2. Delete the "Go to page" value


**Expected behavior**
Let to not allow too delete the "Go to page" fields value

**Screenshots**

https://user-images.githubusercontent.com/104780608/184827833-f3dfe350-7c97-4209-aded-f66108f4a232.mov



**Additional context**
<!--
(Add any other context about the problem here)
-->


Issue comments:
@Haarolean can you please assign this to me ?
Checked ok on http://master.internal.kafka-ui.provectus.io/ui/clusters/local/schemas
Works ok.

Pull request review comments:

Patch that resolved the issue:
diff --git a/kafka-ui-react-app/src/components/common/NewTable/Table.tsx b/kafka-ui-react-app/src/components/common/NewTable/Table.tsx
index 3b3a780674c..646762bf548 100644
--- a/kafka-ui-react-app/src/components/common/NewTable/Table.tsx
+++ b/kafka-ui-react-app/src/components/common/NewTable/Table.tsx
@@ -190,6 +190,25 @@ const Table: React.FC<TableProps<any>> = ({
     return undefined;
   };
 
+  // Prevent Users to paste negative values in go to page field
+  const preventPasteNegative = (event: React.ClipboardEvent) => {
+    const { clipboardData } = event;
+    if (clipboardData != null) {
+      const pastedData = parseFloat(clipboardData.getData('text'));
+
+      if (pastedData < 0) {
+        event.preventDefault();
+      }
+    }
+  };
+
+  // Prevent Users to type negative values in go to page field
+  const preventMinus = (event: React.KeyboardEvent) => {
+    if (event.code === 'Minus') {
+      event.preventDefault();
+    }
+  };
+
   return (
     <>
       {table.getSelectedRowModel().flatRows.length > 0 && Bar && (
@@ -333,6 +352,8 @@ const Table: React.FC<TableProps<any>> = ({
                 inputSize="M"
                 max={table.getPageCount()}
                 min={1}
+                onKeyPress={preventMinus}
+                onPaste={preventPasteNegative}
                 onChange={({ target: { value } }) => {
                   const index = value ? Number(value) - 1 : 0;
                   table.setPageIndex(index);


Test patch:
Repository: provectus/kafka-ui
Issue: Add "Total lag" within Consumer group 
Labels: type/enhancement
good first issue
scope/frontend
status/accepted



**Describe the bug**
Add "Total lag" into Consumer group profile

**Set up**
https://www.kafka-ui.provectus.io/


**Screenshots**

<img width="1356" alt="total lag" src="https://user-images.githubusercontent.com/104780608/188556151-c77c8eba-0dd2-44b8-95c2-da5a8863a4aa.png">





Issue comments:
messagesBehind field in api request
I'd like to work on this task.

Pull request review comments:

Patch that resolved the issue:
diff --git a/kafka-ui-react-app/src/components/ConsumerGroups/Details/Details.tsx b/kafka-ui-react-app/src/components/ConsumerGroups/Details/Details.tsx
index 912b88a8a38..f1ce947fdb7 100644
--- a/kafka-ui-react-app/src/components/ConsumerGroups/Details/Details.tsx
+++ b/kafka-ui-react-app/src/components/ConsumerGroups/Details/Details.tsx
@@ -103,6 +103,9 @@ const Details: React.FC = () => {
           <Metrics.Indicator label="Coordinator ID">
             {consumerGroup.coordinator?.id}
           </Metrics.Indicator>
+          <Metrics.Indicator label="Total lag">
+            {consumerGroup.messagesBehind}
+          </Metrics.Indicator>
         </Metrics.Section>
       </Metrics.Wrapper>
       <Table isFullwidth>


Test patch:
Repository: provectus/kafka-ui
Issue: Add "Num of Topics" to Topic/Consumer tab
Labels: good first issue
scope/frontend
status/accepted
type/chore



**Describe the bug**
"Num of Topics" should be displayed within Topic/Consumer tab

**Set up**
https://www.kafka-ui.provectus.io/

**Steps to Reproduce**
<!-- We'd like you to provide an example setup (via docker-compose, helm, etc.) 
to reproduce the problem, especially with a complex setups. -->
Steps to reproduce the behavior:

1. Navigate to Topics
2. Open the Topic
3. Turn to Consumers tab


**Expected behavior**
The "num of Topics" should be displayed 

**Screenshots**
<img width="1717" alt="num of topics" src="https://user-images.githubusercontent.com/104780608/188837318-c4392509-87fc-4005-bc10-a9a97b40a023.png">






Issue comments:
`/api/clusters/<cluster>/topics/<topic>/consumer-groups`, field "topics".
I'd like to take this issue.

Pull request review comments:

Patch that resolved the issue:
diff --git a/kafka-ui-react-app/src/components/Topics/Topic/ConsumerGroups/TopicConsumerGroups.tsx b/kafka-ui-react-app/src/components/Topics/Topic/ConsumerGroups/TopicConsumerGroups.tsx
index 82551ac2502..d1d6c4ece2c 100644
--- a/kafka-ui-react-app/src/components/Topics/Topic/ConsumerGroups/TopicConsumerGroups.tsx
+++ b/kafka-ui-react-app/src/components/Topics/Topic/ConsumerGroups/TopicConsumerGroups.tsx
@@ -22,6 +22,7 @@ const TopicConsumerGroups: React.FC = () => {
         <tr>
           <TableHeaderCell title="Consumer Group ID" />
           <TableHeaderCell title="Num Of Members" />
+          <TableHeaderCell title="Num Of Topics" />
           <TableHeaderCell title="Messages Behind" />
           <TableHeaderCell title="Coordinator" />
           <TableHeaderCell title="State" />
@@ -40,6 +41,7 @@ const TopicConsumerGroups: React.FC = () => {
               </Link>
             </TableKeyLink>
             <td>{consumer.members}</td>
+            <td>{consumer.topics}</td>
             <td>{consumer.messagesBehind}</td>
             <td>{consumer.coordinator?.id}</td>
             <td>


Test patch:
Repository: provectus/kafka-ui
Issue: Add message about not existing offline clusters in Dashboard
Labels: good first issue
scope/frontend
status/accepted
type/chore


**Describe the bug**
In case of missing offline clusters, user should see the corresponding message with toggling the offline clusters in Dashboard

**Set up**
https://www.kafka-ui.provectus.io/


**Steps to Reproduce**

1. Turn the "Only offline clusters" on
2. Make sure no clusters are available


**Expected behavior**
Should be shown "No Offline Clusters" message

**Screenshots**


<img width="1719" alt="disk usage data" src="https://user-images.githubusercontent.com/104780608/194301856-133ce9ee-df79-4734-86d6-6150ac088839.png">


Issue comments:
I reproduce the problem. It's showing like the screenshot down below. Is this issue still need to be modified? or it can be closed.
![image](https://user-images.githubusercontent.com/113582273/196322270-3d89b7fe-9708-4637-a667-11117adbb36d.png)

@winnie-chiu you're running an old version of the app, try `master`-labeled docker image instead of `latest`

Pull request review comments:
```suggestion
        emptyMessage="No clusters are online"
```

Patch that resolved the issue:
diff --git a/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx b/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx
index f1aa4988112..966edecf1fd 100644
--- a/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx
+++ b/kafka-ui-react-app/src/components/Brokers/BrokersList/BrokersList.tsx
@@ -193,7 +193,7 @@ const BrokersList: React.FC = () => {
         onRowClick={({ original: { brokerId } }) =>
           navigate(clusterBrokerPath(clusterName, brokerId))
         }
-        emptyMessage="Disk usage data not available"
+        emptyMessage="No clusters are online"
       />
     </>
   );
diff --git a/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx b/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx
index cf36c64059d..0c60cf4749c 100644
--- a/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx
+++ b/kafka-ui-react-app/src/components/Brokers/BrokersList/__test__/BrokersList.spec.tsx
@@ -145,7 +145,7 @@ describe('BrokersList Component', () => {
           renderComponent();
           expect(screen.getByRole('table')).toBeInTheDocument();
           expect(
-            screen.getByRole('row', { name: 'Disk usage data not available' })
+            screen.getByRole('row', { name: 'No clusters are online' })
           ).toBeInTheDocument();
         });
       });


Test patch:
Repository: netty/netty
Issue: QueryStringDecoder should also support ;
Labels: improvement

QueryStringDecoder currently does not parse
field1=value1;field2=value2
correctly.

http://en.wikipedia.org/wiki/Query_string says
This convention is a W3C recommendation.[3] W3C recommends that all web servers support semicolon separators in the place of ampersand separators.


Issue comments:
We love patches ;)

I think we can preprocess by converting all ; to &. Is that OK?

yes sounds right to me


Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/org/jboss/netty/handler/codec/http/QueryStringDecoder.java b/src/main/java/org/jboss/netty/handler/codec/http/QueryStringDecoder.java
index 743875e5b80..94d2715923a 100644
--- a/src/main/java/org/jboss/netty/handler/codec/http/QueryStringDecoder.java
+++ b/src/main/java/org/jboss/netty/handler/codec/http/QueryStringDecoder.java
@@ -30,9 +30,11 @@
  * Splits an HTTP query string into a path string and key-value parameter pairs.
  * This decoder is for one time use only.  Create a new instance for each URI:
  * <pre>
- * {@link QueryStringDecoder} decoder = new {@link QueryStringDecoder}("/hello?recipient=world");
+ * {@link QueryStringDecoder} decoder = new {@link QueryStringDecoder}("/hello?recipient=world&x=1;y=2");
  * assert decoder.getPath().equals("/hello");
  * assert decoder.getParameters().get("recipient").equals("world");
+ * assert decoder.getParameters().get("x").equals("1");
+ * assert decoder.getParameters().get("y").equals("2");
  * </pre>
  *
  * @author <a href="http://www.jboss.org/netty/">The Netty Project</a>
@@ -73,7 +75,8 @@ public QueryStringDecoder(String uri, Charset charset) {
             throw new NullPointerException("charset");
         }
 
-        this.uri = uri;
+        // http://en.wikipedia.org/wiki/Query_string
+        this.uri = uri.replace(';', '&');
         this.charset = charset;
     }
 
@@ -97,7 +100,8 @@ public QueryStringDecoder(URI uri, Charset charset){
             throw new NullPointerException("charset");
         }
 
-        this.uri = uri.toASCIIString();
+        // http://en.wikipedia.org/wiki/Query_string
+        this.uri = uri.toASCIIString().replace(';', '&');
         this.charset = charset;
     }
 


Test patch:
Repository: netty/netty
Issue: ZlibDecoder does not support preset dictionary

Attempting to create a ZlibDecoder using the preset dictionary constructor throws a compression exception:

org.jboss.netty.handler.codec.compression.CompressionException: failed to set the dictionary (-2)

z.inflateSetDictionary should not be called until the inflate returns JZlib.Z_NEED_DICT


Issue comments:
Both pull requests merged.. Thanks!


Pull request review comments:

Patch that resolved the issue:
diff --git a/codec/src/main/java/io/netty/handler/codec/compression/ZlibDecoder.java b/codec/src/main/java/io/netty/handler/codec/compression/ZlibDecoder.java
index 1c89d42436b..7904ef5b60d 100644
--- a/codec/src/main/java/io/netty/handler/codec/compression/ZlibDecoder.java
+++ b/codec/src/main/java/io/netty/handler/codec/compression/ZlibDecoder.java
@@ -32,6 +32,7 @@
 public class ZlibDecoder extends OneToOneDecoder {
 
     private final ZStream z = new ZStream();
+    private byte[] dictionary;
     private volatile boolean finished;
 
     /**
@@ -72,17 +73,13 @@ public ZlibDecoder(byte[] dictionary) {
         if (dictionary == null) {
             throw new NullPointerException("dictionary");
         }
+        this.dictionary = dictionary;
 
         synchronized (z) {
             int resultCode;
             resultCode = z.inflateInit(JZlib.W_ZLIB);
             if (resultCode != JZlib.Z_OK) {
                 ZlibUtil.fail(z, "initialization failure", resultCode);
-            } else {
-                resultCode = z.inflateSetDictionary(dictionary, dictionary.length);
-                if (resultCode != JZlib.Z_OK) {
-                    ZlibUtil.fail(z, "failed to set the dictionary", resultCode);
-                }
             }
         }
     }
@@ -131,6 +128,16 @@ protected Object decode(ChannelHandlerContext ctx, Channel channel, Object msg)
                     z.next_out_index = 0;
 
                     switch (resultCode) {
+                    case JZlib.Z_NEED_DICT:
+                        if (dictionary == null) {
+                            ZlibUtil.fail(z, "decompression failure", resultCode);
+                        } else {
+                            resultCode = z.inflateSetDictionary(dictionary, dictionary.length);
+                            if (resultCode != JZlib.Z_OK) {
+                                ZlibUtil.fail(z, "failed to set the dictionary", resultCode);
+                            }
+                        }
+                        break;
                     case JZlib.Z_STREAM_END:
                         finished = true; // Do not decode anymore.
                         z.inflateEnd();


Test patch:
Repository: netty/netty
Issue: DefaultCookie equals and compareTo implementation wrong
Labels: defect

The equals and compareTo implementations in DefaultCookie are wrong. Sample code from the equals method:

```
    if (getPath() == null && that.getPath() != null) {
        return false;
    } else if (that.getPath() == null) {
        return false;
    }
    if (!getPath().equals(that.getPath())) {
        return false;
    }
```

If path on both this and that is null, then the above code returns false, even though the cookies may be equal. The problem is in the else if statement, it should also check if getPath() doesn't equal null.  This affects both the equals and the compareTo methods, with the path and domain properties.


Issue comments:
This bug has consequences on downstream projects depending on Netty:

https://play.lighthouseapp.com/projects/82401-play-20/tickets/511-adding-a-cookie-twice-to-a-result-doesnt-override-the-old-cookie

Pull request: https://github.com/netty/netty/pull/379

Merged into '3' and 'master'.  Thanks!


Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultCookie.java b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultCookie.java
index e94e5dc145f..cb5d3c6eaeb 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultCookie.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultCookie.java
@@ -273,7 +273,7 @@ public boolean equals(Object o) {
 
         if (getPath() == null && that.getPath() != null) {
             return false;
-        } else if (that.getPath() == null) {
+        } else if (that.getPath() == null && getPath() != null) {
             return false;
         }
         if (!getPath().equals(that.getPath())) {
@@ -282,7 +282,7 @@ public boolean equals(Object o) {
 
         if (getDomain() == null && that.getDomain() != null) {
             return false;
-        } else if (that.getDomain() == null) {
+        } else if (that.getDomain() == null && getDomain() != null) {
             return false;
         }
         return getDomain().equalsIgnoreCase(that.getDomain());
@@ -298,7 +298,7 @@ public int compareTo(Cookie c) {
 
         if (getPath() == null && c.getPath() != null) {
             return -1;
-        } else if (c.getPath() == null) {
+        } else if (c.getPath() == null && getPath() != null) {
             return 1;
         }
         v = getPath().compareTo(c.getPath());
@@ -308,7 +308,7 @@ public int compareTo(Cookie c) {
 
         if (getDomain() == null && c.getDomain() != null) {
             return -1;
-        } else if (c.getDomain() == null) {
+        } else if (c.getDomain() == null && getDomain() != null) {
             return 1;
         }
         v = getDomain().compareToIgnoreCase(c.getDomain());


Test patch:
Repository: netty/netty
Issue: AbstractDiskHttpData.renameTo() can fail when mmap'ing fails.
Labels: defect

See: http://stackoverflow.com/questions/11006968/netty-file-upload-out-of-memory-error

The current implementation of `renameTo` is not correct.  It has to copy data chunk by chunk (e.g. 8KiB) instead of issuing full transfer at once.


Issue comments:
So the JVM implementation is buggy (not the Netty one), since the transferTo method on FileChannel should be optimized carefully in JVM, taking in consideration the memory footprint. If I'm correct, this method is system optimized, so the reason we used it.
And for the record, the "renameTo" method of Netty first tries to use the simplest "renameTo" method on File directly, since it is the most efficient one, then if not possible (such as different FileSystem under the wood), then it must copy and delete manually.

So we need to make the second part fully manually and not using the FileChannel optimization... This is a sad news, as it will be less efficient than the native one.
Am I correct ?

Now the issue is the optimal chunk size. For nowadays, 8KB could be considered as a good tradeoff among all filesystems implementation...

I meant we can call transferTo() multiple times with smaller length parameter, rather than completely falling back to old I/O.

I update the proposal accordingly. Thank @trustin and @normanmaurer 

Should correct the issue...
Should we ask the user to check first before to close this issue?

Just close it and tell him that it will be part of 3.5.1.Final ;)

2012/6/15 Frédéric Brégier
reply@reply.github.com:

> Should correct the issue...
> Should we ask the user to check first before to close this issue?
> 
> ---
> 
> Reply to this email directly or view it on GitHub:
> https://github.com/netty/netty/issues/398#issuecomment-6353273


Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/org/jboss/netty/handler/codec/http/multipart/AbstractDiskHttpData.java b/src/main/java/org/jboss/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
index 0df3500de38..010bd1d55ff 100644
--- a/src/main/java/org/jboss/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
+++ b/src/main/java/org/jboss/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
@@ -289,10 +289,17 @@ public boolean renameTo(File dest) throws IOException {
             FileOutputStream outputStream = new FileOutputStream(dest);
             FileChannel in = inputStream.getChannel();
             FileChannel out = outputStream.getChannel();
-            long destsize = in.transferTo(0, size, out);
+            int chunkSize = 8196;
+            long position = 0;
+            while (position < size) {
+                if (chunksize < size - position) {
+                    chunksize = size - position;
+                }
+                position += in.transferTo(position, chunkSize , out);
+            }
             in.close();
             out.close();
-            if (destsize == size) {
+            if (position == size) {
                 file.delete();
                 file = dest;
                 isRenamed = true;


Test patch:
Repository: netty/netty
Issue: AbstractDiskHttpData.renameTo() can fail when mmap'ing fails.
Labels: defect

See: http://stackoverflow.com/questions/11006968/netty-file-upload-out-of-memory-error

The current implementation of `renameTo` is not correct.  It has to copy data chunk by chunk (e.g. 8KiB) instead of issuing full transfer at once.


Issue comments:
So the JVM implementation is buggy (not the Netty one), since the transferTo method on FileChannel should be optimized carefully in JVM, taking in consideration the memory footprint. If I'm correct, this method is system optimized, so the reason we used it.
And for the record, the "renameTo" method of Netty first tries to use the simplest "renameTo" method on File directly, since it is the most efficient one, then if not possible (such as different FileSystem under the wood), then it must copy and delete manually.

So we need to make the second part fully manually and not using the FileChannel optimization... This is a sad news, as it will be less efficient than the native one.
Am I correct ?

Now the issue is the optimal chunk size. For nowadays, 8KB could be considered as a good tradeoff among all filesystems implementation...

I meant we can call transferTo() multiple times with smaller length parameter, rather than completely falling back to old I/O.

I update the proposal accordingly. Thank @trustin and @normanmaurer 

Should correct the issue...
Should we ask the user to check first before to close this issue?

Just close it and tell him that it will be part of 3.5.1.Final ;)

2012/6/15 Frédéric Brégier
reply@reply.github.com:

> Should correct the issue...
> Should we ask the user to check first before to close this issue?
> 
> ---
> 
> Reply to this email directly or view it on GitHub:
> https://github.com/netty/netty/issues/398#issuecomment-6353273


Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/org/jboss/netty/handler/codec/http/multipart/AbstractDiskHttpData.java b/src/main/java/org/jboss/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
index 010bd1d55ff..e1400cae675 100644
--- a/src/main/java/org/jboss/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
+++ b/src/main/java/org/jboss/netty/handler/codec/http/multipart/AbstractDiskHttpData.java
@@ -292,8 +292,8 @@ public boolean renameTo(File dest) throws IOException {
             int chunkSize = 8196;
             long position = 0;
             while (position < size) {
-                if (chunksize < size - position) {
-                    chunksize = size - position;
+                if (chunkSize < size - position) {
+                    chunkSize = (int) (size - position);
                 }
                 position += in.transferTo(position, chunkSize , out);
             }


Test patch: