Extract zip and re-zip with password in java

Question

I am trying to extract list of zip files from folder and then re-zipping them with password. The problem is while re-zipping, the iteration/loop is not stopping. Also, re-zipped files should be a separate zip file each rather than merging all contents to one zip.

Here's what I have tried:

import java.io.File;
import java.util.ArrayList;
import java.util.List;
import net.lingala.zip4j.core.ZipFile;
import net.lingala.zip4j.exception.ZipException;
import net.lingala.zip4j.model.ZipParameters;
import net.lingala.zip4j.util.Zip4jConstants;

public class AddFilesWithAESEncryption2 {

    public AddFilesWithAESEncryption2() {

        try {
            //Extract Zip files as folders
            try {
                String ZipSourcePath = "E:/EZipTest/";
                String ExtractTo = "D:/DZipTest/";
                String files1;
                File folder1 = new File(ZipSourcePath);
                File[] listOfFiles1 = folder1.listFiles();

                for (int i = 0; i < listOfFiles1.length; i++) {
                    if (listOfFiles1[i].isFile()) {
                        files1 = listOfFiles1[i].getName();
                        String ZipFiles = "E:/EZipTest/" + files1;

                        try {
                            ZipFile zipFile = new ZipFile(ZipFiles);
                            List fileHeaderList = zipFile.getFileHeaders();
                            zipFile.extractAll(ExtractTo);
                        } catch (ZipException e) {
                            e.printStackTrace();
                        }
                    }
                }
                //Get list of folders    
                String DirectoryNames;
                String ExtractedDirectories1 = "D:/DZipTest/";
                File folder2 = new File(ExtractedDirectories1);
                File[] listOfFiles2 = folder2.listFiles();

                for (int i = 0; i < listOfFiles2.length; i++) {
                    if (listOfFiles2[i].isDirectory()) {
                        DirectoryNames = listOfFiles2[i].getName();
                        String ListOfDirectories = "D:/DZipTest/" + DirectoryNames;

                        //Get list of files
                        String ExtractedDirectories = ListOfDirectories;
                        File folder3 = new File(ExtractedDirectories);
                        File[] listOfFiles3 = folder3.listFiles();

                        for (int j = 0; j < listOfFiles3.length; j++) {
                            File file = listOfFiles3[j];
                            if (file.isFile()) {
                                String FileNames = file.getName();
                                System.out.println(ListOfDirectories + FileNames);

                                //Compress and zip the files
                                ZipFile zipFile = new ZipFile("D:/" + listOfFiles2[i].getName() + ".zip");
                                ArrayList filesToAdd = new ArrayList();
                                filesToAdd.add(new File(ListOfDirectories + FileNames));
                                ZipParameters parameters = new ZipParameters();
                                parameters.setCompressionMethod(Zip4jConstants.COMP_DEFLATE); // set compression method to deflate compression
                                parameters.setCompressionLevel(Zip4jConstants.DEFLATE_LEVEL_NORMAL);
                                parameters.setEncryptFiles(true);
                                parameters.setEncryptionMethod(Zip4jConstants.ENC_METHOD_AES);
                                parameters.setAesKeyStrength(Zip4jConstants.AES_STRENGTH_256);
                                parameters.setPassword("test");
                                zipFile.addFiles(filesToAdd, parameters);
                            }
                        }
                    }
                }
            } catch (ZipException e) {
                e.printStackTrace();
            }

        } catch (Exception e) {
            e.printStackTrace();
        }
    }

    public static void main(String[] args) {
        new AddFilesWithAESEncryption2();
    }
}

Answer 1

Refactoring

Refactoring your code will help you understand what it does. It will reveal the problem and immediately identify a fix. Here's how it goes. Note that this is not a complete tutorial, but I hope you get the point.

First, extract a nice method that does the unzipping. Mark everything inside the first for loop, then right click and choose Refactor / Extract Method.... Name it unzipFile. Note that you now have a nice small, potentially reusable and potentially testable (JUnit) method.

Next, mark everything from ZipParameters parameters to parameters.setPassword("test"); Right click, Refactor / Extract Method.... Name it getEncryptionParameters. Note how 7 lines of code have been removed from the long method and readability is improved.

Right click on parameters and choose Refactor / Inline .... Note how the temporary variable disappears.

See the bug

If you have followed closely, there is a piece of code like this:

//Compress and zip the files
ZipFile zipFile = new ZipFile("D:/" + listOfFiles2[i].getName() + ".zip");
ArrayList filesToAdd = new ArrayList();
filesToAdd.add(new File(ListOfDirectories + FileNames));
zipFile.addFiles(filesToAdd, getEncryptionParameters());

See what it does? It creates a new ZIP file, adds only one file to filesToAdd and that's it. But why? It says FileNames. How can that be one file only?

Looking at

String FileNames = file.getName();

that's really just one file, so the variable name is wrong.

Right click FileNames and choose Refactor/Rename.... Enter fileName. Note how the variable name in your program matches to what it really is. It heavily improves readability of the code.

Simplify

Now that you know you're adding only one file, use addFile() instead of addFiles(). You're getting rid of the ArrayList:

