Skip to content

Commit

Permalink
[PDI-14618] - Creating a connection from an open KTR or KJB when conn…
Browse files Browse the repository at this point in the history
…ected to repository allows to create connections with the same names (identical) or with the same names but with differ case

- check uniqueness of name when adding and editing connection from step dialog
- add tests
- fix Checkstyle violations
  • Loading branch information
Andrey Khayrutdinov authored and akhayrutdinov committed Nov 27, 2015
1 parent 1d3775a commit 59d8e73
Show file tree
Hide file tree
Showing 2 changed files with 243 additions and 37 deletions.
121 changes: 84 additions & 37 deletions ui/src/org/pentaho/di/ui/trans/step/BaseStepDialog.java
Expand Up @@ -2,7 +2,7 @@
*
* Pentaho Data Integration
*
* Copyright (C) 2002-2013 by Pentaho : http://www.pentaho.com
* Copyright (C) 2002-2015 by Pentaho : http://www.pentaho.com
*
*******************************************************************************
*
Expand Down Expand Up @@ -365,7 +365,7 @@ private static final void positionBottomButtons( Composite composite, Button[] b
}

// Compute the left side of the 1st button
switch( alignment ) {
switch ( alignment ) {
case BUTTON_ALIGNMENT_CENTER:
centerButtons( buttons, largest.width, margin, lastControl );
break;
Expand Down Expand Up @@ -705,9 +705,7 @@ public void widgetSelected( SelectionEvent e ) {
DatabaseMeta newDBInfo = cdw.createAndRunDatabaseWizard( shell, props, transMeta.getDatabases() );
if ( newDBInfo != null ) {
transMeta.addDatabase( newDBInfo );
wConnection.removeAll();
addDatabases( wConnection );
selectDatabase( wConnection, newDBInfo.getName() );
reinitConnectionDropDown( wConnection, newDBInfo.getName() );
}
}
} );
Expand All @@ -724,21 +722,7 @@ public void widgetSelected( SelectionEvent e ) {
// NEW button
//
wbnConnection.setText( BaseMessages.getString( PKG, "BaseStepDialog.NewConnectionButton.Label" ) );
wbnConnection.addSelectionListener( new SelectionAdapter() {
public void widgetSelected( SelectionEvent e ) {
DatabaseMeta databaseMeta = new DatabaseMeta();
databaseMeta.shareVariablesWith( transMeta );
DatabaseDialog cid = getDatabaseDialog( shell );
cid.setDatabaseMeta( databaseMeta );
cid.setModalDialog( true );
if ( cid.open() != null ) {
transMeta.addDatabase( databaseMeta );
wConnection.removeAll();
addDatabases( wConnection, databaseType );
selectDatabase( wConnection, databaseMeta.getName() );
}
}
} );
wbnConnection.addSelectionListener( new AddConnectionListener( wConnection ) );
fdbConnection = new FormData();
fdbConnection.right = new FormAttachment( wbwConnection, -margin );
if ( previous != null ) {
Expand All @@ -752,23 +736,7 @@ public void widgetSelected( SelectionEvent e ) {
// Edit button
//
wbeConnection.setText( BaseMessages.getString( PKG, "BaseStepDialog.EditConnectionButton.Label" ) );
wbeConnection.addSelectionListener( new SelectionAdapter() {
public void widgetSelected( SelectionEvent e ) {
DatabaseMeta databaseMeta = transMeta.findDatabase( wConnection.getText() );
if ( databaseMeta != null ) {
databaseMeta.shareVariablesWith( transMeta );

DatabaseDialog cid = getDatabaseDialog( shell );
cid.setDatabaseMeta( databaseMeta );
cid.setModalDialog( true );
if ( cid.open() != null ) {
wConnection.removeAll();
addDatabases( wConnection );
selectDatabase( wConnection, databaseMeta.getName() );
}
}
}
} );
wbeConnection.addSelectionListener( new EditConnectionListener( wConnection ) );
fdeConnection = new FormData();
fdeConnection.right = new FormAttachment( wbnConnection, -margin );
if ( previous != null ) {
Expand All @@ -794,6 +762,39 @@ public void widgetSelected( SelectionEvent e ) {
return wConnection;
}

// package-local visibility to testing purposes
String showDbDialogUnlessCancelledOrValid( DatabaseMeta changing, DatabaseMeta origin ) {
changing.shareVariablesWith( transMeta );
DatabaseDialog cid = getDatabaseDialog( shell );
cid.setDatabaseMeta( changing );
cid.setModalDialog( true );

String name = null;
boolean repeat = true;
while ( repeat ) {
name = cid.open();
if ( name == null ) {
// Cancel was pressed
repeat = false;
} else {
DatabaseMeta same = transMeta.findDatabase( name );
if ( same == null || same == origin ) {
// OK was pressed and input is valid
repeat = false;
} else {
DatabaseDialog.showDatabaseExistsDialog( shell, changing );
}
}
}
return name;
}

private void reinitConnectionDropDown( CCombo dropDown, String selected ) {
dropDown.removeAll();
addDatabases( dropDown );
selectDatabase( dropDown, selected );
}

/**
* Gets the database dialog.
*
Expand Down Expand Up @@ -1346,4 +1347,50 @@ public void setMetaStore( IMetaStore metaStore ) {
protected String getPathOf( RepositoryElementMetaInterface object ) {
return DialogUtils.getPathOf( object );
}


// package local visibility for testing purposes
class AddConnectionListener extends SelectionAdapter {

private final CCombo wConnection;

public AddConnectionListener( CCombo wConnection ) {
this.wConnection = wConnection;
}

@Override
public void widgetSelected( SelectionEvent e ) {
DatabaseMeta databaseMeta = new DatabaseMeta();
String connectionName = showDbDialogUnlessCancelledOrValid( databaseMeta, null );
if ( connectionName != null ) {
transMeta.addDatabase( databaseMeta );
reinitConnectionDropDown( wConnection, databaseMeta.getName() );
}
}
}

// package local visibility for testing purposes
class EditConnectionListener extends SelectionAdapter {

private final CCombo wConnection;

public EditConnectionListener( CCombo wConnection ) {
this.wConnection = wConnection;
}

public void widgetSelected( SelectionEvent e ) {
DatabaseMeta databaseMeta = transMeta.findDatabase( wConnection.getText() );
if ( databaseMeta != null ) {
// cloning to avoid spoiling data on cancel or incorrect input
DatabaseMeta clone = (DatabaseMeta) databaseMeta.clone();
String connectionName = showDbDialogUnlessCancelledOrValid( clone, databaseMeta );
if ( connectionName != null ) {
// need to replace the old connection with a new one
transMeta.removeDatabase( transMeta.indexOfDatabase( databaseMeta ) );
transMeta.addDatabase( clone );
reinitConnectionDropDown( wConnection, connectionName );
}
}
}
}
}
@@ -0,0 +1,159 @@
/*! ******************************************************************************
*
* Pentaho Data Integration
*
* Copyright (C) 2002-2015 by Pentaho : http://www.pentaho.com
*
*******************************************************************************
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
******************************************************************************/

package org.pentaho.di.ui.trans.step;

import org.eclipse.swt.custom.CCombo;
import org.junit.BeforeClass;
import org.junit.Test;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.pentaho.di.core.KettleEnvironment;
import org.pentaho.di.core.database.DatabaseMeta;
import org.pentaho.di.trans.TransMeta;

import static org.junit.Assert.*;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

/**
* @author Andrey Khayrutdinov
*/
public class BaseStepDialog_ConnectionLine_Test {

private static String INITIAL_NAME = "qwerty";
private static String INPUT_NAME = "asdfg";

private static String INITIAL_HOST = "1.2.3.4";
private static String INPUT_HOST = "5.6.7.8";

@BeforeClass
public static void initKettle() throws Exception {
KettleEnvironment.init();
}


@Test
public void adds_WhenConnectionNameIsUnique() throws Exception {
TransMeta transMeta = new TransMeta();

invokeAddConnectionListener( transMeta, INPUT_NAME );

assertOnlyDbExists( transMeta, INPUT_NAME, INPUT_HOST );
}

@Test
public void ignores_WhenConnectionNameIsUsed() throws Exception {
TransMeta transMeta = new TransMeta();
transMeta.addDatabase( createDefaultDatabase() );

invokeAddConnectionListener( transMeta, null );

assertOnlyDbExists( transMeta, INITIAL_NAME, INITIAL_HOST );
}

private void invokeAddConnectionListener( TransMeta transMeta, String answeredName ) throws Exception {
BaseStepDialog dialog = mock( BaseStepDialog.class );
when( dialog.showDbDialogUnlessCancelledOrValid( any( DatabaseMeta.class ), any( DatabaseMeta.class ) ) )
.thenAnswer( new PropsSettingAnswer( answeredName, INPUT_HOST ) );

dialog.transMeta = transMeta;
dialog.new AddConnectionListener( mock( CCombo.class ) ).widgetSelected( null );
}


@Test
public void edits_WhenNotRenamed() throws Exception {
TransMeta transMeta = new TransMeta();
transMeta.addDatabase( createDefaultDatabase() );

invokeEditConnectionListener( transMeta, INITIAL_NAME );

assertOnlyDbExists( transMeta, INITIAL_NAME, INPUT_HOST );
}

@Test
public void edits_WhenNewNameIsUnique() throws Exception {
TransMeta transMeta = new TransMeta();
transMeta.addDatabase( createDefaultDatabase() );

invokeEditConnectionListener( transMeta, INPUT_NAME );

assertOnlyDbExists( transMeta, INPUT_NAME, INPUT_HOST );
}

@Test
public void ignores_WhenNewNameIsUsed() throws Exception {
TransMeta transMeta = new TransMeta();
transMeta.addDatabase( createDefaultDatabase() );

invokeEditConnectionListener( transMeta, null );

assertOnlyDbExists( transMeta, INITIAL_NAME, INITIAL_HOST );
}

private void invokeEditConnectionListener( TransMeta transMeta, String answeredName ) throws Exception {
BaseStepDialog dialog = mock( BaseStepDialog.class );
when( dialog.showDbDialogUnlessCancelledOrValid( any( DatabaseMeta.class ), any( DatabaseMeta.class ) ) )
.thenAnswer( new PropsSettingAnswer( answeredName, INPUT_HOST ) );

CCombo combo = mock( CCombo.class );
when( combo.getText() ).thenReturn( INITIAL_NAME );

dialog.transMeta = transMeta;
dialog.new EditConnectionListener( combo ).widgetSelected( null );
}


private DatabaseMeta createDefaultDatabase() {
DatabaseMeta existing = new DatabaseMeta();
existing.setName( INITIAL_NAME );
existing.setHostname( INITIAL_HOST );
return existing;
}

private void assertOnlyDbExists( TransMeta transMeta, String name, String host ) {
assertEquals( 1, transMeta.getDatabases().size() );
assertEquals( name, transMeta.getDatabase( 0 ).getName() );
assertEquals( host, transMeta.getDatabase( 0 ).getHostname() );
}


private static class PropsSettingAnswer implements Answer<String> {
private final String name;
private final String host;

public PropsSettingAnswer( String name, String host ) {
this.name = name;
this.host = host;
}

@Override
public String answer( InvocationOnMock invocation ) throws Throwable {
DatabaseMeta meta = (DatabaseMeta) invocation.getArguments()[ 0 ];
meta.setName( name );
meta.setHostname( host );
return name;
}
}
}

0 comments on commit 59d8e73

Please sign in to comment.