diff --git a/drools-wb-screens/drools-wb-dtable-xls-editor/drools-wb-dtable-xls-editor-backend/src/main/java/org/drools/workbench/screens/dtablexls/backend/server/DecisionTableXLSServiceImpl.java b/drools-wb-screens/drools-wb-dtable-xls-editor/drools-wb-dtable-xls-editor-backend/src/main/java/org/drools/workbench/screens/dtablexls/backend/server/DecisionTableXLSServiceImpl.java index 4be53237e25..a037973fb9d 100644 --- a/drools-wb-screens/drools-wb-dtable-xls-editor/drools-wb-dtable-xls-editor-backend/src/main/java/org/drools/workbench/screens/dtablexls/backend/server/DecisionTableXLSServiceImpl.java +++ b/drools-wb-screens/drools-wb-dtable-xls-editor/drools-wb-dtable-xls-editor-backend/src/main/java/org/drools/workbench/screens/dtablexls/backend/server/DecisionTableXLSServiceImpl.java @@ -71,7 +71,7 @@ public class DecisionTableXLSServiceImpl implements DecisionTableXLSService, ExtendedDecisionTableXLSService { - private static final Logger log = LoggerFactory.getLogger( DecisionTableXLSServiceImpl.class ); + private static final Logger log = LoggerFactory.getLogger(DecisionTableXLSServiceImpl.class); private IOService ioService; private CopyService copyService; @@ -87,15 +87,15 @@ public DecisionTableXLSServiceImpl() { } @Inject - public DecisionTableXLSServiceImpl( @Named("ioStrategy") final IOService ioService, - final CopyService copyService, - final DeleteService deleteService, - final RenameService renameService, - final Event resourceOpenedEvent, - final DecisionTableXLSConversionService conversionService, - final GenericValidator genericValidator, - final CommentedOptionFactory commentedOptionFactory, - final AuthenticationService authenticationService ) { + public DecisionTableXLSServiceImpl(@Named("ioStrategy") final IOService ioService, + final CopyService copyService, + final DeleteService deleteService, + final RenameService renameService, + final Event resourceOpenedEvent, + final DecisionTableXLSConversionService conversionService, + final GenericValidator genericValidator, + final CommentedOptionFactory commentedOptionFactory, + final AuthenticationService authenticationService) { this.ioService = ioService; this.copyService = copyService; this.deleteService = deleteService; @@ -108,256 +108,245 @@ public DecisionTableXLSServiceImpl( @Named("ioStrategy") final IOService ioServi } @Override - public DecisionTableXLSContent loadContent( final Path path ) { - return super.loadContent( path ); + public DecisionTableXLSContent loadContent(final Path path) { + return super.loadContent(path); } @Override - protected DecisionTableXLSContent constructContent( Path path, - Overview overview ) { + protected DecisionTableXLSContent constructContent(Path path, + Overview overview) { final DecisionTableXLSContent content = new DecisionTableXLSContent(); - content.setOverview( overview ); + content.setOverview(overview); return content; } @Override - public InputStream load( final Path path, - final String sessionId ) { + public InputStream load(final Path path, + final String sessionId) { try { - final InputStream inputStream = ioService.newInputStream( Paths.convert( path ), - StandardOpenOption.READ ); + final InputStream inputStream = ioService.newInputStream(Paths.convert(path), + StandardOpenOption.READ); //Signal opening to interested parties - resourceOpenedEvent.fire( new ResourceOpenedEvent( path, - getSessionInfo( sessionId ) ) ); + resourceOpenedEvent.fire(new ResourceOpenedEvent(path, + getSessionInfo(sessionId))); return inputStream; - } catch ( Exception e ) { - throw ExceptionUtilities.handleException( e ); + } catch (Exception e) { + throw ExceptionUtilities.handleException(e); } } @Override - public Path create( final Path resource, - final InputStream content, - final String sessionId, - final String comment ) { - return writeToFile( resource, - content, - sessionId, - comment, - true ); + public Path create(final Path resource, + final InputStream content, + final String sessionId, + final String comment) { + return writeToFile(resource, + content, + sessionId, + comment, + true); } @Override - public Path save( final Path resource, - final InputStream content, - final String sessionId, - final String comment ) { - return writeToFile( resource, - content, - sessionId, - comment, - false ); + public Path save(final Path resource, + final InputStream content, + final String sessionId, + final String comment) { + return writeToFile(resource, + content, + sessionId, + comment, + false); } - private Path writeToFile( final Path resource, - final InputStream content, - final String sessionId, - final String comment, - boolean create ) { - final SessionInfo sessionInfo = getSessionInfo( sessionId ); + private Path writeToFile(final Path resource, + final InputStream content, + final String sessionId, + final String comment, + boolean create) { + final SessionInfo sessionInfo = getSessionInfo(sessionId); String userAction = "UPDATING"; - if ( create ) { + if (create) { userAction = "CREATING"; } - log.info( "USER:" + sessionInfo.getIdentity().getIdentifier() + " " + userAction + " asset [" + resource.getFileName() + "]" ); + log.info("USER:" + sessionInfo.getIdentity().getIdentifier() + " " + userAction + " asset [" + resource.getFileName() + "]"); - FileInputStream tempFIS = null; - FileOutputStream tempFOS = null; - OutputStream outputStream = null; try { - File tempFile = File.createTempFile( "testxls", null ); - tempFOS = new FileOutputStream( tempFile ); - IOUtils.copy( content, tempFOS ); - tempFOS.flush(); - //Validate the xls - validate( tempFile ); + final File tempFile = File.createTempFile("testxls", null); + try (FileOutputStream tempFOS = new FileOutputStream(tempFile)) { + + IOUtils.copy(content, tempFOS); + tempFOS.flush(); + } - final org.uberfire.java.nio.file.Path nioPath = Paths.convert( resource ); - if ( create ) { - ioService.createFile( nioPath ); + //Validate the xls + validate(tempFile); + + final org.uberfire.java.nio.file.Path nioPath = Paths.convert(resource); + + ioService.startBatch(nioPath.getFileSystem()); + try (FileInputStream tempFIS = new FileInputStream(tempFile)) { + + if (create) { + + ioService.write(nioPath, + IOUtils.toByteArray(tempFIS), + commentedOptionFactory.makeCommentedOption(comment, + sessionInfo.getIdentity(), + sessionInfo)); + } else { + + try (OutputStream outputStream = ioService.newOutputStream(nioPath, + commentedOptionFactory.makeCommentedOption(comment, + sessionInfo.getIdentity(), + sessionInfo))) { + + //InputStream 'content' has been fully read to write to the temp file; so we need to use a new InputStream + //An alternative is to check for content.markSupported() and reset() however since we have a temporary + //file we may as well use it! + IOUtils.copy(tempFIS, + outputStream); + outputStream.flush(); + } + } } - outputStream = ioService.newOutputStream( nioPath, - commentedOptionFactory.makeCommentedOption( comment, - sessionInfo.getIdentity(), - sessionInfo ) ); - - //InputStream 'content' has been fully read to write to the temp file; so we need to use a new InputStream - //An alternative is to check for content.markSupported() and reset() however since we have a temporary - //file we may as well use it! - tempFIS = new FileInputStream( tempFile ); - IOUtils.copy( tempFIS, - outputStream ); - outputStream.flush(); //Read Path to ensure attributes have been set - final Path newPath = Paths.convert( nioPath ); + final Path newPath = Paths.convert(nioPath); return newPath; - } catch ( Exception e ) { - throw ExceptionUtilities.handleException( e ); + } catch (Exception e) { + throw ExceptionUtilities.handleException(e); } finally { + ioService.endBatch(); try { content.close(); - } catch ( IOException e ) { - throw ExceptionUtilities.handleException( e ); - } - if ( tempFIS != null ) { - try { - tempFIS.close(); - } catch ( IOException e ) { - throw ExceptionUtilities.handleException( e ); - } - } - if ( tempFOS != null ) { - try { - tempFOS.close(); - } catch ( IOException e ) { - throw ExceptionUtilities.handleException( e ); - } - } - if ( outputStream != null ) { - try { - outputStream.close(); - } catch ( IOException e ) { - throw ExceptionUtilities.handleException( e ); - } + } catch (IOException e) { + throw ExceptionUtilities.handleException(e); } } } - void validate( final File tempFile ) { + void validate(final File tempFile) { Workbook workbook = null; try { - workbook = WorkbookFactory.create( tempFile ); - } catch ( IOException e ) { - throw new DecisionTableParseException( "DecisionTableParseException: Failed to open Excel stream, " + "please check that the content is xls97 format.", - e ); - } catch ( Throwable e ) { - throw new DecisionTableParseException( "DecisionTableParseException: " + e.getMessage(), - e ); + workbook = WorkbookFactory.create(tempFile); + } catch (IOException e) { + throw new DecisionTableParseException("DecisionTableParseException: Failed to open Excel stream, " + "please check that the content is xls97 format.", + e); + } catch (Throwable e) { + throw new DecisionTableParseException("DecisionTableParseException: " + e.getMessage(), + e); } finally { - if ( workbook != null ) { + if (workbook != null) { try { workbook.close(); - } catch ( IOException e ) { - throw ExceptionUtilities.handleException( e ); + } catch (IOException e) { + throw ExceptionUtilities.handleException(e); } } } } @Override - public String getSource( final Path path ) { + public String getSource(final Path path) { InputStream inputStream = null; try { final SpreadsheetCompiler compiler = new SpreadsheetCompiler(); - inputStream = ioService.newInputStream( Paths.convert( path ), - StandardOpenOption.READ ); - final String drl = compiler.compile( inputStream, - InputType.XLS ); + inputStream = ioService.newInputStream(Paths.convert(path), + StandardOpenOption.READ); + final String drl = compiler.compile(inputStream, + InputType.XLS); return drl; - } catch ( Exception e ) { - throw new SourceGenerationFailedException( e.getMessage() ); + } catch (Exception e) { + throw new SourceGenerationFailedException(e.getMessage()); } finally { - if ( inputStream != null ) { + if (inputStream != null) { try { inputStream.close(); - } catch ( IOException ioe ) { - throw ExceptionUtilities.handleException( ioe ); + } catch (IOException ioe) { + throw ExceptionUtilities.handleException(ioe); } } } } @Override - public void delete( final Path path, - final String comment ) { + public void delete(final Path path, + final String comment) { try { - deleteService.delete( path, - comment ); - } catch ( Exception e ) { - throw ExceptionUtilities.handleException( e ); + deleteService.delete(path, + comment); + } catch (Exception e) { + throw ExceptionUtilities.handleException(e); } } @Override - public Path rename( final Path path, - final String newName, - final String comment ) { + public Path rename(final Path path, + final String newName, + final String comment) { try { - return renameService.rename( path, - newName, - comment ); - } catch ( Exception e ) { - throw ExceptionUtilities.handleException( e ); + return renameService.rename(path, + newName, + comment); + } catch (Exception e) { + throw ExceptionUtilities.handleException(e); } } @Override - public Path copy( final Path path, - final String newName, - final String comment ) { + public Path copy(final Path path, + final String newName, + final String comment) { try { - return copyService.copy( path, - newName, - comment ); - } catch ( Exception e ) { - throw ExceptionUtilities.handleException( e ); + return copyService.copy(path, + newName, + comment); + } catch (Exception e) { + throw ExceptionUtilities.handleException(e); } } @Override - public Path copy( final Path path, - final String newName, - final Path targetDirectory, - final String comment ) { + public Path copy(final Path path, + final String newName, + final Path targetDirectory, + final String comment) { try { - return copyService.copy( path, - newName, - targetDirectory, - comment ); - - } catch ( Exception e ) { - throw ExceptionUtilities.handleException( e ); + return copyService.copy(path, + newName, + targetDirectory, + comment); + } catch (Exception e) { + throw ExceptionUtilities.handleException(e); } } @Override - public ConversionResult convert( final Path path ) { + public ConversionResult convert(final Path path) { try { - return conversionService.convert( path ); - } catch ( Exception e ) { - throw ExceptionUtilities.handleException( e ); + return conversionService.convert(path); + } catch (Exception e) { + throw ExceptionUtilities.handleException(e); } } @Override - public List validate( final Path path, - final Path resource ) { + public List validate(final Path path, + final Path resource) { try { - return genericValidator.validate( path ); - - } catch ( Exception e ) { - throw ExceptionUtilities.handleException( e ); + return genericValidator.validate(path); + } catch (Exception e) { + throw ExceptionUtilities.handleException(e); } } - private SessionInfo getSessionInfo( final String sessionId ) { - return new SafeSessionInfo( new SessionInfoImpl( sessionId, - authenticationService.getUser() ) ); + private SessionInfo getSessionInfo(final String sessionId) { + return new SafeSessionInfo(new SessionInfoImpl(sessionId, + authenticationService.getUser())); } - } diff --git a/drools-wb-screens/drools-wb-dtable-xls-editor/drools-wb-dtable-xls-editor-backend/src/test/java/org/drools/workbench/screens/dtablexls/backend/server/DecisionTableXLSServiceImplTest.java b/drools-wb-screens/drools-wb-dtable-xls-editor/drools-wb-dtable-xls-editor-backend/src/test/java/org/drools/workbench/screens/dtablexls/backend/server/DecisionTableXLSServiceImplTest.java index fdbed03b972..1a6b2103674 100644 --- a/drools-wb-screens/drools-wb-dtable-xls-editor/drools-wb-dtable-xls-editor-backend/src/test/java/org/drools/workbench/screens/dtablexls/backend/server/DecisionTableXLSServiceImplTest.java +++ b/drools-wb-screens/drools-wb-dtable-xls-editor/drools-wb-dtable-xls-editor-backend/src/test/java/org/drools/workbench/screens/dtablexls/backend/server/DecisionTableXLSServiceImplTest.java @@ -39,6 +39,7 @@ import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Captor; +import org.mockito.InOrder; import org.mockito.Mock; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; @@ -55,6 +56,7 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -114,150 +116,181 @@ public class DecisionTableXLSServiceImplTest { @Before public void setup() throws IOException { - when( authenticationService.getUser() ).thenReturn( user ); - when( user.getIdentifier() ).thenReturn( "user" ); + when(authenticationService.getUser()).thenReturn(user); + when(user.getIdentifier()).thenReturn("user"); - when( path.toURI() ).thenReturn( "file://p0/src/main/resources/dtable.xls" ); - when( inputstream.read( anyObject() ) ).thenReturn( -1 ); - when( ioService.newOutputStream( any( org.uberfire.java.nio.file.Path.class ), - commentedOptionArgumentCaptor.capture() ) ).thenReturn( outputStream ); + when(path.toURI()).thenReturn("file://p0/src/main/resources/dtable.xls"); + when(inputstream.read(anyObject())).thenReturn(-1); } @Test public void testSessionInfoOnCreate() { - this.service = getServiceWithValidationOverride( ( tempFile ) -> { + + when(ioService.write(any(org.uberfire.java.nio.file.Path.class), + any(byte[].class), + commentedOptionArgumentCaptor.capture())).thenReturn(null); + this.service = getServiceWithValidationOverride((tempFile) -> { //Do nothing; tests do not use a *real* XLS file - } ); + }); - service.create( path, - inputstream, - sessionId, - comment ); + service.create(path, + inputstream, + sessionId, + comment); assertCommentedOption(); } @Test public void testSessionInfoOnSave() { - this.service = getServiceWithValidationOverride( ( tempFile ) -> { + when(ioService.newOutputStream(any(org.uberfire.java.nio.file.Path.class), + commentedOptionArgumentCaptor.capture())).thenReturn(outputStream); + this.service = getServiceWithValidationOverride((tempFile) -> { //Do nothing; tests do not use a *real* XLS file - } ); + }); + + service.save(path, + inputstream, + sessionId, + comment); + + final InOrder inOrder = inOrder(ioService); + + inOrder.verify(ioService).startBatch(any()); + inOrder.verify(ioService).newOutputStream(any(), any()); + inOrder.verify(ioService).endBatch(); - service.save( path, - inputstream, - sessionId, - comment ); assertCommentedOption(); } @Test public void inputStreamShouldNotBeReused() throws IOException { - this.service = getServiceWithValidationOverride( ( tempFile ) -> { + + when(ioService.write(any(org.uberfire.java.nio.file.Path.class), + any(byte[].class), + commentedOptionArgumentCaptor.capture())).thenReturn(null); + + this.service = getServiceWithValidationOverride((tempFile) -> { //Do nothing; tests do not use a *real* XLS file - } ); + }); + + mockStatic(IOUtils.class); + + service.create(path, + inputstream, + sessionId, + comment); - mockStatic( IOUtils.class ); + final InOrder inOrder = inOrder(ioService); - service.create( path, - inputstream, - sessionId, - comment ); + inOrder.verify(ioService).startBatch(any()); + inOrder.verify(ioService).write(any(org.uberfire.java.nio.file.Path.class), + any(byte[].class), + any(CommentedOption.class)); + inOrder.verify(ioService).endBatch(); - verifyStatic( times( 1 ) ); - IOUtils.copy( eq( inputstream ), - any( OutputStream.class ) ); + verifyStatic(times(1)); + IOUtils.copy(eq(inputstream), + any(OutputStream.class)); } private void assertCommentedOption() { - this.service = getServiceWithValidationOverride( ( tempFile ) -> { + this.service = getServiceWithValidationOverride((tempFile) -> { //Do nothing; tests do not use a *real* XLS file - } ); + }); final CommentedOption commentedOption = commentedOptionArgumentCaptor.getValue(); - assertNotNull( commentedOption ); - assertEquals( "user", - commentedOption.getName() ); - assertEquals( "123", - commentedOption.getSessionId() ); + assertNotNull(commentedOption); + assertEquals("user", + commentedOption.getName()); + assertEquals("123", + commentedOption.getSessionId()); } @Test public void testInvalidTableNotCreated() throws IOException { - testInvalidTable( ( s ) -> s.create( path, inputstream, sessionId, comment ) ); + + when(ioService.write(any(org.uberfire.java.nio.file.Path.class), + any(byte[].class), + commentedOptionArgumentCaptor.capture())).thenReturn(null); + testInvalidTable((s) -> s.create(path, inputstream, sessionId, comment)); } @Test public void testInvalidTableNotSaved() throws IOException { - testInvalidTable( ( s ) -> s.save( path, inputstream, sessionId, comment ) ); + + when(ioService.newOutputStream(any(org.uberfire.java.nio.file.Path.class), + commentedOptionArgumentCaptor.capture())).thenReturn(outputStream); + testInvalidTable((s) -> s.save(path, inputstream, sessionId, comment)); } - private void testInvalidTable( Consumer serviceConsumer ) throws IOException { - this.service = getServiceWithValidationOverride( ( tempFile ) -> { + private void testInvalidTable(Consumer serviceConsumer) throws IOException { + this.service = getServiceWithValidationOverride((tempFile) -> { // mock an invalid file - Throwable t = new Throwable( "testing invalid xls dt creation" ); - throw new DecisionTableParseException( "DecisionTableParseException: " + t.getMessage(), t ); - } ); + Throwable t = new Throwable("testing invalid xls dt creation"); + throw new DecisionTableParseException("DecisionTableParseException: " + t.getMessage(), t); + }); - mockStatic( IOUtils.class ); - when( IOUtils.copy( any( InputStream.class ), any( OutputStream.class ) ) ).thenReturn( 0 ); + mockStatic(IOUtils.class); + when(IOUtils.copy(any(InputStream.class), any(OutputStream.class))).thenReturn(0); try { - serviceConsumer.accept( service ); - } catch ( RuntimeException e ) { + serviceConsumer.accept(service); + } catch (RuntimeException e) { // this is expected correct behavior } - verify( ioService, never() ).newOutputStream( any( org.uberfire.java.nio.file.Path.class ), any( CommentedOption.class ) ); - verifyStatic( never() ); + verify(ioService, never()).newOutputStream(any(org.uberfire.java.nio.file.Path.class), any(CommentedOption.class)); + verifyStatic(never()); } @Test(expected = DecisionTableParseException.class) public void testValidateNonexistentFile() { - this.service = getServiceWithValidationOverride( null ); + this.service = getServiceWithValidationOverride(null); - service.validate( new File( "" ) ); + service.validate(new File("")); } @Test(expected = DecisionTableParseException.class) public void testValidateEmptyFile() throws IOException { - this.service = getServiceWithValidationOverride( null ); + this.service = getServiceWithValidationOverride(null); - service.validate( File.createTempFile( "emptyxls", null ) ); + service.validate(File.createTempFile("emptyxls", null)); } @Test(expected = DecisionTableParseException.class) public void testValidateFileWithInvalidContent() throws IOException { - this.service = getServiceWithValidationOverride( null ); + this.service = getServiceWithValidationOverride(null); - File tempFile = File.createTempFile( "emptyxls", null ); - try ( FileOutputStream tempFOS = new FileOutputStream( tempFile ) ) { - IOUtils.write( "birdplane!", tempFOS ); + File tempFile = File.createTempFile("emptyxls", null); + try (FileOutputStream tempFOS = new FileOutputStream(tempFile)) { + IOUtils.write("birdplane!", tempFOS); tempFOS.flush(); - service.validate( tempFile ); + service.validate(tempFile); } } @Test public void testValidateFileWithValidContent() throws IOException, URISyntaxException { - this.service = getServiceWithValidationOverride( null ); + this.service = getServiceWithValidationOverride(null); - File tempFile = new File( this.getClass().getResource( "dummy.xls" ).toURI() ); - service.validate( tempFile ); + File tempFile = new File(this.getClass().getResource("dummy.xls").toURI()); + service.validate(tempFile); } - private DecisionTableXLSServiceImpl getServiceWithValidationOverride( Consumer validationOverride ) { - return new DecisionTableXLSServiceImpl( ioService, - copyService, - deleteService, - renameService, - resourceOpenedEvent, - conversionService, - genericValidator, - commentedOptionFactory, - authenticationService ) { + private DecisionTableXLSServiceImpl getServiceWithValidationOverride(Consumer validationOverride) { + return new DecisionTableXLSServiceImpl(ioService, + copyService, + deleteService, + renameService, + resourceOpenedEvent, + conversionService, + genericValidator, + commentedOptionFactory, + authenticationService) { @Override - void validate( final File tempFile ) { - if ( validationOverride != null ) { - validationOverride.accept( tempFile ); + void validate(final File tempFile) { + if (validationOverride != null) { + validationOverride.accept(tempFile); } else { - super.validate( tempFile ); + super.validate(tempFile); } } };