Skip to content

Commit

Permalink
Remove non-transient loggers from wicket pages (issue #21?)
Browse files Browse the repository at this point in the history 
Two Wicket pages in Gitblit had non-transient loggers. Replace them by
transient ones.
  • Loading branch information
@tomaswolf
tomaswolf committed Oct 23, 2016
1 parent e39075d commit 53539dd
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 22 deletions.
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ limitations under the License.
<exclude>com/gitblit/wicket/pages/RootPage$*.class</exclude>
<exclude>com/gitblit/wicket/pages/BasePage.class</exclude><!-- Gerrit footer link -->
<exclude>com/gitblit/wicket/pages/BasePage$*.class</exclude>
<exclude>com/gitblit/wicket/pages/BlobPage.class</exclude><!-- Bug fix -->
<exclude>com/gitblit/wicket/pages/BlobPage.class</exclude><!-- Redirect to tree -->
<exclude>com/gitblit/wicket/pages/BlobPage$*.class</exclude>
<exclude>com/gitblit/wicket/pages/LogoutPage.class</exclude><!-- Logout via redirect to Gerrit's /logout -->
<exclude>com/gitblit/wicket/pages/LogoutPage$*.class</exclude>
Expand Down
31 changes: 19 additions & 12 deletions src/main/java/com/gitblit/wicket/pages/RawPage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@

public class RawPage extends SessionPage {

private final Logger logger = LoggerFactory.getLogger(getClass().getSimpleName());
private transient Logger logger;

String contentType;

Expand Down Expand Up @@ -95,7 +95,7 @@ public void respond(RequestCycle requestCycle) {
if (binary == null) {
final String objectNotFound = MessageFormat.format("Raw page failed to find object {0} in {1}",
objectId, repositoryName);
logger.error(objectNotFound);
logger().error(objectNotFound);
throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, objectNotFound);
}
contentType = "application/octet-stream";
Expand All @@ -104,15 +104,15 @@ public void respond(RequestCycle requestCycle) {
try {
response.getOutputStream().write(binary);
} catch (Exception e) {
logger.error("Failed to write binary response", e);
logger().error("Failed to write binary response", e);
}
} else {
// standard raw blob view
RevCommit commit = JGitUtils.getCommit(r, objectId);
if (commit == null) {
final String commitNotFound = MessageFormat.format("Raw page failed to find commit {0} in {1}",
objectId, repositoryName);
logger.error(commitNotFound);
logger().error(commitNotFound);
throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, commitNotFound);
}

Expand Down Expand Up @@ -148,7 +148,7 @@ public void respond(RequestCycle requestCycle) {
// image blobs
byte[] image = JGitUtils.getByteContent(r, commit.getTree(), blobPath, true);
if (image == null) {
logger.error(blobNotFound);
logger().error(blobNotFound);
throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound);
}
contentType = "image/" + extension.toLowerCase();
Expand All @@ -157,14 +157,14 @@ public void respond(RequestCycle requestCycle) {
try {
response.getOutputStream().write(image);
} catch (IOException e) {
logger.error("Failed to write image response", e);
logger().error("Failed to write image response", e);
}
break;
case 3:
// binary blobs (download)
byte[] binary = JGitUtils.getByteContent(r, commit.getTree(), blobPath, true);
if (binary == null) {
logger.error(blobNotFound);
logger().error(blobNotFound);
throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound);
}
contentType = "application/octet-stream";
Expand Down Expand Up @@ -193,23 +193,23 @@ public void respond(RequestCycle requestCycle) {
try {
response.getOutputStream().write(binary);
} catch (IOException e) {
logger.error("Failed to write binary response", e);
logger().error("Failed to write binary response", e);
}
break;
default:
// plain text
String content = JGitUtils.getStringContent(r, commit.getTree(),
blobPath, encodings);
if (content == null) {
logger.error(blobNotFound);
logger().error(blobNotFound);
throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound);
}
contentType = "text/plain; charset=UTF-8";
response.setContentType(contentType);
try {
response.getOutputStream().write(content.getBytes("UTF-8"));
} catch (Exception e) {
logger.error("Failed to write text response", e);
logger().error("Failed to write text response", e);
}
}

Expand All @@ -218,15 +218,15 @@ public void respond(RequestCycle requestCycle) {
String content = JGitUtils.getStringContent(r, commit.getTree(), blobPath,
encodings);
if (content == null) {
logger.error(blobNotFound);
logger().error(blobNotFound);
throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound);
}
contentType = "text/plain; charset=UTF-8";
response.setContentType(contentType);
try {
response.getOutputStream().write(content.getBytes("UTF-8"));
} catch (Exception e) {
logger.error("Failed to write text response", e);
logger().error("Failed to write text response", e);
}
}
}
Expand All @@ -235,6 +235,13 @@ public void respond(RequestCycle requestCycle) {
});
}

protected Logger logger() {
if (logger == null) {
logger = LoggerFactory.getLogger(getClass());
}
return logger;
}

@Override
protected void setHeaders(WebResponse response) {
super.setHeaders(response);
Expand Down
14 changes: 5 additions & 9 deletions src/main/java/com/gitblit/wicket/pages/RepositoryPage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.gitblit.Constants;
import com.gitblit.GitBlitException;
Expand Down Expand Up @@ -78,8 +76,6 @@

public abstract class RepositoryPage extends RootPage {

protected final Logger logger = LoggerFactory.getLogger(getClass());

private final String PARAM_STAR = "star";

protected final String projectName;
Expand All @@ -93,7 +89,7 @@ public abstract class RepositoryPage extends RootPage {
private Map<String, SubmoduleModel> submodules;

private boolean showAdmin;
private boolean isOwner;
private final boolean isOwner;

public RepositoryPage(PageParameters params) {
super(params);
Expand Down Expand Up @@ -144,7 +140,7 @@ public RepositoryPage(PageParameters params) {
try {
app().gitblit().reviseUser(user.username, user);
} catch (GitBlitException e) {
logger.error("Failed to update user " + user.username, e);
logger().error("Failed to update user " + user.username, e);
error(getString("gb.failedToUpdateUser"), false);
}
}
Expand Down Expand Up @@ -269,12 +265,12 @@ protected boolean allowForkControls() {

@Override
protected void setupPage(String repositoryName, String pageName) {

//This method should only be called once in the page lifecycle.
//However, it must be called after the constructor has run, hence not in onInitialize
//It may be attempted to be called again if an info or error message is displayed.
if (get("projectTitle") != null) { return; }

String projectName = StringUtils.getFirstPathElement(repositoryName);
ProjectModel project = app().projects().getProjectModel(projectName);

Expand Down Expand Up @@ -677,7 +673,7 @@ protected void onBeforeRender() {
r.close();
r = null;
}

// setup page header and footer
setupPage(getRepositoryName(), "/ " + getPageName());

Expand Down

0 comments on commit 53539dd

Please sign in to comment.