You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@continuum.apache.org by Brett Porter <br...@apache.org> on 2009/04/09 02:21:34 UTC
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...
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 @@
> </exclusion>
> </exclusions>
> </dependency>
> + <dependency>
> + <groupId>org.slf4j</groupId>
> + <artifactId>slf4j-log4j12</artifactId>
> + <scope>runtime</scope>
> + </dependency>
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(),
> "<groupId>", "</groupId>" );
> + artifactId =
> getSubString( content.toString(), "<artifactId>", "</artifactId>" );
> + 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 @@
> <allowedScheme>http</allowedScheme>
> <allowedScheme>https</allowedScheme>
> <allowedScheme>ftp</allowedScheme>
> - <!-- <allowedScheme>file</allowedScheme> -->
> + <allowedScheme>file</allowedScheme>
> </allowedSchemes>
> </configuration>
> </component>
Seems to have snuck in?
>
> +
> + <logger name="AuditLog">
> + <level value="info" />
> + <appender-ref ref="continuumAuditlog" />
> + </logger>
If using a class, you can have a more specific name like the one
below. Don't forget to add additivity="false"!
>
> <logger
> name="org.codehaus.plexus.redback.struts2.action.AuditEvent"
> additivity="false">
> <level value="info" />
>
>
Cheers,
Brett
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...
Posted by "Jevica Arianne B. Zurbano" <je...@gmail.com>.
Thanks for the comments Brett! :)
I will revise the logging with your suggestions.
--
Thanks,
Jev