Skip to content

Commit

Permalink
Merge pull request #8 from mjdetullio/feature/ignore_unsubmitted_files
Browse files Browse the repository at this point in the history 
Retrieve date and username per changelist number.
  • Loading branch information
@henryju
henryju committed Jan 5, 2016
2 parents 989dd92 + bae4109 commit c1b3ea6
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 108 deletions.
82 changes: 29 additions & 53 deletions src/main/java/org/sonar/plugins/scm/perforce/PerforceBlameCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,19 @@
package org.sonar.plugins.scm.perforce;

import com.google.common.annotations.VisibleForTesting;
import com.perforce.p4java.core.file.FileSpecOpStatus;
import com.perforce.p4java.core.IChangelist;
import com.perforce.p4java.core.file.IFileAnnotation;
import com.perforce.p4java.core.file.IFileRevisionData;
import com.perforce.p4java.core.file.IFileSpec;
import com.perforce.p4java.exception.P4JavaException;
import com.perforce.p4java.impl.generic.core.file.FileSpec;
import com.perforce.p4java.option.server.GetFileAnnotationsOptions;
import com.perforce.p4java.option.server.GetRevisionHistoryOptions;
import com.perforce.p4java.server.IOptionsServer;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import javax.annotation.Nonnull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.batch.fs.FileSystem;
Expand All @@ -47,6 +44,7 @@ public class PerforceBlameCommand extends BlameCommand {

private static final Logger LOG = LoggerFactory.getLogger(PerforceBlameCommand.class);
private final PerforceConfiguration config;
private final Map<Integer, IChangelist> changelistMap = new HashMap<>();

public PerforceBlameCommand(PerforceConfiguration config) {
this.config = config;
Expand All @@ -61,78 +59,55 @@ public void blame(BlameInput input, BlameOutput output) {
for (InputFile inputFile : input.filesToBlame()) {
blame(inputFile, executor.getServer(), output);
}
} catch (P4JavaException e) {
throw new IllegalStateException(e.getLocalizedMessage(), e);
} finally {
executor.clean();
}
}

@VisibleForTesting
void blame(InputFile inputFile, IOptionsServer server, BlameOutput output) {
void blame(InputFile inputFile, IOptionsServer server, BlameOutput output) throws P4JavaException {
IFileSpec fileSpec = createFileSpec(inputFile);
List<IFileAnnotation> fileAnnotations;
List<IFileRevisionData> revisions;
try {
// Get file annotations
List<IFileSpec> fileSpecs = Arrays.asList(fileSpec);
fileAnnotations = server.getFileAnnotations(fileSpecs, getFileAnnotationOptions());
if (fileAnnotations.size() == 1 && fileAnnotations.get(0).getDepotPath() == null) {
LOG.debug("File " + inputFile + " is not submitted. Skipping it.");
return;
}
// Get revision history
Map<IFileSpec, List<IFileRevisionData>> revisionMap = server.getRevisionHistory(fileSpecs, getRevisionHistoryOptions());
Entry<IFileSpec, List<IFileRevisionData>> singleEntry = revisionMap.entrySet().iterator().next();
IFileSpec resultFileSpec = singleEntry.getKey();
if (!FileSpecOpStatus.VALID.equals(resultFileSpec.getOpStatus()) && !FileSpecOpStatus.INFO.equals(resultFileSpec.getOpStatus())) {
String statusMessage = resultFileSpec.getStatusMessage();
LOG.debug("Unable to get revisions of file " + inputFile + " [" + statusMessage + "]. Skipping it.");
return;
}
revisions = singleEntry.getValue();
} catch (P4JavaException e) {
throw new IllegalStateException(e.getLocalizedMessage(), e);
}
List<IFileSpec> fileSpecs = Collections.singletonList(fileSpec);

computeBlame(inputFile, output, fileAnnotations, revisions);
}

private void computeBlame(InputFile inputFile, BlameOutput output, List<IFileAnnotation> fileAnnotations, List<IFileRevisionData> revisions) {
Map<Integer, Date> changelistDates = new HashMap<>();
Map<Integer, String> changelistAuthors = new HashMap<>();
for (IFileRevisionData revision : revisions) {
changelistDates.put(revision.getChangelistId(), revision.getDate());
changelistAuthors.put(revision.getChangelistId(), revision.getUserName());
// Get file annotations
List<IFileAnnotation> fileAnnotations = server.getFileAnnotations(fileSpecs, getFileAnnotationOptions());
if (fileAnnotations.size() == 1 && fileAnnotations.get(0).getDepotPath() == null) {
LOG.debug("File " + inputFile + " is not submitted. Skipping it.");
return;
}

// Compute blame, getting changelist from server if not already retrieved
List<BlameLine> lines = new ArrayList<>();
for (IFileAnnotation fileAnnotation : fileAnnotations) {
int lowerChangelistId = fileAnnotation.getLower();

IChangelist changelist = changelistMap.get(lowerChangelistId);
if (changelist == null) {
changelist = server.getChangelist(lowerChangelistId);
changelistMap.put(lowerChangelistId, changelist);
}

lines.add(new BlameLine()
.revision(String.valueOf(lowerChangelistId))
.date(changelistDates.get(lowerChangelistId))
.author(changelistAuthors.get(lowerChangelistId)));
.date(changelist.getDate())
.author(changelist.getUsername()));
}

// SONARPLUGINS-3097: Perforce does not report blame on last empty line, so populate from last line with blame
if (lines.size() == (inputFile.lines() - 1)) {
// SONARPLUGINS-3097 Perforce do not report blame on last empty line
lines.add(lines.get(lines.size() - 1));
}
output.blameResult(inputFile, lines);
}

/**
* Creating options for revision history command (filelog).
* @return options for requests.
*/
private static GetRevisionHistoryOptions getRevisionHistoryOptions() {
GetRevisionHistoryOptions options = new GetRevisionHistoryOptions();
options.setIncludeInherited(true);
return options;
output.blameResult(inputFile, lines);
}

/**
* Creating options for file annotation command.
* @return options for requests.
*/
@Nonnull
private static GetFileAnnotationsOptions getFileAnnotationOptions() {
GetFileAnnotationsOptions options = new GetFileAnnotationsOptions();
options.setUseChangeNumbers(true);
Expand All @@ -146,7 +121,8 @@ private static GetFileAnnotationsOptions getFileAnnotationOptions() {
* in the current client workspace.
* @param inputFile file to create file spec for
*/
private static IFileSpec createFileSpec(InputFile inputFile) {
@Nonnull
private static IFileSpec createFileSpec(@Nonnull InputFile inputFile) {
IFileSpec fileSpec = new FileSpec(PerforceExecutor.encodeWildcards(inputFile.absolutePath()));
fileSpec.setEndRevision(IFileSpec.HAVE_REVISION);
return fileSpec;
Expand Down
49 changes: 13 additions & 36 deletions src/main/java/org/sonar/plugins/scm/perforce/PerforceExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.net.URISyntaxException;
import java.util.List;
import java.util.Properties;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
Expand All @@ -58,14 +59,10 @@ public class PerforceExecutor {
/**
* Instantiates a new p4 command helper.
*
* @param repository
* the repository
* @param fileSet
* the file set
* @param logger
* the logger
* @throws ScmException
* the scm exception
* @param config
* the plugin configuration
* @param workDir
* the working directory
*/
public PerforceExecutor(PerforceConfiguration config, File workDir) {
this.config = config;
Expand All @@ -81,25 +78,6 @@ public IOptionsServer getServer() {
return server;
}

/**
* Gets the client.
*
* @return the client
*/
public IClient getClient() {
return client;
}

/**
* Sets the client.
*
* @param client
* the new client
*/
public void setClient(IClient client) {
this.client = client;
}

/**
* Initialize Perforce server and client instances.
*
Expand Down Expand Up @@ -167,11 +145,7 @@ private static boolean isLogin(IOptionsServer connection) throws P4JavaException
return true;
}
// If there is a broker or something else that swallows the message
if (status.isEmpty()) {
return true;
}

return false;
return status.isEmpty();
}

private void createServer() throws URISyntaxException, P4JavaException {
Expand Down Expand Up @@ -295,10 +269,10 @@ private void initClient(File workDir) {
/**
* Creates the client view mapping.
*
* @param repo
* the repo
* @param basedir
* the basedir
* @param p4ClientName
* the Perforce client name
* @return the client view mapping
*/
private ClientViewMapping createClientViewMapping(File basedir, String p4ClientName) {
Expand All @@ -315,7 +289,8 @@ private ClientViewMapping createClientViewMapping(File basedir, String p4ClientN
* the path
* @return the repo location
*/
private String getRepoLocation(String path) {
@Nullable
private String getRepoLocation(@Nonnull String path) {
String location = null;
if (StringUtils.isNotBlank(path) && client != null) {
try {
Expand All @@ -340,7 +315,8 @@ private String getRepoLocation(String path) {
* the repo path
* @return the canonical repo path
*/
private static String getCanonicalRepoPath(String repoPath) {
@Nullable
private static String getCanonicalRepoPath(@Nullable String repoPath) {
if (repoPath == null) {
return null;
}
Expand All @@ -359,6 +335,7 @@ private static String getCanonicalRepoPath(String repoPath) {
* @param filePath the file path
* @return the string
*/
@Nonnull
static String encodeWildcards(@Nullable String filePath) {
if (filePath != null) {
return filePath.replaceAll("%", "%25").replaceAll("\\*", "%2A").replaceAll("#", "%23").replaceAll("@", "%40");
Expand Down
57 changes: 38 additions & 19 deletions src/test/java/org/sonar/plugins/scm/perforce/PerforceBlameCommandTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,14 @@
*/
package org.sonar.plugins.scm.perforce;

import com.perforce.p4java.core.file.FileSpecOpStatus;
import com.perforce.p4java.core.IChangelist;
import com.perforce.p4java.core.file.IFileAnnotation;
import com.perforce.p4java.core.file.IFileRevisionData;
import com.perforce.p4java.core.file.IFileSpec;
import com.perforce.p4java.option.server.GetFileAnnotationsOptions;
import com.perforce.p4java.option.server.GetRevisionHistoryOptions;
import com.perforce.p4java.server.IOptionsServer;

import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.junit.Test;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.scm.BlameCommand.BlameOutput;
Expand All @@ -39,6 +35,7 @@
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyList;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
Expand All @@ -54,7 +51,7 @@ public void testBlameUnSubmittedFile() throws Exception {
IFileAnnotation annotation = mock(IFileAnnotation.class);
when(annotation.getDepotPath()).thenReturn(null);

when(server.getFileAnnotations(anyList(), any(GetFileAnnotationsOptions.class))).thenReturn(Arrays.asList(annotation));
when(server.getFileAnnotations(anyList(), any(GetFileAnnotationsOptions.class))).thenReturn(Collections.singletonList(annotation));

command.blame(mock(InputFile.class), server, blameOutput);

Expand All @@ -71,24 +68,46 @@ public void testBlameSubmittedFile() throws Exception {
when(annotation.getDepotPath()).thenReturn("foo/bar/src/Foo.java");
when(annotation.getLower()).thenReturn(3);

when(server.getFileAnnotations(anyList(), any(GetFileAnnotationsOptions.class))).thenReturn(Arrays.asList(annotation));
when(server.getFileAnnotations(anyList(), any(GetFileAnnotationsOptions.class))).thenReturn(Arrays.asList(annotation, annotation));

Map<IFileSpec, List<IFileRevisionData>> result = new HashMap<>();
IFileSpec fileSpecResult = mock(IFileSpec.class);
when(fileSpecResult.getOpStatus()).thenReturn(FileSpecOpStatus.VALID);
IFileRevisionData revision3 = mock(IFileRevisionData.class);
when(revision3.getChangelistId()).thenReturn(3);
IChangelist changelist = mock(IChangelist.class);
Date date = new Date();
when(revision3.getDate()).thenReturn(date);
when(revision3.getUserName()).thenReturn("jhenry");
result.put(fileSpecResult, Arrays.asList(revision3));
when(changelist.getDate()).thenReturn(date);
when(changelist.getUsername()).thenReturn("jhenry");
when(server.getChangelist(3)).thenReturn(changelist);

when(server.getRevisionHistory(anyList(), any(GetRevisionHistoryOptions.class))).thenReturn(result);
InputFile inputFile = mock(InputFile.class);
command.blame(inputFile, server, blameOutput);

BlameLine line = new BlameLine().revision("3").date(date).author("jhenry");
verify(blameOutput).blameResult(inputFile, Arrays.asList(line, line));
verify(server, times(1)).getChangelist(3);
}

@Test
public void testBlameSubmittedFileLastEmptyLine() throws Exception {
BlameOutput blameOutput = mock(BlameOutput.class);
IOptionsServer server = mock(IOptionsServer.class);
PerforceBlameCommand command = new PerforceBlameCommand(mock(PerforceConfiguration.class));

IFileAnnotation annotation = mock(IFileAnnotation.class);
when(annotation.getDepotPath()).thenReturn("foo/bar/src/Foo.java");
when(annotation.getLower()).thenReturn(3);

when(server.getFileAnnotations(anyList(), any(GetFileAnnotationsOptions.class))).thenReturn(Collections.singletonList(annotation));

IChangelist changelist = mock(IChangelist.class);
Date date = new Date();
when(changelist.getDate()).thenReturn(date);
when(changelist.getUsername()).thenReturn("jhenry");
when(server.getChangelist(3)).thenReturn(changelist);

InputFile inputFile = mock(InputFile.class);
when(inputFile.lines()).thenReturn(2);
command.blame(inputFile, server, blameOutput);

verify(blameOutput).blameResult(inputFile, Arrays.asList(new BlameLine().revision("3").date(date).author("jhenry")));
BlameLine line = new BlameLine().revision("3").date(date).author("jhenry");
verify(blameOutput).blameResult(inputFile, Arrays.asList(line, line));
}

}

0 comments on commit c1b3ea6

Please sign in to comment.