5
\$\begingroup\$

The code is actually a TemplateBuilder.
We have 2 different templates, French and Dutch.
We need to change the template and then convert it to PDF so it can be stored in the DB later.

The main method is createBulletins().

What am I looking for?

  • I don't like the constantly reading the template from the byte[] for each bulletin, so if there is a possibility this could be better (with less memory consumption) I'm fully open.
  • Any other suggestions/improvements.

Framework used :

<dependency>
    <groupId>fr.opensagres.xdocreport</groupId>
    <artifactId>xdocreport</artifactId>
    <version>1.0.5</version>
</dependency>
<dependency>
    <groupId>fr.opensagres.xdocreport</groupId>
    <artifactId>fr.opensagres.xdocreport.converter.odt.odfdom</artifactId>
    <version>1.0.5</version>
</dependency>
<dependency>
    <groupId>fr.opensagres.xdocreport</groupId>
    <artifactId>org.odftoolkit.odfdom.converter.core</artifactId>
    <version>1.0.5</version>
</dependency>

Code (Java 1.6) :

public class TemplateBuilder {

    private static final Logger LOGGER = LoggerFactory.getLogger(TemplateBuilder.class);
    private static final int NATIONAL_NUMBER = 0;
    private static final int LANGUAGE = 2;
    private static final int NAME = 3;
    private static final int FIRST_NAME = 4;
    private static final int STREET = 5;
    private static final int ZIP = 6;
    private static final int CITY = 7;
    private static final int COUNTRY = 8;
    private static final int BIRTH_DATE = 9;
    private static final int NUMBER = 10;
    private static final int EMPLOYD_BY = 12;
    private static final int TYPE = 13;

    private byte[] templateFR;
    private byte[] templateNL;

    private RarExtractor extractor;
    private Report report;

    /**
     * The bulletin service.
     */
    @Inject
    @Named("bulletinWebService")
    private BulletinWebService bulletinService;

    /**
     * The country dao.
     */
    @Autowired
    private CountryDAO countryDAO;

    /**
     * The user helper.
     */
    @Autowired
    private BulletinUserHelper userHelper;

    private final String today = new SimpleDateFormat("yyyyMMdd").format(new Date());

    /**
     * Creates all the bulletins from the given templates, with data stored in the extractor.
     * 
     * @return map with bulletin info as key and byte array (file) as value.
     * @throws Exception => createPdf(Bulletin bulletin, String[] data) throws Exception so can't be more specified here.
     */
    public Map<Bulletin, byte[]> createBulletins() throws Exception {
        Map<Bulletin, byte[]> bulletins = Maps.newHashMap();
        for (Entry<String, String[]> entry : extractor.getCsv().entrySet()) {
            Bulletin bulletin = createBulletin(entry.getValue());
            bulletins.put(bulletin, createPdf(bulletin, entry.getValue()));
            this.report.addReaded(bulletins.size());
        }
        return bulletins;
    }

    /**
     * 
     * @param bulletin bulletin info what need's to be filled in
     * @param data the data what must be injected into the template.
     * @return the created bulletin.
     * @throws Exception => OdfTextDocument.loadDocument throws raw exception so can't be more specified here.
     */
    private byte[] createPdf(Bulletin bulletin, String[] data) throws Exception {
        InputStream in = null;
        OdfTextDocument document = null;
        ByteArrayOutputStream output = null;
        try {
            if ("NL".equalsIgnoreCase(data[LANGUAGE])) {
                Objects.requireNonNull(templateNL, "Nederlandstalige template is niet aanwezig.");
                Objects.requireNonNull(extractor.getBulletinTitle(), "Nederlandstalige titel is niet aanwezig.");
                in = new ByteArrayInputStream(templateNL);
                bulletin.setTitle(extractor.getBulletinTitle());
            } else if ("FR".equalsIgnoreCase(data[LANGUAGE])) {
                Objects.requireNonNull(templateFR, "Franstalige template is niet aanwezig.");
                Objects.requireNonNull(extractor.getBulletinTitleFR(), "Franstalige titel is niet aanwezig.");
                in = new ByteArrayInputStream(templateFR);
                bulletin.setTitle(extractor.getBulletinTitleFR());
            } else {
                throw new IOException("Kan niet vinden of het NL of FR moet zijn. Is deze op de 3de entry in de csv?");
            }
            document = OdfTextDocument.loadDocument(in);

            replace("%name%", data[NAME], document);
            replace("%firstName%", data[FIRST_NAME], document);
            replace("%street%", data[STREET], document);
            replace("%city%", data[ZIP] + " " + data[CITY], document);
            replace("%birthDate%", data[BIRTH_DATE], document);
            replace("%formNr%", data[NUMBER], document);
            replace("%sendDate%", "//TODO : TO BE CONFIRMED", document);
            replace("%employedBy%", data[EMPLOYD_BY], document);

            PdfOptions options = PdfOptions.create();
            output = new ByteArrayOutputStream();
            PdfConverter.getInstance().convert(document, output, options);

            output.flush();
            return output.toByteArray();
        } finally {
            close(in);
            close(document);
            close(output);

        }
    }

    private Bulletin createBulletin(String[] data) throws IOException {
        Bulletin bulletin = new Bulletin();
        bulletin.setZipFileName(FilenameUtils.getBaseName(extractor.getFileName()));
        bulletin.setCreationDate(new Date());
        bulletin.setPdfFileName("N" + data[NATIONAL_NUMBER] + data[TYPE] + today + "0001");
        final Properties tokenize = parseProperties(bulletin);
        bulletin.setBulletinUser(getBulletinUser(bulletin));
        bulletin.setBulletinType(this.bulletinService
                .findBulletinType(tokenize.getProperty(BulletinTokens.DOC_TYPE)));
        updateWitchCsvInformation(data, bulletin);
        return bulletin;
    }

