Search code examples
javasocketsfile-transferfileoutputstream

FileOutputStream in parts does not work


I'm trying to work on a file transfer feature for an application of mine. My implementation of the file transfer is to send a file in parts in the form of objects which contain info about the file and also the bytes sent. However, I've noticed that I can only actually write to the file if I save all the received bytes in a list then write it to the file at once. If I try writing to the file in parts, it ends up with an empty file, as if the file hadn't been written to at all.

Here is my method which reads the original file and then sends it in parts:

public void sendFile(File src) {
    try {

        BufferedInputStream is = new BufferedInputStream(new FileInputStream(src));
        Message msg = new Message(MType.FILE_OPEN, true);
        com.transmit(msg);
        byte[] buf = new byte[Utility.bufferSize];
        msg = new Message(MType.FILE_NAME, src.getName());
        msg.setValue(MType.FILE_SIZE, Files.size(src.toPath()));
        com.transmit(msg);
        for (int count = is.read(buf); count > 0; count = is.read(buf)) {

            msg = new Message(MType.FILE_NAME, src.getName());
            msg.setValue(MType.FILE_SIZE, Files.size(src.toPath()));
            msg.setValue(MType.FILE_BYTE, buf);
            msg.setValue(MType.FILE_COUNT, count);
            com.transmit(msg);

        }
        msg = new Message(MType.FILE_NAME, src.getName());
        msg.setValue(MType.FILE_SIZE, Files.size(src.toPath()));
        msg.setValue(MType.FILE_CLOSE, true);
        is.close();
        com.transmit(msg);

    } catch (IOException e) {
        sender.getChatRoomController().error(ProgramError.ATTACH_FILE);
        Utility.log(e);
        e.printStackTrace();
    }
}

Here is my method which receives the Message objects on the other end:

public void readFile(Message msg) {

    if (msg.hasID(MType.FILE_NAME)) {
        String name = msg.getValue(MType.FILE_NAME).toString();
        long size = (long) msg.getValue(MType.FILE_SIZE);
        File file = new File(Directories.fDir.getDirectory(), name);
        TempFile tf = new TempFile(file);
        if (!map.containsKey(file)) {
            if (!file.exists()) {
                try {
                    file.createNewFile();
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }
            map.put(file, tf);
        } else {
            tf = map.get(file);
        }
        if (msg.hasValue(MType.FILE_BYTE)) {
            byte[] buf = (byte[]) msg.getValue(MType.FILE_BYTE);
            int count = (int) msg.getValue(MType.FILE_COUNT);
            tf.addEntry(buf, count);
        }
        if (msg.hasValue(MType.FILE_CLOSE)) {
            tf.writeFile(true);
            map.remove(file);

            if (sender instanceof Server) {
                Server server = (Server) sender;
                msg = new Message(MType.FILE_NAME, name);
                msg.setValue(MType.FILE_SIZE, size);
                msg.setValue(MType.FILE_ATTACHMENT, server.getFileID());
                addFile(file, server);
                server.broadcast(msg);
            }

        }
    }
}

And here is my TempFile class:

    public class TempFile {

    private ArrayList<Byte[]> data;
    private ArrayList<Integer> counts;
    private File file;

    public TempFile(File file) {
        data = new ArrayList<>();
        counts = new ArrayList<>();
        this.file = file;
    }

    public void addEntry(byte[] data, int count) {

        this.data.add(Utility.toWrapper(data));
        this.counts.add(count);

    }

    public void writeFile(boolean append) {
        try {
            BufferedOutputStream os = new BufferedOutputStream(new FileOutputStream(file));
            for (int i = 0; i < data.size(); i++) {
                byte[] chunk = Utility.toPrimitive(data.get(i));
                int count = counts.get(i);
                os.write(chunk, 0, count);
            }
            os.flush();
            os.close();
        } catch (FileNotFoundException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

}

And here is my other implementation which involves actual temp files:

    public class TempFile2 {

    private File file;
    private File tempFile;
    private FileOutputStream os;

    public TempFile2(File file) {
        this.file = file;
        this.tempFile = new File(file.getParent(), FilenameUtils.getBaseName(file.getName()) + ".tmp");
        if (!tempFile.exists()) {
            try {
                tempFile.createNewFile();
            } catch (IOException e) {
                e.printStackTrace();
            }
        }
        try {
            os = new FileOutputStream(tempFile);
        } catch (FileNotFoundException e) {
            e.printStackTrace();
        }
    }

    public void addEntry(byte[] data, int count) {

        try {
            os.write(data, 0, count);
        } catch (IOException e) {
            e.printStackTrace();
        }

    }

    public void writeFile() {

        try {
            os.close();
            Files.copy(tempFile.toPath(), file.toPath(), StandardCopyOption.REPLACE_EXISTING);
        } catch (IOException e) {
            e.printStackTrace();
        }

    }

}

Solution

  • To make this work, you need to flush your output stream as you go.

        public class TempFile2 {
    
        private File file;
        private File tempFile;
        private FileOutputStream os;
    
        public TempFile2(File file) {
            this.file = file;
            this.tempFile = new File(file.getParent(), FilenameUtils.getBaseName(file.getName()) + ".tmp");
            if (!tempFile.exists()) {
                try {
                    tempFile.createNewFile();
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }
            try {
                os = new FileOutputStream(tempFile);
            } catch (FileNotFoundException e) {
                e.printStackTrace();
            }
        }
    
        public void addEntry(byte[] data, int count) {
    
            try {
                os.write(data, 0, count);
                os.flush();
            } catch (IOException e) {
                e.printStackTrace();
            }
    
        }
    
        public void writeFile() {
    
            try {
                os.close();
                Files.copy(tempFile.toPath(), file.toPath(), StandardCopyOption.REPLACE_EXISTING);
            } catch (IOException e) {
                e.printStackTrace();
            }
    
        }
    
    }
    

    In fact, instead of keeping the output stream open, you could open and close it each time you receive a data chunk:

        public class TempFile2 {
    
        private File file;
        private File tempFile;
    
        public TempFile2(File file) {
            this.file = file;
            this.tempFile = new File(file.getParent(), FilenameUtils.getBaseName(file.getName()) + ".tmp");
            if (!tempFile.exists()) {
                try {
                    tempFile.createNewFile();
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }
    
        }
    
        public void addEntry(byte[] data, int count) {
            try(OutputStream os = new FileOutputStream(tempFile, true)) {
                os.write(data, 0, count);
                os.flush();
            } catch (IOException e) {
                e.printStackTrace();
            }
    
        }
    
        public void writeFile() {
    
            try {
                Files.copy(tempFile.toPath(), file.toPath(), StandardCopyOption.REPLACE_EXISTING);
            } catch (IOException e) {
                e.printStackTrace();
            }
    
        }
    
    }
    

    Also, I have a suggestion.

    The read() operation might only read a few bytes, but your current implementation will send the entire array over anyway. One solution to this would be to make a new smaller array to hold the data:

    byte[] bytesToSend = new byte[count];
    System.arraycopy(buf, 0, bytesToSend, 0, count);
    

    I think an even better solution would be to make use of the Base64 class here and send a serialized version of the byte array instead of the byte array itself.

    // In sender
    byte[] bytesToSend = new byte[count];
    System.arraycopy(buf, 0, bytesToSend, 0, count);
    String encodedBytes = Base64.getEncoder().encodeToString(bytesToSend);
    msg.setValue(MType.FILE_BYTE, encodedBytes);
    
    // In receiver
    String encodedBytes = (String) msg.getValue(MType.FILE_BYTE);
    buf = Base64.getDecoder().decode(encodedBytes);