From 38c59a524df30415d11c81d1022ce1d60ec3be57 Mon Sep 17 00:00:00 2001 From: Brent Atkinson Date: Mon, 4 May 2015 01:43:08 +0000 Subject: [PATCH] Fixed broken build report action test. git-svn-id: https://svn.apache.org/repos/asf/continuum/trunk@1677512 13f79535-47bb-0310-9956-ffa450edef68 --- .../web/action/ViewBuildsReportAction.java | 73 +++++++++++++------ .../web/action/ContinuumActionSupport.java | 10 +++ .../action/ViewBuildsReportActionTest.java | 52 ++++++++----- .../stub/ViewBuildsReportActionStub.java | 24 ++++++ 4 files changed, 117 insertions(+), 42 deletions(-) diff --git a/continuum-webapp/src/main/java/org/apache/continuum/web/action/ViewBuildsReportAction.java b/continuum-webapp/src/main/java/org/apache/continuum/web/action/ViewBuildsReportAction.java index cecd05575a..6eea83a4cc 100644 --- a/continuum-webapp/src/main/java/org/apache/continuum/web/action/ViewBuildsReportAction.java +++ b/continuum-webapp/src/main/java/org/apache/continuum/web/action/ViewBuildsReportAction.java @@ -135,9 +135,9 @@ public static boolean knownState( int state ) private Map buildStatuses; - private Map projectGroups; + private Map groupSelections; - private Map permittedGroups; + private Map permittedGroupMap; private List filteredResults = new ArrayList(); @@ -148,7 +148,7 @@ public void setServletResponse( HttpServletResponse response ) this.rawResponse = response; } - private boolean isAuthorized( String projectGroupName ) + protected boolean isAuthorized( String projectGroupName ) { try { @@ -166,19 +166,48 @@ public void prepare() { super.prepare(); - // Populate the state drop downs - buildStatuses = new LinkedHashMap(); - buildStatuses.put( 0, "ALL" ); - for ( ResultState state : ResultState.values() ) + Collection permittedGroups = getAuthorizedGroups(); + + groupSelections = createGroupSelections( permittedGroups ); + permittedGroupMap = createPermittedGroupMap( permittedGroups ); + buildStatuses = createStatusSelections(); + } + + protected Map createPermittedGroupMap( Collection allowedGroups ) + { + Map result = new HashMap(); + for ( ProjectGroup group : allowedGroups ) { - buildStatuses.put( state.getDataId(), getText( state.getTextKey() ) ); + result.put( group.getName(), group.getId() ); } + return result; + } - permittedGroups = new HashMap(); - projectGroups = new LinkedHashMap(); - projectGroups.put( 0, "ALL" ); + protected Map createGroupSelections( Collection permittedGroups ) + { + Map result = new LinkedHashMap(); + result.put( 0, "ALL" ); + for ( ProjectGroup group : permittedGroups ) + { + result.put( group.getId(), group.getName() ); + } + return result; + } + + protected Map createStatusSelections() + { + Map result = new LinkedHashMap(); + result.put( 0, "ALL" ); + for ( ResultState state : ResultState.values() ) + { + result.put( state.getDataId(), getText( state.getTextKey() ) ); + } + return result; + } - // TODO: Use these to limit results at the data layer + protected Collection getAuthorizedGroups() + { + Collection permitted = new ArrayList(); List groups = getContinuum().getAllProjectGroups(); if ( groups != null ) { @@ -187,11 +216,11 @@ public void prepare() String groupName = group.getName(); if ( isAuthorized( groupName ) ) { - projectGroups.put( group.getId(), groupName ); - permittedGroups.put( groupName, group.getId() ); + permitted.add( group ); } } } + return permitted; } public String init() @@ -206,7 +235,7 @@ public String init() return REQUIRES_AUTHORIZATION; } - if ( permittedGroups.isEmpty() ) + if ( permittedGroupMap.isEmpty() ) { addActionError( getText( "projectBuilds.report.noGroupsAuthorized" ) ); return REQUIRES_AUTHORIZATION; @@ -228,7 +257,7 @@ public String execute() return REQUIRES_AUTHORIZATION; } - if ( permittedGroups.isEmpty() ) + if ( permittedGroupMap.isEmpty() ) { addActionError( getText( "projectBuilds.report.noGroupsAuthorized" ) ); return REQUIRES_AUTHORIZATION; @@ -262,7 +291,7 @@ public String execute() } else { - groupIds.addAll( permittedGroups.values() ); + groupIds.addAll( permittedGroupMap.values() ); } // Users can preview a limited number of records (use export for more) @@ -279,7 +308,7 @@ public String execute() for ( BuildResult result : results ) { - if ( permittedGroups.containsKey( result.getProject().getProjectGroup().getName() ) ) + if ( permittedGroupMap.containsKey( result.getProject().getProjectGroup().getName() ) ) { filteredResults.add( result ); } @@ -329,7 +358,7 @@ public String downloadBuildsReport() return REQUIRES_AUTHORIZATION; } - if ( permittedGroups.isEmpty() ) + if ( permittedGroupMap.isEmpty() ) { addActionError( getText( "projectBuilds.report.noGroupsAuthorized" ) ); return REQUIRES_AUTHORIZATION; @@ -377,7 +406,7 @@ public String downloadBuildsReport() } else { - groupIds.addAll( permittedGroups.values() ); + groupIds.addAll( permittedGroupMap.values() ); } // Build the output file by walking through the results in batches @@ -395,7 +424,7 @@ public String downloadBuildsReport() for ( BuildResult result : results ) { - if ( !permittedGroups.containsKey( result.getProject().getProjectGroup().getName() ) ) + if ( !permittedGroupMap.containsKey( result.getProject().getProjectGroup().getName() ) ) { continue; } @@ -555,6 +584,6 @@ public List getFilteredResults() public Map getProjectGroups() { - return projectGroups; + return groupSelections; } } \ No newline at end of file diff --git a/continuum-webapp/src/main/java/org/apache/maven/continuum/web/action/ContinuumActionSupport.java b/continuum-webapp/src/main/java/org/apache/maven/continuum/web/action/ContinuumActionSupport.java index 2ad763332f..eca131c7e0 100644 --- a/continuum-webapp/src/main/java/org/apache/maven/continuum/web/action/ContinuumActionSupport.java +++ b/continuum-webapp/src/main/java/org/apache/maven/continuum/web/action/ContinuumActionSupport.java @@ -79,6 +79,16 @@ public void prepare() } } + /** + * Here for unit testing support, it allows configuring a mock security session. + * + * @param securitySession + */ + protected void setSecuritySession( SecuritySession securitySession ) + { + this.securitySession = securitySession; + } + public Continuum getContinuum() { return continuum; diff --git a/continuum-webapp/src/test/java/org/apache/continuum/web/action/ViewBuildsReportActionTest.java b/continuum-webapp/src/test/java/org/apache/continuum/web/action/ViewBuildsReportActionTest.java index 58ede83c0d..8915588042 100644 --- a/continuum-webapp/src/test/java/org/apache/continuum/web/action/ViewBuildsReportActionTest.java +++ b/continuum-webapp/src/test/java/org/apache/continuum/web/action/ViewBuildsReportActionTest.java @@ -25,13 +25,17 @@ import org.apache.maven.continuum.model.project.BuildResult; import org.apache.maven.continuum.model.project.Project; import org.apache.maven.continuum.model.project.ProjectGroup; +import org.codehaus.plexus.redback.system.SecuritySession; import org.junit.Before; import org.junit.Test; import javax.servlet.http.HttpServletResponse; import java.io.ByteArrayOutputStream; import java.io.PrintWriter; +import java.text.DateFormat; +import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Calendar; import java.util.Date; import java.util.List; @@ -48,6 +52,8 @@ public class ViewBuildsReportActionTest private List buildResults = new ArrayList(); + private ProjectGroup allowedGroup; + @Before public void setUp() throws Exception @@ -55,17 +61,15 @@ public void setUp() continuum = mock( Continuum.class ); action = new ViewBuildsReportActionStub(); + action.setSecuritySession( mock( SecuritySession.class ) ); action.setContinuum( continuum ); - } - @Test - public void testInvalidRowCount() - { - String result = action.execute(); + allowedGroup = new ProjectGroup(); + allowedGroup.setId( 1 ); + allowedGroup.setName( "Allowed Group" ); + action.setAuthorizedGroups( Arrays.asList( allowedGroup ) ); - assertEquals( Action.INPUT, result ); - assertTrue( action.hasFieldErrors() ); - assertFalse( action.hasActionErrors() ); + action.prepare(); } @Test @@ -86,7 +90,7 @@ public void testMalformedStartDate() action.setStartDate( "not a date" ); String result = action.execute(); - assertEquals( Action.ERROR, result ); + assertEquals( Action.INPUT, result ); assertTrue( action.hasActionErrors() ); assertFalse( action.hasFieldErrors() ); } @@ -97,7 +101,7 @@ public void testMalformedEndDate() action.setEndDate( "not a date" ); String result = action.execute(); - assertEquals( Action.ERROR, result ); + assertEquals( Action.INPUT, result ); assertTrue( action.hasActionErrors() ); assertFalse( action.hasFieldErrors() ); } @@ -152,7 +156,7 @@ public void testExportToCsv() String result = action.downloadBuildsReport(); assertNull( "result should be null", result ); - assertFileContentsEqual( out.toString(), cal.getTime().toString() ); + assertExportContents( results, out.toString() ); } private void assertSuccessResult( String result ) @@ -168,12 +172,9 @@ private List createBuildResult( long timeInMillis ) BuildResult result = new BuildResult(); - ProjectGroup group = new ProjectGroup(); - group.setName( "Test Group" ); - Project project = new Project(); project.setName( "Test Project" ); - project.setProjectGroup( group ); + project.setProjectGroup( allowedGroup ); result.setProject( project ); result.setState( 2 ); @@ -185,11 +186,22 @@ private List createBuildResult( long timeInMillis ) return results; } - private void assertFileContentsEqual( String report, String buildDate ) + private void assertExportContents( List results, String actualContents ) { - String result = "Project Group,Project Name,Build Date,Triggered By,Build Status\n" + - "Test Group,Test Project," + buildDate + ",test-admin,Ok\n"; - - assertEquals( report, result ); + DateFormat dateFormat = new SimpleDateFormat( "yyyy-MM-dd'T'HH:mm:ss.SSSZ" ); + String expectedContents = "Group,Project,ID,Build#,Started,Duration,Triggered By,Status\n"; + for ( BuildResult result : results ) + { + Project p = result.getProject(); + ProjectGroup pg = p.getProjectGroup(); + expectedContents += String.format( "%s,%s,%s,%s,%s,%s,%s,%s\n", + pg.getName(), p.getName(), result.getId(), result.getBuildNumber(), + dateFormat.format( new Date( result.getStartTime() ) ), + ( result.getEndTime() - result.getStartTime() ) / 1000, + result.getUsername(), + ViewBuildsReportAction.ResultState.fromId( + result.getState() ).getTextKey() ); + } + assertEquals( expectedContents, actualContents ); } } diff --git a/continuum-webapp/src/test/java/org/apache/continuum/web/action/stub/ViewBuildsReportActionStub.java b/continuum-webapp/src/test/java/org/apache/continuum/web/action/stub/ViewBuildsReportActionStub.java index 97137e33f0..d9c6815430 100644 --- a/continuum-webapp/src/test/java/org/apache/continuum/web/action/stub/ViewBuildsReportActionStub.java +++ b/continuum-webapp/src/test/java/org/apache/continuum/web/action/stub/ViewBuildsReportActionStub.java @@ -20,10 +20,34 @@ */ import org.apache.continuum.web.action.ViewBuildsReportAction; +import org.apache.maven.continuum.model.project.ProjectGroup; +import org.codehaus.plexus.redback.system.SecuritySession; + +import java.util.Collection; +import java.util.Collections; public class ViewBuildsReportActionStub extends ViewBuildsReportAction { + + Collection authorizedGroups = Collections.EMPTY_LIST; + + public void setSecuritySession( SecuritySession securitySession ) + { + super.setSecuritySession( securitySession ); + } + + @Override + protected Collection getAuthorizedGroups() + { + return authorizedGroups; + } + + public void setAuthorizedGroups( Collection authorizedGroups ) + { + this.authorizedGroups = authorizedGroups; + } + protected void checkViewProjectGroupAuthorization( String resource ) { // skip authorization check