From dev-return-7997-apmail-continuum-dev-archive=continuum.apache.org@continuum.apache.org Thu Apr 09 00:22:17 2009 Return-Path: Delivered-To: apmail-continuum-dev-archive@www.apache.org Received: (qmail 43299 invoked from network); 9 Apr 2009 00:22:15 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 9 Apr 2009 00:22:15 -0000 Received: (qmail 620 invoked by uid 500); 9 Apr 2009 00:22:15 -0000 Delivered-To: apmail-continuum-dev-archive@continuum.apache.org Received: (qmail 528 invoked by uid 500); 9 Apr 2009 00:22:15 -0000 Mailing-List: contact dev-help@continuum.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@continuum.apache.org Delivered-To: mailing list dev@continuum.apache.org Received: (qmail 516 invoked by uid 99); 9 Apr 2009 00:22:15 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 09 Apr 2009 00:22:15 +0000 X-ASF-Spam-Status: No, hits=3.4 required=10.0 tests=HTML_MESSAGE,SPF_NEUTRAL X-Spam-Check-By: apache.org Received-SPF: neutral (nike.apache.org: local policy) Received: from [61.9.189.137] (HELO nschwmtas01p.mx.bigpond.com) (61.9.189.137) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 09 Apr 2009 00:22:02 +0000 Received: from nschwotgx03p.mx.bigpond.com ([124.179.8.45]) by nschwmtas01p.mx.bigpond.com with ESMTP id <20090409002138.HUFQ5250.nschwmtas01p.mx.bigpond.com@nschwotgx03p.mx.bigpond.com> for ; Thu, 9 Apr 2009 00:21:38 +0000 Received: from [10.0.0.1] (really [124.179.8.45]) by nschwotgx03p.mx.bigpond.com with ESMTP id <20090409002137.LTFD13025.nschwotgx03p.mx.bigpond.com@[10.0.0.1]> for ; Thu, 9 Apr 2009 00:21:37 +0000 Message-Id: <29D7C7D2-7397-4350-9EF3-CA32A6A61635@apache.org> From: Brett Porter To: dev@continuum.apache.org In-Reply-To: <20090408235706.E99B72388975@eris.apache.org> Content-Type: multipart/alternative; boundary=Apple-Mail-576--1063710416 Mime-Version: 1.0 (Apple Message framework v930.3) Subject: Re: svn commit: r763464 - in /continuum/trunk: continuum-purge/ continuum-purge/src/main/java/org/apache/continuum/purge/ continuum-purge/src/main/java/org/apache/continuum/purge/executor/ continuum-webapp/src/main/java/org/apache/continuum/web/util/ conti... Date: Thu, 9 Apr 2009 10:21:34 +1000 References: <20090408235706.E99B72388975@eris.apache.org> X-Mailer: Apple Mail (2.930.3) X-RPD-ScanID: Class unknown; VirusThreatLevel unknown, RefID str=0001.0A150201.49DD3F92.0073,ss=1,fgs=0 X-Virus-Checked: Checked by ClamAV on apache.org --Apple-Mail-576--1063710416 Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Content-Transfer-Encoding: 7bit Hi Jev, Some comments on this... On 09/04/2009, at 9:57 AM, jzurbano@apache.org wrote: > Modified: continuum/trunk/continuum-purge/pom.xml > URL: http://svn.apache.org/viewvc/continuum/trunk/continuum-purge/pom.xml?rev=763464&r1=763463&r2=763464&view=diff > = > = > = > = > = > = > = > = > ====================================================================== > --- continuum/trunk/continuum-purge/pom.xml (original) > +++ continuum/trunk/continuum-purge/pom.xml Wed Apr 8 23:57:04 2009 > @@ -86,6 +86,11 @@ > > > > + > + org.slf4j > + slf4j-log4j12 > + runtime > + I find it's best to just put the simple logger in at test scope and let the eventual user (in continuum, the webapp and the CLI apps) determine the runtime logging framework. > + private static final char DELIM = ' '; > > + private Logger logger = LoggerFactory.getLogger( "AuditLog" ); Instead of doing the log message processing in the executor, how about moving all the logging activity into an audit log component, similar to the one in redback? > import org.apache.maven.continuum.ContinuumException; > import > org > .apache > .maven.continuum.builddefinition.BuildDefinitionServiceException; > import > org > .apache > .maven.continuum.project.builder.ContinuumProjectBuildingResult; > @@ -58,9 +63,15 @@ > throws ContinuumException > { > ContinuumProjectBuildingResult result = null; > + > + String groupId = ""; > + > + String artifactId = ""; > + > + String resource = ""; > > // TODO: remove this part once uploading of an m2 project > with modules is supported ( CONTINUUM-1098 ) > - if ( checkProtocol == false ) > + if ( ( checkProtocol == false ) || ( ( checkProtocol == > true ) && ( pomUrl.startsWith( FILE_SCHEME ) ) ) ) > { > MavenXpp3Reader m2pomReader = new MavenXpp3Reader(); > > @@ -80,10 +91,14 @@ > } > > Model model = m2pomReader.read( new > FileReader( filePath ) ); > + > + groupId = model.getGroupId(); > + artifactId = model.getArtifactId(); > + resource = groupId + ":" + artifactId; > > List modules = model.getModules(); > > - if ( modules != null && modules.size() != 0 ) > + if ( ( checkProtocol == false ) && ( modules != > null && modules.size() != 0 ) ) > { > result = new ContinuumProjectBuildingResult(); > > result.addError( ERROR_UPLOADING_M2_PROJECT_WITH_MODULES ); > @@ -102,12 +117,49 @@ > throw new > ContinuumException( ERROR_READING_POM_EXCEPTION_MESSAGE, e ); > } > } > + else > + { > + if ( ( pomUrl.startsWith( "http" ) ) && > ( pomUrl.endsWith( "pom.xml" ) ) ) > + { > + try > + { > + URL url = new URL( pomUrl ); > + BufferedReader in = new BufferedReader( new > InputStreamReader( url.openStream() ) ); > + StringBuilder content = new StringBuilder(); > + String line = in.readLine(); > + > + while ( line != null ) > + { > + content.append( line ); > + line = in.readLine(); > + } > + in.close(); > + > + if ( content.length() > 0 ) > + { > + groupId = getSubString( content.toString(), > "", "" ); > + artifactId = > getSubString( content.toString(), "", "" ); > + resource = groupId + ":" + artifactId; > + } > + } > + catch ( MalformedURLException e ) > + { > + > addActionError( ERROR_READING_POM_EXCEPTION_MESSAGE ); > + } > + catch ( IOException e ) > + { > + throw new > ContinuumException( ERROR_READING_POM_EXCEPTION_MESSAGE, e ); > + } > + } > + } > > if ( result == null ) > { > result = getContinuum().addMavenTwoProject( pomUrl, > selectedProjectGroup, checkProtocol, scmUseCache, > ! > this.isNonRecursiveProject(), this.getBuildDefinitionTemplateId() ); > } > + > + triggerAuditEvent( getPrincipal(), > AuditLogConstants.PROJECT, resource, > AuditLogConstants.ADD_M2_PROJECT ); > > return result; > } > @@ -159,5 +211,16 @@ > { > this.nonRecursiveProject = nonRecursiveProject; > } > + > + private String getSubString( String content, String tagStart, > String tagEnd ) > + { > + String subString = ""; > + > + int start = content.indexOf( tagStart ) + tagStart.length(); > + int end = content.indexOf( tagEnd ); > + subString = content.substring( start, end ); > + > + return subString; > + } > > } Most of this seems an unrelated change? > > + > + Project proj = getContinuum().getProject( projectId ); > + triggerAuditEvent( getPrincipal(), > AuditLogConstants.PROJECT, proj.getGroupId() + ":" + > proj.getArtifactId(), AuditLogConstants.FORCE_BUILD ); Getting the project can be costly, just for logging - can you use existing IDs instead? > > + > + Project proj = getContinuum().getProject( projectId ); > + > + triggerAuditEvent( getPrincipal(), > AuditLogConstants.PROJECT, proj.getGroupId() + ":" + > proj.getArtifactId(), AuditLogConstants.CANCEL_BUILD ); as above :) > > + > + Project proj = getContinuum().getProject( projectId ); > + triggerAuditEvent( getPrincipal(), > AuditLogConstants.PROJECT, proj.getGroupId() + ":" + > + proj.getArtifactId(), > AuditLogConstants.REMOVE_PROJECT ); Another one here. > > - logger.info( "Removing Project with id=" + > projectId ); > + Project proj = > getContinuum().getProject( projectId ); > + triggerAuditEvent( getPrincipal(), > AuditLogConstants.PROJECT, proj.getGroupId() + ":" + > + proj.getArtifactId(), > AuditLogConstants.REMOVE_PROJECT ); And one more :) > > Modified: continuum/trunk/continuum-webapp/src/main/resources/META- > INF/plexus/application.xml > URL: http://svn.apache.org/viewvc/continuum/trunk/continuum-webapp/src/main/resources/META-INF/plexus/application.xml?rev=763464&r1=763463&r2=763464&view=diff > = > = > = > = > = > = > = > = > ====================================================================== > --- continuum/trunk/continuum-webapp/src/main/resources/META-INF/ > plexus/application.xml (original) > +++ continuum/trunk/continuum-webapp/src/main/resources/META-INF/ > plexus/application.xml Wed Apr 8 23:57:04 2009 > @@ -157,7 +157,7 @@ > http > https > ftp > - > + file > > > Seems to have snuck in? > > + > + > + > + > + If using a class, you can have a more specific name like the one below. Don't forget to add additivity="false"! > > name="org.codehaus.plexus.redback.struts2.action.AuditEvent" > additivity="false"> > > > Cheers, Brett --Apple-Mail-576--1063710416--