Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimize: GzipUtil code style #2140

Merged
merged 18 commits into from
Jan 21, 2020
Merged

Conversation

EZKAYOTWJPRKXWCUYEEB
Copy link
Contributor

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Jan 6, 2020

Codecov Report

Merging #2140 into develop will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2140      +/-   ##
=============================================
- Coverage      54.37%   54.36%   -0.01%     
+ Complexity      2470     2469       -1     
=============================================
  Files            447      447              
  Lines          14742    14742              
  Branches        1734     1734              
=============================================
- Hits            8016     8015       -1     
+ Misses          5967     5966       -1     
- Partials         759      761       +2
Impacted Files Coverage Δ Complexity Δ
...src/main/java/io/seata/server/ParameterParser.java 44.68% <0%> (-2.13%) 11 <1> (ø)
...in/java/io/seata/server/session/GlobalSession.java 84.13% <0%> (-0.49%) 67% <0%> (-1%)
...very/registry/zk/ZookeeperRegisterServiceImpl.java 63.7% <0%> (+0.8%) 24% <0%> (ø) ⬇️

Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some comments.

@EZKAYOTWJPRKXWCUYEEB
Copy link
Contributor Author

@xingfudeshi
Hello, the following code is the decompiled code. It can be clearly seen that he uses the close method of gzip.

public static byte[] compress(byte[] bytes) {
        if (bytes == null) {
            throw new NullPointerException("bytes is null");
        } else {
            try {
                ByteArrayOutputStream out = new ByteArrayOutputStream();
                Throwable var2 = null;

                Object var5;
                try {
                    GZIPOutputStream gzip = new GZIPOutputStream(out);
                    Throwable var4 = null;

                    try {
                        gzip.write(bytes);
                        gzip.flush();
                        gzip.finish();
                        var5 = out.toByteArray();
                    } catch (Throwable var30) {
                        var5 = var30;
                        var4 = var30;
                        throw var30;
                    } finally {
                        if (gzip != null) {
                            if (var4 != null) {
                                try {
                                    gzip.close();
                                } catch (Throwable var29) {
                                    var4.addSuppressed(var29);
                                }
                            } else {
                                gzip.close();
                            }
                        }

                    }
                } catch (Throwable var32) {
                    var2 = var32;
                    throw var32;
                } finally {
                    if (out != null) {
                        if (var2 != null) {
                            try {
                                out.close();
                            } catch (Throwable var28) {
                                var2.addSuppressed(var28);
                            }
                        } else {
                            out.close();
                        }
                    }

                }

                return (byte[])var5;
            } catch (IOException var34) {
                throw new RuntimeException("gzip compress error", var34);
            }
        }
    }

@EZKAYOTWJPRKXWCUYEEB
Copy link
Contributor Author

@xingfudeshi
decompress method is the same as compress method.

public static byte[] decompress(byte[] bytes) {
        if (bytes == null) {
            throw new NullPointerException("bytes is null");
        } else {
            try {
                ByteArrayOutputStream out = new ByteArrayOutputStream();
                Throwable var2 = null;

                try {
                    GZIPInputStream gunzip = new GZIPInputStream(new ByteArrayInputStream(bytes));
                    Throwable var4 = null;

                    try {
                        byte[] buffer = new byte[8192];

                        int n;
                        while((n = gunzip.read(buffer)) > -1) {
                            out.write(buffer, 0, n);
                        }

                        byte[] var7 = out.toByteArray();
                        return var7;
                    } catch (Throwable var32) {
                        var4 = var32;
                        throw var32;
                    } finally {
                        if (gunzip != null) {
                            if (var4 != null) {
                                try {
                                    gunzip.close();
                                } catch (Throwable var31) {
                                    var4.addSuppressed(var31);
                                }
                            } else {
                                gunzip.close();
                            }
                        }

                    }
                } catch (Throwable var34) {
                    var2 = var34;
                    throw var34;
                } finally {
                    if (out != null) {
                        if (var2 != null) {
                            try {
                                out.close();
                            } catch (Throwable var30) {
                                var2.addSuppressed(var30);
                            }
                        } else {
                            out.close();
                        }
                    }

                }
            } catch (IOException var36) {
                throw new RuntimeException("gzip decompress error", var36);
            }
        }
    }

@xingfudeshi
Copy link
Member

What i mean is that the use of try with resource statement is redundant for ByteArrayOutputStream or ByteArrayInputStream.Because the method close does not do anything.

@EZKAYOTWJPRKXWCUYEEB
Copy link
Contributor Author

What i mean is that the use of try with resource statement is redundant for ByteArrayOutputStream or ByteArrayInputStream.Because the method close does not do anything.

Oh, Im sorry, I will change my code.

Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@codecov-io
Copy link

codecov-io commented Jan 16, 2020

Codecov Report

Merging #2140 into develop will increase coverage by 0.13%.
The diff coverage is 41.04%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2140      +/-   ##
=============================================
+ Coverage       54.2%   54.33%   +0.13%     
- Complexity      2469     2492      +23     
=============================================
  Files            447      454       +7     
  Lines          14723    14865     +142     
  Branches        1695     1749      +54     
=============================================
+ Hits            7980     8077      +97     
- Misses          5960     6018      +58     
+ Partials         783      770      -13
Impacted Files Coverage Δ Complexity Δ
.../rm/datasource/undo/mysql/MySQLUndoLogManager.java 82.35% <ø> (ø) 7 <0> (ø) ⬇️
...ava/io/seata/core/constants/ConfigurationKeys.java 0% <ø> (ø) 0 <0> (ø) ⬇️
.../seata/rm/datasource/undo/UndoExecutorFactory.java 75% <ø> (+2.77%) 3 <0> (-1) ⬇️
...a/io/seata/discovery/registry/RegistryFactory.java 0% <ø> (ø) 0 <0> (ø) ⬇️
.../seata/integration/http/HttpAutoConfiguration.java 0% <0%> (ø) 0 <0> (?)
...o/seata/rm/datasource/AbstractConnectionProxy.java 11.7% <0%> (ø) 5 <0> (ø) ⬇️
...ery/registry/consul/ConsulRegistryServiceImpl.java 1.01% <0%> (+0.02%) 1 <0> (ø) ⬇️
...integration/http/HttpHandlerExceptionResolver.java 0% <0%> (ø) 0 <0> (?)
...rc/main/java/io/seata/core/rpc/ChannelManager.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...re/src/main/java/io/seata/core/rpc/RpcContext.java 44.44% <0%> (+6.79%) 21 <0> (ø) ⬇️
... and 61 more

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@slievrly slievrly merged commit 16d7030 into apache:develop Jan 21, 2020
@slievrly slievrly added this to the 1.1.0 milestone Feb 3, 2020
booogu pushed a commit to booogu/seata that referenced this pull request Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants