Skip to content

Commit

Permalink
lengthen the timeout in the TransactionIT to prevent premature reapin…
Browse files Browse the repository at this point in the history 
…g; use long instead of date for expiry time; ditch MultiExceptionMapper
  • Loading branch information
@barmintor
barmintor committed Nov 14, 2013
1 parent 4a26e39 commit 3840478
Show file tree
Hide file tree
Showing 19 changed files with 282 additions and 105 deletions.
3 changes: 2 additions & 1 deletion fcrepo-generator-dc/src/main/java/org/fcrepo/generator/DublinCoreGenerator.java
View file
Expand Up @@ -46,6 +46,7 @@
@Path("/{path: .*}/oai:dc")
public class DublinCoreGenerator extends AbstractResource {

private static String PNF_PREFIX = "No node found at path: ";
@Resource
List<DCGenerator> dcgenerators;

Expand Down Expand Up @@ -77,7 +78,7 @@ public Response getObjectAsDublinCore(
}
}
// no indexers = no path for DC
throw new PathNotFoundException();
throw new PathNotFoundException(PNF_PREFIX.concat(path));
} finally {
session.logout();
}
Expand Down
5 changes: 1 addition & 4 deletions fcrepo-http-api/src/main/java/org/fcrepo/http/api/FedoraNodes.java
View file
Expand Up @@ -167,7 +167,6 @@ public Dataset describe(@PathParam("path") final List<PathSegment> pathList,
}
final HttpGraphSubjects subjects =
new HttpGraphSubjects(session, FedoraNodes.class, uriInfo);
logger.info("1 session is {} live?: {}", session, session.isLive());

final int realLimit;
if (nonMemberProperties != null && limit == -1) {
Expand All @@ -178,7 +177,6 @@ public Dataset describe(@PathParam("path") final List<PathSegment> pathList,

final Dataset propertiesDataset =
resource.getPropertiesDataset(subjects, offset, realLimit);
logger.info("2 session is {} live?: {}", session, session.isLive());

final Model treeModel = propertiesDataset.getNamedModel(propertiesDataset.getContext().getAsString(INLINED_RESOURCES_MODEL, "NO SUCH MODEL"));
if (limit > 0 && treeModel != null && treeModel
Expand Down Expand Up @@ -214,7 +212,6 @@ public Dataset describe(@PathParam("path") final List<PathSegment> pathList,
requestModel.add(subjects.getContext(), FIRST_PAGE, firstPageResource);

propertiesDataset.addNamedModel("requestModel", requestModel);
logger.info("3 session is {} live?: {}", session, session.isLive());
}

if (!etag.getValue().isEmpty()) {
Expand All @@ -230,7 +227,7 @@ public Dataset describe(@PathParam("path") final List<PathSegment> pathList,
return propertiesDataset;

} finally {
logger.info("finally session is {} live?: {}", session, session.isLive());
session.logout();
}

}
Expand Down
5 changes: 3 additions & 2 deletions fcrepo-http-api/src/main/java/org/fcrepo/http/api/FedoraTransactions.java
View file
Expand Up @@ -23,6 +23,7 @@
import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
import static org.slf4j.LoggerFactory.getLogger;

import java.util.Date;
import java.util.List;

import javax.inject.Inject;
Expand Down Expand Up @@ -83,7 +84,7 @@ public Response createTransaction(@PathParam("path")
txService.getTransaction(((TxSession) session)
.getTxId());
t.updateExpiryDate();
return noContent().expires(t.getExpires()).build();
return noContent().expires(new Date(t.getExpires())).build();

} else {
final Transaction t = txService.beginTransaction(session);
Expand All @@ -96,7 +97,7 @@ public Response createTransaction(@PathParam("path")
.buildFromMap(
singletonMap("path", "tx:" +
t.getId()))).expires(
t.getExpires()).build();
new Date(t.getExpires())).build();
}
}

Expand Down
7 changes: 5 additions & 2 deletions fcrepo-http-api/src/test/java/org/fcrepo/integration/http/api/FedoraTransactionsIT.java
View file
Expand Up @@ -73,8 +73,11 @@ public void testRequestsInTransactionThatDoestExist() throws Exception {
public void testCreateAndTimeoutTransaction() throws Exception {

/* create a short-lived tx */
final long testTimeout = min(500, REAP_INTERVAL / 2);
System.setProperty(TIMEOUT_SYSTEM_PROPERTY, Long.toString(testTimeout));
/** this is a pretty fragile test, as it is largely dependent on
* how long it takes to spin up the test resources. Too short,
* and the TX will expire before the first GET
*/
System.setProperty(TIMEOUT_SYSTEM_PROPERTY, Long.toString(5 * REAP_INTERVAL));

/* create a tx */
final HttpPost createTx = new HttpPost(serverAddress + "fcr:tx");
Expand Down
18 changes: 8 additions & 10 deletions ...po-http-api/src/test/java/org/fcrepo/integration/http/api/html/FedoraHtmlResponsesIT.java
View file
Expand Up @@ -28,8 +28,7 @@
import org.fcrepo.integration.http.api.AbstractResourceIT;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
//import org.junit.Test;
import org.junit.Test;

import java.io.IOException;

Expand All @@ -38,7 +37,6 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

@Ignore
public class FedoraHtmlResponsesIT extends AbstractResourceIT {

private WebClient webClient;
Expand All @@ -58,15 +56,15 @@ public void cleanUp() {
javascriptlessWebClient.closeAllWindows();
}

@Ignore
@Test
public void testDescribeHtml() throws IOException {
final HtmlPage page = webClient.getPage(serverAddress);

checkForHeaderBranding(page);
checkForHeaderSearch(page);
}

@Ignore
@Test
public void testCreateNewNodeWithProvidedId() throws IOException, InterruptedException {

final String pid = randomUUID().toString();
Expand All @@ -82,7 +80,7 @@ public void testCreateNewNodeWithProvidedId() throws IOException, InterruptedExc
assertEquals(serverAddress + pid, page1.getTitleText());
}

@Ignore
@Test
public void testCreateNewNodeWithGeneratedId() throws IOException, InterruptedException {

final HtmlPage page = webClient.getPage(serverAddress);
Expand All @@ -95,7 +93,7 @@ public void testCreateNewNodeWithGeneratedId() throws IOException, InterruptedEx
assertTrue(page1.asText().length() > page.asText().length());
}

@Ignore
@Test
public void testCreateNewDatastream() throws IOException, InterruptedException {

final String pid = randomUUID().toString();
Expand All @@ -121,7 +119,7 @@ public void testCreateNewDatastream() throws IOException, InterruptedException {
assertEquals(serverAddress + pid, page1.getTitleText());
}

@Ignore
@Test
public void testCreateNewObjectAndDeleteIt() throws IOException {
final boolean throwExceptionOnFailingStatusCode = webClient.getOptions().isThrowExceptionOnFailingStatusCode();
webClient.getOptions().setThrowExceptionOnFailingStatusCode(false);
Expand All @@ -140,13 +138,13 @@ public void testCreateNewObjectAndDeleteIt() throws IOException {
webClient.getOptions().setThrowExceptionOnFailingStatusCode(throwExceptionOnFailingStatusCode);
}

@Ignore
@Test
public void testNodeTypes() throws IOException {
final HtmlPage page = webClient.getPage(serverAddress + "fcr:nodetypes");
assertTrue(page.asText().contains("fedora:object"));
}

@Ignore
@Test
public void testCreateNewObjectAndSetProperties() throws IOException {
final String pid = createNewObject();

Expand Down
7 changes: 2 additions & 5 deletions fcrepo-http-commons/src/main/java/org/fcrepo/http/FedoraApplication.java
View file
Expand Up @@ -19,14 +19,12 @@

import org.fcrepo.http.commons.exceptionhandlers.AccessControlExceptionMapper;
import org.fcrepo.http.commons.exceptionhandlers.InvalidChecksumExceptionMapper;
import org.fcrepo.http.commons.exceptionhandlers.MultiExceptionMapper;
import org.fcrepo.http.commons.exceptionhandlers.PathNotFoundExceptionMapper;
import org.fcrepo.http.commons.exceptionhandlers.RepositoryExceptionMapper;
import org.fcrepo.http.commons.exceptionhandlers.TransactionMissingExceptionMapper;
import org.fcrepo.http.commons.exceptionhandlers.WildcardExceptionMapper;
import org.fcrepo.http.commons.responses.RdfProvider;
import org.fcrepo.http.commons.session.AuthenticatedSessionProvider;
import org.fcrepo.kernel.services.TransactionService;
import org.glassfish.hk2.utilities.binding.AbstractBinder;
import org.glassfish.jersey.jackson.JacksonFeature;
import org.glassfish.jersey.media.multipart.MultiPartFeature;
Expand All @@ -53,16 +51,15 @@ public FedoraApplication() {
register(RepositoryExceptionMapper.class);
register(TransactionMissingExceptionMapper.class);
register(WildcardExceptionMapper.class);
register(MultiExceptionMapper.class);
}

static class FactoryBinder extends AbstractBinder {

@Override
protected void configure() {
bindFactory(AuthenticatedSessionProvider.class)
.to(Session.class)
.in(RequestScoped.class);
.to(Session.class)
.in(RequestScoped.class);
}
}
}
12 changes: 2 additions & 10 deletions fcrepo-http-commons/src/main/java/org/fcrepo/http/commons/AbstractResource.java
View file
Expand Up @@ -17,8 +17,8 @@
package org.fcrepo.http.commons;

import static com.hp.hpl.jena.rdf.model.ModelFactory.createDefaultModel;
import static javax.ws.rs.core.Response.notAcceptable;
import static javax.ws.rs.core.MediaType.APPLICATION_OCTET_STREAM_TYPE;
import static javax.ws.rs.core.Response.notAcceptable;
import static org.apache.jena.riot.WebContent.contentTypeSPARQLUpdate;
import static org.apache.jena.riot.WebContent.contentTypeToLang;
import static org.fcrepo.jcr.FedoraJcrTypes.FEDORA_DATASTREAM;
Expand All @@ -32,15 +32,12 @@
import javax.jcr.Node;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.PathSegment;
import javax.ws.rs.core.SecurityContext;
import javax.ws.rs.core.UriInfo;

import com.hp.hpl.jena.rdf.model.Model;
import org.apache.commons.io.IOUtils;
import org.apache.jena.riot.Lang;
import org.fcrepo.http.commons.api.rdf.HttpTripleUtil;
Expand All @@ -58,6 +55,7 @@

import com.google.common.eventbus.EventBus;
import com.hp.hpl.jena.query.Dataset;
import com.hp.hpl.jena.rdf.model.Model;

/**
* Abstract superclass for Fedora JAX-RS Resources, providing convenience fields
Expand Down Expand Up @@ -108,12 +106,6 @@ public abstract class AbstractResource {
@Autowired
protected PidMinter pidMinter;

@Context
private HttpServletRequest servletRequest;

@Context
private SecurityContext securityContext;

/**
* A convenience object provided by ModeShape for acting against the JCR
* repository.
Expand Down
2 changes: 1 addition & 1 deletion ...mons/src/main/java/org/fcrepo/http/commons/exceptionhandlers/WildcardExceptionMapper.java
View file
Expand Up @@ -26,7 +26,7 @@
import javax.ws.rs.ext.ExceptionMapper;
import javax.ws.rs.ext.Provider;

import org.fcrepo.kernel.exception.TransactionMissingException;
import org.fcrepo.http.commons.exceptions.TransactionMissingException;
import org.slf4j.Logger;

/**
Expand Down
22 changes: 19 additions & 3 deletions ...po-http-commons/src/main/java/org/fcrepo/http/commons/exceptions/RepositoryException.java
View file
@@ -1,15 +1,31 @@
/**
* Copyright 2013 DuraSpace, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.fcrepo.http.commons.exceptions;

import javax.ws.rs.WebApplicationException;


public class RepositoryException extends WebApplicationException {

/**
*
*/
private static final long serialVersionUID = -336224627233731847L;

/**
* Wrap an exception in a mappable container
*/
public RepositoryException(javax.jcr.RepositoryException e) {
super(e);
}
Expand Down
25 changes: 21 additions & 4 deletions ...commons/src/main/java/org/fcrepo/http/commons/exceptions/TransactionMissingException.java
View file
@@ -1,16 +1,33 @@
/**
* Copyright 2013 DuraSpace, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.fcrepo.http.commons.exceptions;

import javax.ws.rs.WebApplicationException;


public class TransactionMissingException extends WebApplicationException {

/**
*
*/
private static final long serialVersionUID = -336224627233731847L;

public TransactionMissingException(Exception e) {
/**
* Wrap an exception in a mappable container
*/
public TransactionMissingException(
org.fcrepo.kernel.exception.TransactionMissingException e) {
super(e);
}

Expand Down
4 changes: 2 additions & 2 deletions ...p-commons/src/main/java/org/fcrepo/http/commons/session/AuthenticatedSessionProvider.java
View file
Expand Up @@ -55,13 +55,13 @@ public AuthenticatedSessionProvider(final SessionFactory sessions,
@Override
public Session provide() {
Session result = sessions.getSession(securityContext, request);
System.err.println("SESSION PROVIDED " + result.toString());
return result;
}

@Override
public void dispose(Session instance) {
System.err.println("SESSION DISPOSED " + instance.toString());
// no-op until we can get this called
// it ought to be where we handle logout
}

}
6 changes: 2 additions & 4 deletions fcrepo-http-commons/src/main/java/org/fcrepo/http/commons/session/SessionFactory.java
View file
Expand Up @@ -152,7 +152,6 @@ public Session getSession(final SecurityContext securityContext,
final Transaction transaction =
getEmbeddedTransaction(servletRequest);

logger.warn("creds: {} tx: {}", creds, transaction);
final Session session;

if (transaction != null && creds != null) {
Expand All @@ -168,7 +167,7 @@ public Session getSession(final SecurityContext securityContext,
httpSession.getAttribute(TX_ATTRIBUTE))) {
session = transaction.getSession();
} else {
logger.warn("impersonating credentials");
logger.debug("impersonating credentials");
session = transaction.getSession().impersonate(creds);
}
} else if (creds != null) {
Expand All @@ -186,11 +185,10 @@ public Session getSession(final SecurityContext securityContext,
session = repo.login(creds);
}
} else {
logger.warn("Falling back on a unauthenticated session");
logger.info("Falling back on a unauthenticated session");
session = getSession(servletRequest);
}

logger.warn("session is {} live?: {}", session, session.isLive());
return session;
} catch (final org.fcrepo.kernel.exception.TransactionMissingException e) {
throw new TransactionMissingException(e);
Expand Down
6 changes: 3 additions & 3 deletions fcrepo-kernel/src/main/java/org/fcrepo/kernel/Transaction.java
View file
Expand Up @@ -74,7 +74,7 @@ public Transaction(final Session session) {
*/
public Session getSession() {
updateExpiryDate();
return TxAwareSession.newInstance(session, id);
return new TxAwareSession(session, id);
}

/**
Expand Down Expand Up @@ -110,8 +110,8 @@ public State getState() throws RepositoryException {
* garbage-collected
* @return
*/
public Date getExpires() {
return expires.getTime();
public long getExpires() {
return expires.getTimeInMillis();
}

/**
Expand Down

0 comments on commit 3840478

Please sign in to comment.