    /**
     * Update witch csv information.
     *
     * @param bulletin the bulletin
     * @throws IOException Signals that an I/O exception has occurred.
     */
    private void updateWitchCsvInformation(String[] line, Bulletin bulletin) {
        bulletin.setCountry(this.countryDAO.findOne(line[COUNTRY]));
        bulletin.setPostCode(line[ZIP]);
        bulletin.setCreationDate(new Date());
        if (bulletin.getCountry() == null) {
            report.getFilesWithoutCsvEntry().add(bulletin.getPdfFileName() + " missing country code");
            LOGGER.debug(bulletin.getZipFileName() + " for " + bulletin.getPdfFileName() + " missing country code");
        }

    }

    public void clear() {
        templateFR = null;
        templateNL = null;
        extractor = null;
        report = null;
    }

    /**
     * Search a searchfield and replace that field to the data in the document.
     * 
     * @param searchField String to search for in Document
     * @param data to be set in place of the searchfield.
     * @param document what need's a replace from searchfield to the data.
     * @throws InvalidNavigationException 
     */
    private void replace(String searchField, String data, OdfTextDocument document) throws InvalidNavigationException {
        TextNavigation search = new TextNavigation(searchField, document);
        while (search.hasNext()) {
            TextSelection item = (TextSelection) search.getCurrentItem();
            item.replaceWith(data);
        }
    }

    /**
     * safely try to close the closeable 
     * 
     * @param closeable to close.
     */
    private void close(Closeable closeable) {
        if (closeable!=null) {
            try {
                closeable.close();
            } catch (IOException ex) {
                LOGGER.error("closeable not able to close, classname : " + closeable.getClass().getSimpleName(), ex);
            }
        }
    }

    /**
     * Gets the bulletin user.
     *
     * @param tokenize the tokenize
     * @return the bulletin user
     */
    private BulletinUser getBulletinUser(final Bulletin bulletin) {
        final BulletinUser x = this.userHelper.findBulletinUser(bulletin);
        if (x == null) {
            report.addNotLinked(bulletin.getPdfFileName());
        } else {
            report.addLinked(bulletin.getPdfFileName());
        }
        return x;
    }

    /**
     * Parses the properties.
     *
     * @param bulletin the bulletin
     * @return the properties
     * @throws IOException Signals that an I/O exception has occurred.
     */
    private Properties parseProperties(Bulletin bulletin) throws IOException {
        try {
            return BulletinTokenizer.tokenize(bulletin
                    .getPdfFileName());
        } catch (ParseException e) {
            throw new IOException(e);
        }
    }

    public byte[] getTemplateFR() {
        return templateFR;
    }

    public void setTemplateFR(ISimpleInArchiveItem templateFR) throws IOException {
        this.templateFR = RarUtil.extract(templateFR);
    }

    public byte[] getTemplateNL() {
        return templateNL;
    }

    public void setTemplateNL(ISimpleInArchiveItem templateNL) throws IOException {
        this.templateNL = RarUtil.extract(templateNL);
    }

    public void setReport(Report report) {
        this.report = report;
    }

    public RarExtractor getExtractor() {
        return extractor;
    }

    public void setExtractor(RarExtractor extractor) {
        this.extractor = extractor;
    }
}
\$\endgroup\$
2
\$\begingroup\$

Logging multiple languages

While you do make usage of logging, you write them in sometimes in Dutch and sometimes in English.
Please try to hold to 1 language, preferable English for the logging.

Static or not?

private final String today = new SimpleDateFormat("yyyyMMdd").format(new Date());

It's clear that this String need's to hold todays day formatted.
The thing what you could improve is making the SimpleDateFormat static.
Like this you don't have an instantiation each time you create an instantiation of the TemplateBuilder

private static final SimpleDateFormat DATE_FORMATTER = new SimpleDateFormat("yyyyMMdd");
private final String today = DATE_FORMATTER.format(new Date());

Known memory leak

Your code has a memory leak, not by you but by Apache.
There is a tracker for it but it isn't resolved because Apache would not see it as a leak.
I came upon this because I'm the OP and the server died after approximate 250 pdf's done.

I rewrote the replace function to a simpler search, this means no looking in header and footer, and just returns the first Node what match the text to search :

    private void replace(String searchField, String data, OdfTextDocument document) throws Exception {
    Node node = findNode(searchField, document.getContentRoot().getChildNodes());
    if (node != null) {
        node.setTextContent(data);
        LOGGER.info("found node : " + searchField);
    } else {
        LOGGER.warn("didn't found node : " + searchField);
    }
}

private Node findNode(String searchField, NodeList nodeList) {
    int length = nodeList.getLength();
    for (int counter = 0; counter < length; counter++) {
        Node node = nodeList.item(counter);
        if (node.getNodeType() == Node.TEXT_NODE && searchField.equals(node.getTextContent())) {
            return node;
        } else {
            Node child = findNode(searchField, node.getChildNodes());
            if (child != null) {
                return child;
            }
        }
    }
    return null;
}

Memory building up

The method public Map<Bulletin, byte[]> createBulletins() design isn't good

Why do you need to hold all those and return it later for saving?
What if your getCsv().getEntrySet() holds more then 30k entries?
Note : This is real the real situation

It's stacking up memory waiting to go out of memory.
Change the method to void or int if you want to return the size for a report and save every x entries. (x could also be 1). You have the BulletinWebService in the class who provides the save so use it.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.