//Compress and zip the files
ZipFile zipFile = new ZipFile("D:/" + listOfFiles2[i].getName() + ".zip");
File fileToAdd = new File(ListOfDirectories + fileName);
zipFile.addFile(fileToAdd, getEncryptionParameters());

Fix the bug

As spotted before, a new ZipFile(...) is created inside the loop and only one file is added to it. Move that line out of the loop pressing Alt+Up.

Continue refactoring

A part of the problem is already fixed (I haven't tried, actually), but your code is still not error-free. Let's go on:

Mark everything from File[] listOfFiles3 to the end of the for loop that follows. Right click, Refactor/Extract Method..., name it rezip. Your big method becomes smaller again.

Right click on ExtractedDirectories, Refactor / Inline .... You just got rid of a unnecessary temporary variable.

See something? Your code should look like this:

//Get list of files
File folder3 = new File(ListOfDirectories);
rezip(listOfFiles2, i, ListOfDirectories, folder3);

Note how folder3 and ListOfDirectories is essentially the same. Let's get rid of it. Move the line File folder3 = new File(ListOfDirectories); into the method, just behind private void rezip(...){ and remove the parameter File folder3 from both, the method call and the method declaration of rezip().

The loop using rezip() now looks like this:

for (int i = 0; i < listOfFiles2.length; i++) {
    if (listOfFiles2[i].isDirectory()) {
        DirectoryNames = listOfFiles2[i].getName();
        String ListOfDirectories = "D:/DZipTest/" + DirectoryNames;
        rezip(listOfFiles2, i, ListOfDirectories);
    }
}

You might spot that DirectoryNames is actually just one, not many. Right click, Refactor/Rename.... Enter subDirectory.

Right click subDirectory, Refactor / Inline .... Read the error message. Right click References / Workspace. Check the results and find out that this variable is only used within the for loop. Delete the declaration outside and declare it at its first use. Now do the Refactor / Inline ... operation.

Your code looks like this:

for (int i = 0; i < listOfFiles2.length; i++) {
    if (listOfFiles2[i].isDirectory()) {
        String ListOfDirectories = "D:/DZipTest/" + listOfFiles2[i].getName();
        rezip(listOfFiles2, i, ListOfDirectories);
    }
}

Again, there's a variable name indicating a list or an Array, but that's not true. Refactor / Rename..., name it directoryToZip.

Inline the following variables in this order: ExtractedDirectories1, folder2, ZipSourcePath, folder1.

Rename in this order listOfFiles1 to zipFiles and listOfFiles2 to extractedDirectories.

Remove files1 since it is never used.

The final bug

The method is now short and readable enough to understand it completely. Does the following make sense?

String ExtractTo = "D:/DZipTest/";
File[] zipFiles = new File("E:/EZipTest/").listFiles();
for (int i = 0; i < zipFiles.length; i++) {
    unzipFile(ExtractTo, zipFiles, i);
}

File[] extractedDirectories = new File("D:/DZipTest/").listFiles();
for (int i = 0; i < extractedDirectories.length; i++) {
    if (extractedDirectories[i].isDirectory()) {
        String directoryToZip = "D:/DZipTest/" + extractedDirectories[i].getName();
        rezip(extractedDirectories, i, directoryToZip);
    }
}

No it doesn't.

1. You don't want to extract all archives first but one by one
2. You don't want to zip subdirectories, you want to zip everything in the ExtractTo directory

Fixing the final bug

The signature of unzipFile() does not look right. If it unzips one file only as the name suggests, why does it get access to all files then?

Replace unzipFile(ExtractTo, zipFiles, i); by unzipFile(ExtractTo, zipFiles[i]);. This breaks the code. Eclipse will mark it red. Fix it by changing the parameters from

private void unzipFile(String ExtractTo, File[] listOfFiles1, int i)

to

private void unzipFile(String ExtractTo, File listOfFiles1)

Inside unzip, replace listOfFiles1[i] by listOfFiles1. Then Refactor/Rename... it to sourceZipFile.

Similar for the rezip method: it should get the directory to zip and the target file name only. Therefore change

rezip(extractedDirectories, i, directoryToZip);

to

rezip(extractedDirectories[i], directoryToZip);

Then adapt the method itself from

private void rezip(File[] listOfFiles2, int i, String ListOfDirectories) throws ZipException

to

private void rezip(File listOfFiles2, String ListOfDirectories) throws ZipException

then change listOfFiles2[i] to listOfFiles2. Rename it to targetFile.

Now you have a nice unzipFile() method and a rezip() method. Let's combine it in a cool way:

String ExtractTo = "D:/DZipTest/";
File[] zipFiles = new File("E:/EZipTest/").listFiles();
for (int i = 0; i < zipFiles.length; i++) {
    unzipFile(ExtractTo, zipFiles[i]);
    rezip(zipFiles[i], ExtractTo);
    // TODO: delete extracted files here
}

Awesome, ain't it?

Notes

Maybe you've seen how much effort it is to understand your code and provide a fix. Actually, too much effort for Stack Overflow. Next time you ask a question, please try to provide code that is at minumum as readable as your code now.

The code is still not as clean as it should be. Spend some more time on it. When you think it's superb, post it on https://codereview.stackexchange.com/ to get even more instructions.