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 agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.