0
\$\begingroup\$

Recently I refactored an application to start using spring, and after some research, I found out that @Component is not thread safe. The class is this: (It's a converter from xlsx to json string)

@Component
public class ExcelFileConverter {

    private XSSFWorkbook loadedWorkbook;

    private FileInputStream fileStream;

    private String filePassword;

    private InformationGroupDAO informationGroupDAO;

    private List<WorkSheet> dbWorksheetList = new ArrayList<>();

    private WorkSheetExistsValidation workSheetExistsValidation;

    private WorkSheetOrderValidation workSheetOrderValidation;

    private WorkSheetValuesValidation workSheetValuesValidation;

    public ExcelFile() {

    }

    @Autowired
    public ExcelFile(WorkSheetExistsValidation workSheetExistsValidation,
            WorkSheetOrderValidation workSheetOrderValidation, WorkSheetValuesValidation workSheetValuesValidation, InformationGroupDAO informationGroupDAO) {
        this.workSheetExistsValidation = workSheetExistsValidation;
        this.workSheetOrderValidation = workSheetOrderValidation;
        this.workSheetValuesValidation = workSheetValuesValidation;
        this.informationGroupDAO = informationGroupDAO;
    }

    public void loadProtectedFile(final List<ValidationError> errors) throws WorkbookLoadException {

        try {
            POIFSFileSystem fs = new POIFSFileSystem(this.fileStream);
            EncryptionInfo info = new EncryptionInfo(fs);
            Decryptor d = Decryptor.getInstance(info);

            if (d.verifyPassword(this.filePassword)) {
                this.loadedWorkbook = new XSSFWorkbook(d.getDataStream(fs));
            } else {
                ValidationError error = new ValidationError();
                error.setId(1);
                error.setMessage(new WorkbookPasswordError().getMessage());
                errors.add(error);
            }
        } catch (OfficeXmlFileException oxfe) {
            ValidationError error = new ValidationError();
            error.setId(1);
            error.setMessage(new WorkbookNotProtectedError().getMessage());
            errors.add(error);
        } catch (IOException ioe) {
            throw new WorkbookLoadException();
        } catch (GeneralSecurityException gse) {
            throw new WorkbookLoadException();
        } catch (Exception e) {
            throw e; // new WorkbookLoadException();
        }
    }

    @SuppressWarnings("unused")
    private void loadUnprotectedFile(final List<ValidationError> errors) throws WorkbookLoadException {
        try {
            this.loadedWorkbook = new XSSFWorkbook(this.fileStream);
        } catch (IOException ioe) {
            throw new WorkbookLoadException();
        } catch (Exception e) {
            throw new WorkbookLoadException();
        }
    }

    public List<XSSFSheet> getWorksheets() {
        try {
            List<XSSFSheet> result = new ArrayList<XSSFSheet>();

            for (int i = 0; i < this.loadedWorkbook.getNumberOfSheets(); i++)
                result.add(this.loadedWorkbook.getSheetAt(i));

            return result;
        } catch (Exception e) {
            throw e;
        }
    }

    public XSSFCell getWorksheetCell(XSSFSheet sheet, String address) {
        try {
            CellReference cellReference = new CellReference(address);
            XSSFRow row = sheet.getRow(cellReference.getRow());
            XSSFCell cell = row.getCell(cellReference.getCol());

            return cell;

        } catch (Exception e) {
            throw e;
        }
    }

    public List<ValidationError> validateIfWorkSheetExists() {
        return workSheetExistsValidation.validateIfWorkSheetExists(dbWorksheetList, loadedWorkbook);
    }

    public List<ValidationError> validateWorkSheetOrder() {
        return workSheetOrderValidation.validateWorkSheetOrder(dbWorksheetList, loadedWorkbook);
    }

    public List<ValidationError> validateWorkSheetValues() {
        return workSheetValuesValidation.validateWorkSheetValues(loadedWorkbook);
    }

    public String convetFileToJSON() {

        JSONObject result = new JSONObject();

        Map<String, Object> arrGroups = new LinkedHashMap<>();

        String guid = java.util.UUID.randomUUID().toString();

        try {

            // Get information groups
            List<InformationGroup> worksheetGroups = informationGroupDAO.findByParent(null);

            for (InformationGroup group : worksheetGroups) {
                arrGroups.put(group.getAlias(), getGroupJSON(group));
            }
            JSONObject groupObj = new JSONObject(arrGroups);

            result.put("guid", guid);
            result.put("dfe", groupObj);

        } catch (Exception e) {
            throw e;
        }

        return result.toString();
    }

    private JSONObject getGroupJSON(InformationGroup group) {

        Map<String, Object> gCells = new LinkedHashMap<>();

        // Get the group cells
        List<GroupCell> groupCells = group.getGroupCells();

        for (GroupCell cell : groupCells) {
            // Get the file cell value
            XSSFCell fileCell = this.getWorksheetCell(this.loadedWorkbook.getSheetAt(cell.getWorksheet().getIndex()),
                    cell.getAddress());

            switch (DataType.valueOf(cell.getDataType().toUpperCase())) {

            case STRING:
                gCells.put(cell.getAlias(), fileCell.getStringCellValue());
                break;

            case INTEGER:
                double cellValue = fileCell.getNumericCellValue();
                Integer i_cellValue = (int) cellValue;
                gCells.put(cell.getAlias(), i_cellValue);
                break;

            case DECIMAL:
                double d_cellValue = fileCell.getNumericCellValue();
                gCells.put(cell.getAlias(), d_cellValue);
                break;

            case DATE:
                Date cellDate = fileCell.getDateCellValue();
                SimpleDateFormat format = new SimpleDateFormat("DD/MM/yyyy");
                String dateString = format.format(cellDate);

                gCells.put(cell.getAlias(), dateString);
                break;
            }
        }

        if (group.getSubgroups().size() > 0) {
            for (InformationGroup subGroup : group.getSubgroups()) {
                gCells.put(subGroup.getAlias(), getGroupJSON(subGroup));
            }
        }

        JSONObject groupValues = new JSONObject(gCells);
        return groupValues;
    }

    public FileInputStream getFileStream() {
        return fileStream;
    }

    public void setFileStream(FileInputStream fileStream) {
        this.fileStream = fileStream;
    }

    public String getFilePassword() {
        return filePassword;
    }

    public void setFilePassword(String filePassword) {
        this.filePassword = filePassword;
    }

I wonder for if it's ok to have the fileStream and the arraylist as a field, and how to deal with this component not being thread safe. Also, any feedbacks regarding anything else will be appreciated.

Thanks in advance!

EDIT1: I think I understood the main point. I refactored again, removing the fields and instead, using them as method variables:

@Service
public class ExcelToJsonService {

    @Autowired
    private WorkSheetExistsValidation workSheetExistsValidation;

    @Autowired
    private WorkSheetOrderValidation workSheetOrderValidation;

    @Autowired
    private WorkSheetValuesValidation workSheetValuesValidation;

    @Autowired
    private InformationGroupDAO informationGroupDAO;

    public String convertExcelToJson(File file, String password) {
        List<ValidationError> errors = new ArrayList<>();
        try {
            // Load the file
            XSSFWorkbook workbook = loadProtectedFile(file, password);

            // Validations
            errors.addAll(workSheetExistsValidation.validateIfWorkSheetExists(workbook));
            errors.addAll(workSheetOrderValidation.validateWorkSheetOrder(workbook));
            errors.addAll(workSheetValuesValidation.validateWorkSheetValues(workbook));
            if (!errors.isEmpty()) {
                return errorsAsJson(errors);
            }

            return convetFileToJSON(workbook);
        } catch (WorkbookInvalidPasswordException e) {
            errors.add(new ValidationError(ErrorMessagesConfig.WORKBOOK_PASS_ERROR));
        } catch (WorkbookNoPasswordException e) {
            errors.add(new ValidationError(ErrorMessagesConfig.WORKBOOK_NOT_PROTECTED_ERROR));
        } catch (Throwable e) {
            errors.add(
                    new ValidationError(MessageFormat.format(ErrorMessagesConfig.WORKBOOK_LOAD_ERROR, e.getMessage())));
        }
        return errorsAsJson(errors);
    }

What you guys think?

\$\endgroup\$

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Browse other questions tagged or ask your own question.