From e9c1328ecc11043edb782c10dae408f0aa06f374 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Fri, 30 Sep 2016 14:08:46 -0400 Subject: [PATCH 01/13] initial types for replacing jcr.Session in kernel-api --- .../fcrepo/kernel/api/FedoraRepository.java | 41 +++++ .../org/fcrepo/kernel/api/FedoraSession.java | 150 ++++++++++++++++++ .../kernel/api/services/BatchService.java | 121 ++++++++++++++ 3 files changed, 312 insertions(+) create mode 100644 fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraRepository.java create mode 100644 fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java create mode 100644 fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraRepository.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraRepository.java new file mode 100644 index 0000000000..2cc87be4c9 --- /dev/null +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraRepository.java @@ -0,0 +1,41 @@ +/* + * Licensed to DuraSpace under one or more contributor license agreements. + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * DuraSpace licenses this file to you 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.kernel.api; + +/** + * The basic abstraction for a Fedora repository + * @author acoburn + */ +public interface FedoraRepository { + + /** + * Login to the repository + * + * @return a FedoraSession + */ + FedoraSession login(); + + + /** + * Login to the repository with credentials + * + * @param credentials the credentials + * @return a FedoraSession + */ + FedoraSession login(final Object credentials); +} diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java new file mode 100644 index 0000000000..38fbcb42c3 --- /dev/null +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java @@ -0,0 +1,150 @@ +/* + * Licensed to DuraSpace under one or more contributor license agreements. + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * DuraSpace licenses this file to you 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.kernel.api; + +import static java.time.Duration.ofMinutes; +import static java.time.Duration.ofMillis; + +import java.time.Duration; +import java.time.Instant; +import java.util.Map; +import java.util.Optional; + +/** + * The Fedora Session abstraction + * + * @author acoburn + */ +public interface FedoraSession { + + // The default timeout is 3 minutes + public static final Duration DEFAULT_TIMEOUT = ofMinutes(3); + + public static final String TIMEOUT_SYSTEM_PROPERTY = "fcrepo.transactions.timeout"; + + public interface State {} + + public enum RequiredState implements State { + BATCH, EXPIRED, ACTIVE + } + + /** + * Begin a BATCH session + */ + void beginBatch(); + + /** + * Expire the session + */ + void expire(); + + /** + * Commit any batch operations + */ + void commit(); + + /** + * Rollback any batch operations + */ + void rollback(); + + /** + * Update the expiry by the provided amount + * @param amountToAdd the amount of time to add + * @return the new expiration date + */ + Instant updateExpiry(Duration amountToAdd); + + /** + * Determine whether the session has the provided state + * @param an enum that extends the default State enum + * @param state the state to test + * @return whether the session is in the given state + */ + boolean hasState(T state); + + /** + * Get the date this session was created + * @return creation date + */ + Instant getCreated(); + + /** + * Get the date this session expires + * @return expiration date + */ + Instant getExpires(); + + /** + * Get the session identifier + * @return the session id + */ + String getId(); + + /** + * Get the user identifier associated with this session + * @return the user id + */ + String getUserId(); + + /** + * Set a namespace prefix + * @param prefix the prefix + * @param uri the URI + */ + void setNamespacePrefix(String prefix, String uri); + + /** + * Get a namespace for a given prefix. + * @param prefix the prefix + * @return the namespace URI + */ + Optional getNamespace(String prefix); + + /** + * Get a mapping of registered namespaces + * @return the namespace mapping + */ + Map getNamespaces(); + + /** + * Set session-specific data + * @param key the key + * @param value the value + */ + void setSessionData(String key, String value); + + /** + * Retrieve the session data for a given key + * @param key the key + * @return the value + */ + Optional getSessionData(String key); + + /** + * Retrieve the default operation timeout value + * @return the default timeout value + */ + public static Duration operationTimeout() { + if (System.getProperty(TIMEOUT_SYSTEM_PROPERTY) != null) { + return ofMillis(Long.parseLong(System.getProperty(TIMEOUT_SYSTEM_PROPERTY))); + } else { + return DEFAULT_TIMEOUT; + } + } +} diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java new file mode 100644 index 0000000000..b42530f3a7 --- /dev/null +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java @@ -0,0 +1,121 @@ +/* + * Licensed to DuraSpace under one or more contributor license agreements. + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * DuraSpace licenses this file to you 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.kernel.api.services; + +import org.fcrepo.kernel.api.FedoraSession; + +/** + * @author acoburn + * @since Sept 30, 2016 + */ +public interface BatchService { + + /** + * Check for expired batch operations and remove them + */ + void removeAndRollbackExpired(); + + /** + * Create a new batch operation with a FedoraSession and add it to the currently open ones + * + * @param session The session to use for this batch operation + * @param username the name of the {@link java.security.Principal} + */ + void beginBatch(FedoraSession session, String username); + + /** + * Create a new FedoraSession for the anonymous user and add it to the currently open ones + * + * @param session The session to use for this batch operation + */ + default void beginBatch(FedoraSession session) { + beginBatch(session, null); + } + + /** + * Retrieve an open {@link FedoraSession} for a given user + * + * @param txId the Id of the {@link FedoraSession} + * @param username the name of the {@link java.security.Principal} + * @return the {@link FedoraSession} with this user + */ + FedoraSession getBatchSession(String txId, String username); + + /** + * Retrieve an open {@link FedoraSession} for an anonymous user + * + * @param txId the Id of the {@link FedoraSession} + * @return the {@link FedoraSession} + */ + default FedoraSession getBatchSession(String txId) { + return getBatchSession(txId, null); + } + + /** + * Check if a FedoraSession exists for a particular user + * + * @param txid the Id of the {@link FedoraSession} + * @param username the name of the {@link java.security.Principal} + * @return the {@link FedoraSession} object for the defined user + */ + boolean exists(String txid, String username); + + /** + * Check if a FedoraSession exists for the anonymous user + * + * @param txid the Id of the {@link FedoraSession} + * @return the {@link FedoraSession} object + */ + default boolean exists(String txid) { + return exists(txid, null); + } + + /** + * Commit any changes during a {@link FedoraSession} with the given id and username + * + * @param txid the id of the {@link FedoraSession} + * @param username the name of the {@link java.security.Principal} + */ + void commit(String txid, String username); + + /** + * Commit any changes during a {@link FedoraSession} with the given id for the anonymous user + * + * @param txid the id of the {@link FedoraSession} + */ + default void commit(String txid) { + commit(txid, null); + } + + /** + * Roll back any uncommited changes during a {@link FedoraSession} + * + * @param txid the id of the {@link FedoraSession} + * @param username the name of the {@link java.security.Principal} + */ + void rollback(String txid, String username); + + /** + * Roll back any uncommited changes during a {@link FedoraSession} for the anonymous user + * + * @param txid the id of the {@link FedoraSession} + */ + default void rollback(String txid) { + rollback(txid, null); + } +} From 023398c76b752605eb8e14be6dfad0c8eea0624a Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Fri, 30 Sep 2016 17:22:06 -0400 Subject: [PATCH 02/13] code review --- .../org/fcrepo/kernel/api/FedoraSession.java | 49 ++----------------- 1 file changed, 4 insertions(+), 45 deletions(-) diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java index 38fbcb42c3..8893061759 100644 --- a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java @@ -17,6 +17,7 @@ */ package org.fcrepo.kernel.api; +import static java.lang.Long.parseLong; import static java.time.Duration.ofMinutes; import static java.time.Duration.ofMillis; @@ -33,20 +34,9 @@ public interface FedoraSession { // The default timeout is 3 minutes - public static final Duration DEFAULT_TIMEOUT = ofMinutes(3); + public static final String DEFAULT_TIMEOUT = Long.toString(ofMinutes(3).toMillis()); - public static final String TIMEOUT_SYSTEM_PROPERTY = "fcrepo.transactions.timeout"; - - public interface State {} - - public enum RequiredState implements State { - BATCH, EXPIRED, ACTIVE - } - - /** - * Begin a BATCH session - */ - void beginBatch(); + public static final String TIMEOUT_SYSTEM_PROPERTY = "fcrepo.session.timeout"; /** * Expire the session @@ -58,11 +48,6 @@ public enum RequiredState implements State { */ void commit(); - /** - * Rollback any batch operations - */ - void rollback(); - /** * Update the expiry by the provided amount * @param amountToAdd the amount of time to add @@ -70,14 +55,6 @@ public enum RequiredState implements State { */ Instant updateExpiry(Duration amountToAdd); - /** - * Determine whether the session has the provided state - * @param an enum that extends the default State enum - * @param state the state to test - * @return whether the session is in the given state - */ - boolean hasState(T state); - /** * Get the date this session was created * @return creation date @@ -102,20 +79,6 @@ public enum RequiredState implements State { */ String getUserId(); - /** - * Set a namespace prefix - * @param prefix the prefix - * @param uri the URI - */ - void setNamespacePrefix(String prefix, String uri); - - /** - * Get a namespace for a given prefix. - * @param prefix the prefix - * @return the namespace URI - */ - Optional getNamespace(String prefix); - /** * Get a mapping of registered namespaces * @return the namespace mapping @@ -141,10 +104,6 @@ public enum RequiredState implements State { * @return the default timeout value */ public static Duration operationTimeout() { - if (System.getProperty(TIMEOUT_SYSTEM_PROPERTY) != null) { - return ofMillis(Long.parseLong(System.getProperty(TIMEOUT_SYSTEM_PROPERTY))); - } else { - return DEFAULT_TIMEOUT; - } + return ofMillis(parseLong(System.getProperty(TIMEOUT_SYSTEM_PROPERTY, DEFAULT_TIMEOUT))); } } From f0d8bffe2f1acf2b8c1ff82651d0bf4a3fc825c7 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Fri, 30 Sep 2016 17:35:14 -0400 Subject: [PATCH 03/13] missed some changes with earlier commit --- .../kernel/api/services/BatchService.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java index b42530f3a7..e308e65ea0 100644 --- a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java @@ -28,7 +28,7 @@ public interface BatchService { /** * Check for expired batch operations and remove them */ - void removeAndRollbackExpired(); + void removeExpired(); /** * Create a new batch operation with a FedoraSession and add it to the currently open ones @@ -36,15 +36,15 @@ public interface BatchService { * @param session The session to use for this batch operation * @param username the name of the {@link java.security.Principal} */ - void beginBatch(FedoraSession session, String username); + void begin(FedoraSession session, String username); /** * Create a new FedoraSession for the anonymous user and add it to the currently open ones * * @param session The session to use for this batch operation */ - default void beginBatch(FedoraSession session) { - beginBatch(session, null); + default void begin(FedoraSession session) { + begin(session, null); } /** @@ -54,7 +54,7 @@ default void beginBatch(FedoraSession session) { * @param username the name of the {@link java.security.Principal} * @return the {@link FedoraSession} with this user */ - FedoraSession getBatchSession(String txId, String username); + FedoraSession getSession(String txId, String username); /** * Retrieve an open {@link FedoraSession} for an anonymous user @@ -62,8 +62,8 @@ default void beginBatch(FedoraSession session) { * @param txId the Id of the {@link FedoraSession} * @return the {@link FedoraSession} */ - default FedoraSession getBatchSession(String txId) { - return getBatchSession(txId, null); + default FedoraSession getSession(String txId) { + return getSession(txId, null); } /** @@ -108,14 +108,14 @@ default void commit(String txid) { * @param txid the id of the {@link FedoraSession} * @param username the name of the {@link java.security.Principal} */ - void rollback(String txid, String username); + void abort(String txid, String username); /** * Roll back any uncommited changes during a {@link FedoraSession} for the anonymous user * * @param txid the id of the {@link FedoraSession} */ - default void rollback(String txid) { - rollback(txid, null); + default void abort(String txid) { + abort(txid, null); } } From 7410e8ba5874b0e06beaa2b956382f9a79d97746 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Fri, 30 Sep 2016 21:16:34 -0400 Subject: [PATCH 04/13] code review --- .../kernel/api/services/BatchService.java | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java index e308e65ea0..dc3795959b 100644 --- a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java @@ -50,72 +50,72 @@ default void begin(FedoraSession session) { /** * Retrieve an open {@link FedoraSession} for a given user * - * @param txId the Id of the {@link FedoraSession} + * @param sessionId the Id of the {@link FedoraSession} * @param username the name of the {@link java.security.Principal} * @return the {@link FedoraSession} with this user */ - FedoraSession getSession(String txId, String username); + FedoraSession getSession(String sessionId, String username); /** * Retrieve an open {@link FedoraSession} for an anonymous user * - * @param txId the Id of the {@link FedoraSession} + * @param sessionId the Id of the {@link FedoraSession} * @return the {@link FedoraSession} */ - default FedoraSession getSession(String txId) { - return getSession(txId, null); + default FedoraSession getSession(String sessionId) { + return getSession(sessionId, null); } /** * Check if a FedoraSession exists for a particular user * - * @param txid the Id of the {@link FedoraSession} + * @param sessionId the Id of the {@link FedoraSession} * @param username the name of the {@link java.security.Principal} * @return the {@link FedoraSession} object for the defined user */ - boolean exists(String txid, String username); + boolean exists(String sessionId, String username); /** * Check if a FedoraSession exists for the anonymous user * - * @param txid the Id of the {@link FedoraSession} + * @param sessionId the Id of the {@link FedoraSession} * @return the {@link FedoraSession} object */ - default boolean exists(String txid) { - return exists(txid, null); + default boolean exists(String sessionId) { + return exists(sessionId, null); } /** * Commit any changes during a {@link FedoraSession} with the given id and username * - * @param txid the id of the {@link FedoraSession} + * @param sessionId the id of the {@link FedoraSession} * @param username the name of the {@link java.security.Principal} */ - void commit(String txid, String username); + void commit(String sessionId, String username); /** * Commit any changes during a {@link FedoraSession} with the given id for the anonymous user * - * @param txid the id of the {@link FedoraSession} + * @param sessionId the id of the {@link FedoraSession} */ - default void commit(String txid) { - commit(txid, null); + default void commit(String sessionId) { + commit(sessionId, null); } /** * Roll back any uncommited changes during a {@link FedoraSession} * - * @param txid the id of the {@link FedoraSession} + * @param sessionId the id of the {@link FedoraSession} * @param username the name of the {@link java.security.Principal} */ - void abort(String txid, String username); + void abort(String sessionId, String username); /** * Roll back any uncommited changes during a {@link FedoraSession} for the anonymous user * - * @param txid the id of the {@link FedoraSession} + * @param sessionId the id of the {@link FedoraSession} */ - default void abort(String txid) { - abort(txid, null); + default void abort(String sessionId) { + abort(sessionId, null); } } From 2552b0d1c8fcdb5aeceecf4057086042d21af074 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Sun, 2 Oct 2016 15:51:41 -0400 Subject: [PATCH 05/13] add initial implementation --- .../org/fcrepo/kernel/api/FedoraSession.java | 17 -- .../kernel/api/services/BatchService.java | 17 ++ .../modeshape/FedoraRepositoryImpl.java | 87 ++++++++ .../kernel/modeshape/FedoraSessionImpl.java | 172 +++++++++++++++ .../modeshape/services/BatchServiceImpl.java | 135 ++++++++++++ .../services/BatchServiceImplTest.java | 198 ++++++++++++++++++ 6 files changed, 609 insertions(+), 17 deletions(-) create mode 100644 fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java create mode 100644 fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java create mode 100644 fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java create mode 100644 fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java index 8893061759..2fe4f8fed5 100644 --- a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java @@ -17,10 +17,6 @@ */ package org.fcrepo.kernel.api; -import static java.lang.Long.parseLong; -import static java.time.Duration.ofMinutes; -import static java.time.Duration.ofMillis; - import java.time.Duration; import java.time.Instant; import java.util.Map; @@ -33,11 +29,6 @@ */ public interface FedoraSession { - // The default timeout is 3 minutes - public static final String DEFAULT_TIMEOUT = Long.toString(ofMinutes(3).toMillis()); - - public static final String TIMEOUT_SYSTEM_PROPERTY = "fcrepo.session.timeout"; - /** * Expire the session */ @@ -98,12 +89,4 @@ public interface FedoraSession { * @return the value */ Optional getSessionData(String key); - - /** - * Retrieve the default operation timeout value - * @return the default timeout value - */ - public static Duration operationTimeout() { - return ofMillis(parseLong(System.getProperty(TIMEOUT_SYSTEM_PROPERTY, DEFAULT_TIMEOUT))); - } } diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java index dc3795959b..de1f91d15c 100644 --- a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java @@ -85,6 +85,23 @@ default boolean exists(String sessionId) { return exists(sessionId, null); } + /** + * Refresh an existing session using an implementation-defined default + * + * @param sessionId the Id of the {@link FedoraSession} + * @param username the name of the {@link java.security.Principal} + */ + void refresh(String sessionId, String username); + + /** + * Refresh an existing anonymous session using an implementation-defined default + * + * @param sessionId the Id of the {@link FedoraSession} + */ + default void refresh(String sessionId) { + refresh(sessionId, null); + } + /** * Commit any changes during a {@link FedoraSession} with the given id and username * diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java new file mode 100644 index 0000000000..8ef31660b0 --- /dev/null +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java @@ -0,0 +1,87 @@ +/* + * Licensed to DuraSpace under one or more contributor license agreements. + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * DuraSpace licenses this file to you 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.kernel.modeshape; + +import javax.jcr.Credentials; +import javax.jcr.Repository; +import javax.jcr.RepositoryException; + +import org.fcrepo.kernel.api.FedoraRepository; +import org.fcrepo.kernel.api.FedoraSession; +import org.fcrepo.kernel.api.exception.RepositoryRuntimeException; + +/** + * The basic abstraction for a Fedora repository + * @author acoburn + */ +public class FedoraRepositoryImpl implements FedoraRepository { + + private final Repository repository; + + /** + * Create a FedoraRepositoryImpl with a JCR-based Repository + * + * @param repository the JCR repository + */ + public FedoraRepositoryImpl(final Repository repository) { + this.repository = repository; + } + + @Override + public FedoraSession login() { + try { + return new FedoraSessionImpl(repository.login()); + } catch (final RepositoryException ex) { + throw new RepositoryRuntimeException(ex); + } + } + + @Override + public FedoraSession login(final Object credentials) { + if (credentials instanceof Credentials) { + try { + return new FedoraSessionImpl(repository.login((Credentials) credentials)); + } catch (final RepositoryException ex) { + throw new RepositoryRuntimeException(ex); + } + } + throw new IllegalArgumentException("Credentials are of the wrong type"); + } + + /** + * Retrieve the internal JCR Repository object + * + * @return the JCR Repository + */ + public Repository getRepository() { + return repository; + } + + /** + * Retrieve the internal JCR Repository from a FedoraRepository object + * + * @param repository the FedoraRepository + * @return the JCR Repository + */ + public static Repository getJcrRepository(final FedoraRepository repository) { + if (repository instanceof FedoraRepositoryImpl) { + return ((FedoraRepositoryImpl)repository).getRepository(); + } + throw new IllegalArgumentException("FedoraRepository is of the wrong type"); + } +} diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java new file mode 100644 index 0000000000..9d460908d3 --- /dev/null +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java @@ -0,0 +1,172 @@ +/* + * Licensed to DuraSpace under one or more contributor license agreements. + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * DuraSpace licenses this file to you 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.kernel.modeshape; + +import static java.lang.Long.parseLong; +import static java.time.Duration.ofMillis; +import static java.time.Duration.ofMinutes; +import static java.time.Instant.now; +import static java.util.Optional.ofNullable; +import static java.util.UUID.randomUUID; + +import java.time.Duration; +import java.time.Instant; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +import javax.jcr.RepositoryException; +import javax.jcr.Session; + +import org.fcrepo.kernel.api.FedoraSession; +import org.fcrepo.kernel.api.exception.AccessDeniedException; +import org.fcrepo.kernel.api.exception.RepositoryRuntimeException; +import org.fcrepo.kernel.modeshape.utils.NamespaceTools; + +/** + * An implementation of the FedoraSession abstraction + * @author acoburn + */ +public class FedoraSessionImpl implements FedoraSession { + + // The default timeout is 3 minutes + public static final String DEFAULT_TIMEOUT = Long.toString(ofMinutes(3).toMillis()); + + public static final String TIMEOUT_SYSTEM_PROPERTY = "fcrepo.session.timeout"; + + private final Session session; + private final String id; + private final Instant created; + private final Map sessionData; + private Instant expires; + + /** + * A key for looking up the transaction id in a session key-value pair + */ + public static final String FCREPO_TX_ID = "fcrepo.tx.id"; + + /** + * Create a Fedora session with a JCR session and a username + * @param session the JCR session + */ + public FedoraSessionImpl(final Session session) { + this.session = session; + + created = now(); + id = randomUUID().toString(); + expires = created.plus(operationTimeout()); + sessionData = new HashMap<>(); + } + + @Override + public void commit() { + try { + if (session.isLive()) { + session.save(); + } + } catch (final javax.jcr.AccessDeniedException ex) { + throw new AccessDeniedException(ex); + } catch (final RepositoryException ex) { + throw new RepositoryRuntimeException(ex); + } + } + + @Override + public void expire() { + expires = now(); + try { + if (session.isLive()) { + session.refresh(false); + session.logout(); + } + } catch (final RepositoryException ex) { + throw new RepositoryRuntimeException(ex); + } + } + + @Override + public Instant updateExpiry(final Duration amountToAdd) { + if (session.isLive()) { + expires = now().plus(amountToAdd); + } + return expires; + } + + @Override + public Instant getCreated() { + return created; + } + + @Override + public Instant getExpires() { + return expires; + } + + @Override + public String getId() { + return id; + } + + @Override + public String getUserId() { + return session.getUserID(); + } + + @Override + public Map getNamespaces() { + return NamespaceTools.getNamespaces(session); + } + + @Override + public void setSessionData(final String key, final String value) { + sessionData.put(key, value); + } + + @Override + public Optional getSessionData(final String key) { + return ofNullable(sessionData.get(key)); + } + + /** + * Get the internal JCR session + * @return the internal JCR session + */ + public Session getSession() { + return session; + } + + /** + * Get the internal JCR session from an existing FedoraSession + * @param session the FedoraSession + * @return the JCR session + */ + public static Session getJcrSession(final FedoraSession session) { + if (session instanceof FedoraSessionImpl) { + return ((FedoraSessionImpl)session).getSession(); + } + throw new IllegalArgumentException("FedoraSession is of the wrong type"); + } + + /** + * Retrieve the default operation timeout value + * @return the default timeout value + */ + public static Duration operationTimeout() { + return ofMillis(parseLong(System.getProperty(TIMEOUT_SYSTEM_PROPERTY, DEFAULT_TIMEOUT))); + } +} diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java new file mode 100644 index 0000000000..63edd71e51 --- /dev/null +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java @@ -0,0 +1,135 @@ +/* + * Licensed to DuraSpace under one or more contributor license agreements. + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * DuraSpace licenses this file to you 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.kernel.modeshape.services; + +import static java.time.Instant.now; +import static java.util.stream.Collectors.toSet; +import static org.fcrepo.kernel.modeshape.FedoraSessionImpl.operationTimeout; +import static org.slf4j.LoggerFactory.getLogger; + +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import org.fcrepo.kernel.api.FedoraSession; +import org.fcrepo.kernel.api.exception.RepositoryRuntimeException; +import org.fcrepo.kernel.api.exception.SessionMissingException; +import org.fcrepo.kernel.api.services.BatchService; + +import org.slf4j.Logger; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Component; + +/** + * This is part of the strawman implementation for Fedora batch operations This + * service implements a simple {@link FedoraSession} service which is able to + * create/commit/rollback {@link FedoraSession} objects. A {@link Scheduled} + * annotation is used for removing timed out operations + * + * @author frank asseg + * @author ajs6f + * @author acoburn + */ +@Component +public class BatchServiceImpl extends AbstractService implements BatchService { + + private static final Logger LOGGER = getLogger(BatchServiceImpl.class); + + /** + * TODO since sessions have to be available on all nodes, they have to + * be either persisted or written to a distributed map or sth, not just this + * plain hashmap that follows + */ + private static Map transactions = new ConcurrentHashMap<>(); + + public static final long REAP_INTERVAL = 1000; + + /** + * Every REAP_INTERVAL milliseconds, check for expired sessions. If the + * tx is expired, roll it back and remove it from the registry. + */ + @Override + @Scheduled(fixedRate = REAP_INTERVAL) + public void removeExpired() { + final Set reapable = transactions.entrySet().stream() + .filter(e -> e.getValue().getExpires().isBefore(now())).map(Map.Entry::getKey).collect(toSet()); + reapable.forEach(key -> { + final FedoraSession s = transactions.get(key); + if (s != null) { + try { + s.expire(); + } catch (final RepositoryRuntimeException e) { + LOGGER.error("Got exception rolling back expired session {}: {}", s, e.getMessage()); + } + } + transactions.remove(key); + }); + } + + @Override + public void begin(final FedoraSession session, final String username) { + transactions.put(getTxKey(session.getId(), username), session); + } + + @Override + public FedoraSession getSession(final String sessionId, final String username) { + final FedoraSession tx = transactions.computeIfAbsent(getTxKey(sessionId, username), s -> { + throw new SessionMissingException("Batch session with id: " + s + " is not available"); + }); + return tx; + } + + @Override + public boolean exists(final String sessionId, final String username) { + return transactions.containsKey(getTxKey(sessionId, username)); + } + + @Override + public void commit(final String sessionId, final String username) { + final FedoraSession session = transactions.remove(getTxKey(sessionId, username)); + if (session == null) { + throw new SessionMissingException("Batch session with id " + sessionId + + " is not available"); + } + session.commit(); + } + + @Override + public void refresh(final String sessionId, final String username) { + final FedoraSession session = getSession(sessionId, username); + session.updateExpiry(operationTimeout()); + } + + @Override + public void abort(final String sessionId, final String username) { + final FedoraSession tx = transactions.remove(getTxKey(sessionId, username)); + if (tx == null) { + throw new SessionMissingException("Batch session with id " + sessionId + + " is not available for " + username); + } + tx.expire(); + } + + private static String getTxKey(final String sessionId, final String username) { + return username == null ? sessionId : username + ":" + sessionId; + } +} diff --git a/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java b/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java new file mode 100644 index 0000000000..c35beae0c7 --- /dev/null +++ b/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java @@ -0,0 +1,198 @@ +/* + * Licensed to DuraSpace under one or more contributor license agreements. + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * DuraSpace licenses this file to you 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.kernel.modeshape.services; + +import static java.time.Instant.now; +import static org.fcrepo.kernel.modeshape.FedoraSessionImpl.FCREPO_TX_ID; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Field; +import java.time.Instant; +import java.util.Map; + +import javax.jcr.NamespaceException; +import javax.jcr.RepositoryException; +import javax.jcr.Session; + +import org.fcrepo.kernel.api.FedoraSession; +import org.fcrepo.kernel.api.exception.RepositoryRuntimeException; +import org.fcrepo.kernel.api.exception.SessionMissingException; +import org.fcrepo.kernel.api.services.BatchService; +import org.fcrepo.kernel.modeshape.FedoraSessionImpl; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +/** + * @author frank asseg + * @author ajs6f + */ +@RunWith(MockitoJUnitRunner.class) +public class BatchServiceImplTest { + + private static final String IS_A_TX = "foo"; + + private static final String NOT_A_TX = "bar"; + + private static final String USER_NAME = "test"; + + private static final String ANOTHER_USER_NAME = "another"; + + BatchService service; + + @Mock + private FedoraSession mockTx; + + @Mock + private Session mockSession; + + private FedoraSession fedoraSession; + + @Before + public void setup() throws Exception { + fedoraSession = new FedoraSessionImpl(mockSession); + service = new BatchServiceImpl(); + when(mockTx.getId()).thenReturn(IS_A_TX); + when(mockTx.getUserId()).thenReturn(null); + final Field txsField = + BatchServiceImpl.class.getDeclaredField("transactions"); + txsField.setAccessible(true); + @SuppressWarnings("unchecked") + final Map txs = + (Map) txsField + .get(BatchService.class); + txs.put(IS_A_TX, mockTx); + } + + @Test + public void testExpiration() { + final Instant fiveSecondsAgo = now().minusSeconds(5); + when(mockTx.getExpires()).thenReturn(fiveSecondsAgo); + service.removeExpired(); + verify(mockTx).expire(); + } + + @Test + public void testExpirationThrowsRepositoryException() { + final Instant fiveSecondsAgo = now().minusSeconds(5); + doThrow(new RepositoryRuntimeException("")).when(mockTx).expire(); + when(mockTx.getExpires()).thenReturn(fiveSecondsAgo); + service.removeExpired(); + } + + @Test + public void testCreateTx() { + service.begin(fedoraSession); + assertTrue(service.exists(fedoraSession.getId())); + assertTrue(service.exists(fedoraSession.getId(), null)); + assertEquals(service.getSession(fedoraSession.getId()).getId(), fedoraSession.getId()); + } + + @Test + public void testGetTx() { + final FedoraSession tx = service.getSession(IS_A_TX, null); + assertNotNull(tx); + } + + @Test(expected = SessionMissingException.class) + public void testHijackingNotPossible() { + service.begin(fedoraSession); + service.getSession(fedoraSession.getId(), ANOTHER_USER_NAME); + } + + @Test(expected = SessionMissingException.class) + public void testHijackingNotPossibleWithAnonUser() { + service.begin(fedoraSession, USER_NAME); + service.getSession(fedoraSession.getId(), null); + } + + @Test(expected = SessionMissingException.class) + public void testHijackingNotPossibleWhenStartedAnonUser() { + when(mockSession.getUserID()).thenReturn(USER_NAME); + service.begin(fedoraSession); + service.getSession(fedoraSession.getId(), ANOTHER_USER_NAME); + } + + @Test(expected = SessionMissingException.class) + public void testGetNonTx() throws SessionMissingException { + service.getSession(NOT_A_TX, null); + } + + @Test + public void testGetTxForSession() throws Exception { + when(mockSession.getNamespaceURI(FCREPO_TX_ID)).thenReturn(IS_A_TX); + when(mockTx.getId()).thenReturn(IS_A_TX); + final FedoraSession tx = service.getSession(mockTx.getId()); + assertEquals(IS_A_TX, tx.getId()); + } + + @Test(expected = SessionMissingException.class) + public void testGetTxForNonTxSession() throws RepositoryException { + when(mockSession.getNamespaceURI(FCREPO_TX_ID)).thenThrow(new NamespaceException("")); + service.getSession(fedoraSession.getId()); + } + + @Test + public void testExists() { + assertTrue(service.exists(IS_A_TX)); + assertFalse(service.exists(NOT_A_TX)); + } + + @Test + public void testCommitTx() { + service.commit(IS_A_TX); + verify(mockTx).commit(); + } + + @Test(expected = SessionMissingException.class) + public void testCommitRemovedSession() { + service.commit(IS_A_TX); + service.getSession(fedoraSession.getId(), null); + } + + @Test + public void testAbortTx() { + service.abort(IS_A_TX); + verify(mockTx).expire(); + } + + @Test(expected = SessionMissingException.class) + public void testAbortRemovedSession() { + service.abort(IS_A_TX); + service.getSession(IS_A_TX, null); + } + + @Test(expected = SessionMissingException.class) + public void testAbortWithNonTx() { + service.abort(NOT_A_TX); + } + + @Test(expected = SessionMissingException.class) + public void testCommitWithNonTx() { + service.commit(NOT_A_TX); + } +} From 629b0de5a5f936506781110965c1345184047117 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Mon, 3 Oct 2016 09:37:43 -0400 Subject: [PATCH 06/13] rename methods --- .../org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java | 4 ++-- .../java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java index 8ef31660b0..9ffd390e83 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java @@ -68,7 +68,7 @@ public FedoraSession login(final Object credentials) { * * @return the JCR Repository */ - public Repository getRepository() { + public Repository getJcrRepository() { return repository; } @@ -80,7 +80,7 @@ public Repository getRepository() { */ public static Repository getJcrRepository(final FedoraRepository repository) { if (repository instanceof FedoraRepositoryImpl) { - return ((FedoraRepositoryImpl)repository).getRepository(); + return ((FedoraRepositoryImpl)repository).getJcrRepository(); } throw new IllegalArgumentException("FedoraRepository is of the wrong type"); } diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java index 9d460908d3..6903c21576 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java @@ -146,7 +146,7 @@ public Optional getSessionData(final String key) { * Get the internal JCR session * @return the internal JCR session */ - public Session getSession() { + public Session getJcrSession() { return session; } @@ -157,7 +157,7 @@ public Session getSession() { */ public static Session getJcrSession(final FedoraSession session) { if (session instanceof FedoraSessionImpl) { - return ((FedoraSessionImpl)session).getSession(); + return ((FedoraSessionImpl)session).getJcrSession(); } throw new IllegalArgumentException("FedoraSession is of the wrong type"); } From bdec501c3658f36bb002caef29f6f60548532d54 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Mon, 3 Oct 2016 10:21:11 -0400 Subject: [PATCH 07/13] add support for event-based data --- .../fcrepo/kernel/modeshape/FedoraSessionImpl.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java index 6903c21576..16bf6f8984 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java @@ -32,7 +32,11 @@ import javax.jcr.RepositoryException; import javax.jcr.Session; +import javax.jcr.observation.ObservationManager; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; import org.fcrepo.kernel.api.FedoraSession; import org.fcrepo.kernel.api.exception.AccessDeniedException; import org.fcrepo.kernel.api.exception.RepositoryRuntimeException; @@ -60,6 +64,8 @@ public class FedoraSessionImpl implements FedoraSession { */ public static final String FCREPO_TX_ID = "fcrepo.tx.id"; + private static final ObjectMapper mapper = new ObjectMapper(); + /** * Create a Fedora session with a JCR session and a username * @param session the JCR session @@ -77,11 +83,15 @@ public FedoraSessionImpl(final Session session) { public void commit() { try { if (session.isLive()) { + final ObservationManager obs = session.getWorkspace().getObservationManager(); + final ObjectNode json = mapper.createObjectNode(); + sessionData.forEach(json::put); + obs.setUserData(mapper.writeValueAsString(json)); session.save(); } } catch (final javax.jcr.AccessDeniedException ex) { throw new AccessDeniedException(ex); - } catch (final RepositoryException ex) { + } catch (final RepositoryException | JsonProcessingException ex) { throw new RepositoryRuntimeException(ex); } } From 8e82574d941be7e1a9ac9c2306fc8b2883e9da67 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Wed, 5 Oct 2016 11:52:27 -0400 Subject: [PATCH 08/13] Code review --- .../org/fcrepo/kernel/api/FedoraSession.java | 26 +++++++-- .../modeshape/FedoraRepositoryImpl.java | 2 +- .../kernel/modeshape/FedoraSessionImpl.java | 54 +++++++++++-------- .../modeshape/services/BatchServiceImpl.java | 30 +++++------ .../services/BatchServiceImplTest.java | 2 +- 5 files changed, 71 insertions(+), 43 deletions(-) diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java index 2fe4f8fed5..0a6c5519af 100644 --- a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java @@ -19,13 +19,16 @@ import java.time.Duration; import java.time.Instant; +import java.util.Collection; import java.util.Map; -import java.util.Optional; /** * The Fedora Session abstraction * * @author acoburn + * + * Note: This interface does not make any guarantees about thread-safety. Any + * synchronization behavior is left to the discretion of the implementation. */ public interface FedoraSession { @@ -77,16 +80,31 @@ public interface FedoraSession { Map getNamespaces(); /** - * Set session-specific data + * Add session-specific data * @param key the key * @param value the value */ - void setSessionData(String key, String value); + void addSessionData(String key, String value); /** * Retrieve the session data for a given key * @param key the key * @return the value */ - Optional getSessionData(String key); + Collection getSessionData(String key); + + /** + * Remove a particular session key-value pair + * @param key the data key + * @param value the data value + */ + void removeSessionData(String key, String value); + + /** + * Remove all session data for a particular key + * @param key the data key + */ + default void removeSessionData(String key) { + getSessionData(key).forEach(v -> removeSessionData(key, v)); + } } diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java index 9ffd390e83..19fcd71c81 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java @@ -82,6 +82,6 @@ public static Repository getJcrRepository(final FedoraRepository repository) { if (repository instanceof FedoraRepositoryImpl) { return ((FedoraRepositoryImpl)repository).getJcrRepository(); } - throw new IllegalArgumentException("FedoraRepository is of the wrong type"); + throw new ClassCastException("FedoraRepository is not a " + FedoraRepositoryImpl.class.getCanonicalName()); } } diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java index 16bf6f8984..064e05a50d 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java @@ -21,14 +21,14 @@ import static java.time.Duration.ofMillis; import static java.time.Duration.ofMinutes; import static java.time.Instant.now; -import static java.util.Optional.ofNullable; +import static java.util.Collections.singleton; import static java.util.UUID.randomUUID; import java.time.Duration; import java.time.Instant; -import java.util.HashMap; +import java.util.Collection; import java.util.Map; -import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import javax.jcr.RepositoryException; import javax.jcr.Session; @@ -45,6 +45,8 @@ /** * An implementation of the FedoraSession abstraction * @author acoburn + * + * Note: This class is not guaranteed to be thread-safe. */ public class FedoraSessionImpl implements FedoraSession { @@ -53,10 +55,10 @@ public class FedoraSessionImpl implements FedoraSession { public static final String TIMEOUT_SYSTEM_PROPERTY = "fcrepo.session.timeout"; - private final Session session; + private final Session jcrSession; private final String id; private final Instant created; - private final Map sessionData; + private final ConcurrentHashMap sessionData; private Instant expires; /** @@ -71,23 +73,23 @@ public class FedoraSessionImpl implements FedoraSession { * @param session the JCR session */ public FedoraSessionImpl(final Session session) { - this.session = session; + this.jcrSession = session; created = now(); id = randomUUID().toString(); expires = created.plus(operationTimeout()); - sessionData = new HashMap<>(); + sessionData = new ConcurrentHashMap<>(); } @Override public void commit() { try { - if (session.isLive()) { - final ObservationManager obs = session.getWorkspace().getObservationManager(); + if (jcrSession.isLive()) { + final ObservationManager obs = jcrSession.getWorkspace().getObservationManager(); final ObjectNode json = mapper.createObjectNode(); sessionData.forEach(json::put); obs.setUserData(mapper.writeValueAsString(json)); - session.save(); + jcrSession.save(); } } catch (final javax.jcr.AccessDeniedException ex) { throw new AccessDeniedException(ex); @@ -100,9 +102,9 @@ public void commit() { public void expire() { expires = now(); try { - if (session.isLive()) { - session.refresh(false); - session.logout(); + if (jcrSession.isLive()) { + jcrSession.refresh(false); + jcrSession.logout(); } } catch (final RepositoryException ex) { throw new RepositoryRuntimeException(ex); @@ -111,7 +113,7 @@ public void expire() { @Override public Instant updateExpiry(final Duration amountToAdd) { - if (session.isLive()) { + if (jcrSession.isLive()) { expires = now().plus(amountToAdd); } return expires; @@ -134,22 +136,32 @@ public String getId() { @Override public String getUserId() { - return session.getUserID(); + return jcrSession.getUserID(); } @Override public Map getNamespaces() { - return NamespaceTools.getNamespaces(session); + return NamespaceTools.getNamespaces(jcrSession); } @Override - public void setSessionData(final String key, final String value) { + public void addSessionData(final String key, final String value) { sessionData.put(key, value); } @Override - public Optional getSessionData(final String key) { - return ofNullable(sessionData.get(key)); + public Collection getSessionData(final String key) { + return singleton(sessionData.get(key)); + } + + @Override + public void removeSessionData(final String key, final String value) { + sessionData.remove(key, value); + } + + @Override + public void removeSessionData(final String key) { + sessionData.remove(key); } /** @@ -157,7 +169,7 @@ public Optional getSessionData(final String key) { * @return the internal JCR session */ public Session getJcrSession() { - return session; + return jcrSession; } /** @@ -169,7 +181,7 @@ public static Session getJcrSession(final FedoraSession session) { if (session instanceof FedoraSessionImpl) { return ((FedoraSessionImpl)session).getJcrSession(); } - throw new IllegalArgumentException("FedoraSession is of the wrong type"); + throw new ClassCastException("FedoraSession is not a " + FedoraSessionImpl.class.getCanonicalName()); } /** diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java index 63edd71e51..e5f23e8987 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java @@ -23,6 +23,7 @@ import static java.time.Instant.now; import static java.util.stream.Collectors.toSet; +import static com.google.common.base.Strings.nullToEmpty; import static org.fcrepo.kernel.modeshape.FedoraSessionImpl.operationTimeout; import static org.slf4j.LoggerFactory.getLogger; @@ -92,10 +93,13 @@ public void begin(final FedoraSession session, final String username) { @Override public FedoraSession getSession(final String sessionId, final String username) { - final FedoraSession tx = transactions.computeIfAbsent(getTxKey(sessionId, username), s -> { - throw new SessionMissingException("Batch session with id: " + s + " is not available"); - }); - return tx; + System.out.println(transactions.keySet().toString()); + System.out.println(getTxKey(sessionId, username)); + final FedoraSession session = transactions.get(getTxKey(sessionId, username)); + if (session == null) { + throw new SessionMissingException("Batch session with id: " + sessionId + " is not available"); + } + return session; } @Override @@ -105,12 +109,9 @@ public boolean exists(final String sessionId, final String username) { @Override public void commit(final String sessionId, final String username) { - final FedoraSession session = transactions.remove(getTxKey(sessionId, username)); - if (session == null) { - throw new SessionMissingException("Batch session with id " + sessionId + - " is not available"); - } + final FedoraSession session = getSession(sessionId, username); session.commit(); + transactions.remove(getTxKey(sessionId, username)); } @Override @@ -121,15 +122,12 @@ public void refresh(final String sessionId, final String username) { @Override public void abort(final String sessionId, final String username) { - final FedoraSession tx = transactions.remove(getTxKey(sessionId, username)); - if (tx == null) { - throw new SessionMissingException("Batch session with id " + sessionId + - " is not available for " + username); - } - tx.expire(); + final FedoraSession session = getSession(sessionId, username); + session.expire(); + transactions.remove(getTxKey(sessionId, username)); } private static String getTxKey(final String sessionId, final String username) { - return username == null ? sessionId : username + ":" + sessionId; + return nullToEmpty(username) + ":" + sessionId; } } diff --git a/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java b/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java index c35beae0c7..cca8332694 100644 --- a/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java +++ b/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java @@ -85,7 +85,7 @@ public void setup() throws Exception { final Map txs = (Map) txsField .get(BatchService.class); - txs.put(IS_A_TX, mockTx); + txs.put(":" + IS_A_TX, mockTx); } @Test From 4fb0626a7e7d5afd6fd74e2bc3bc9d31ebe497a3 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Wed, 5 Oct 2016 15:24:30 -0400 Subject: [PATCH 09/13] more code review --- .../java/org/fcrepo/kernel/api/FedoraSession.java | 3 +++ .../fcrepo/kernel/modeshape/FedoraSessionImpl.java | 11 +++++++++++ .../kernel/modeshape/services/BatchServiceImpl.java | 2 -- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java index 0a6c5519af..8aef548b83 100644 --- a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java @@ -83,6 +83,9 @@ public interface FedoraSession { * Add session-specific data * @param key the key * @param value the value + * + * Note: it is up to the particular implementation as to whether multi-valued session data + * is allowed. */ void addSessionData(String key, String value); diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java index 064e05a50d..53e1aaf000 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java @@ -144,6 +144,17 @@ public Map getNamespaces() { return NamespaceTools.getNamespaces(jcrSession); } + /** + * Add session data + * @param key the data key + * @param value the data value + * + * Note: while the FedoraSession inteface permits multi-valued + * session data, this implementation constrains that to be single-valued. + * That is, calling obj.addSessionData("key", "value1") followed by + * obj.addSessionData("key", "value2") will result in only "value2" being assciated + * with the given key. + */ @Override public void addSessionData(final String key, final String value) { sessionData.put(key, value); diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java index e5f23e8987..c106132bd6 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java @@ -93,8 +93,6 @@ public void begin(final FedoraSession session, final String username) { @Override public FedoraSession getSession(final String sessionId, final String username) { - System.out.println(transactions.keySet().toString()); - System.out.println(getTxKey(sessionId, username)); final FedoraSession session = transactions.get(getTxKey(sessionId, username)); if (session == null) { throw new SessionMissingException("Batch session with id: " + sessionId + " is not available"); From b323e343444b4e5e1c113f68d051923b90e7d191 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Wed, 5 Oct 2016 15:42:44 -0400 Subject: [PATCH 10/13] remove thread-safety comments --- .../src/main/java/org/fcrepo/kernel/api/FedoraSession.java | 3 --- .../java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java | 2 -- 2 files changed, 5 deletions(-) diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java index 8aef548b83..8b51c0d801 100644 --- a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java @@ -26,9 +26,6 @@ * The Fedora Session abstraction * * @author acoburn - * - * Note: This interface does not make any guarantees about thread-safety. Any - * synchronization behavior is left to the discretion of the implementation. */ public interface FedoraSession { diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java index 53e1aaf000..bca9545b4b 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java @@ -45,8 +45,6 @@ /** * An implementation of the FedoraSession abstraction * @author acoburn - * - * Note: This class is not guaranteed to be thread-safe. */ public class FedoraSessionImpl implements FedoraSession { From 1911fe3168aac482f2f169b0727eb1076cd24f58 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Wed, 5 Oct 2016 16:43:34 -0400 Subject: [PATCH 11/13] more code review --- .../src/main/java/org/fcrepo/kernel/api/FedoraSession.java | 5 +++-- .../java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java | 6 ++++-- .../fcrepo/kernel/modeshape/services/BatchServiceImpl.java | 4 +++- .../kernel/modeshape/services/BatchServiceImplTest.java | 5 +++-- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java index 8b51c0d801..42db8b3cc0 100644 --- a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/FedoraSession.java @@ -21,6 +21,7 @@ import java.time.Instant; import java.util.Collection; import java.util.Map; +import java.util.Optional; /** * The Fedora Session abstraction @@ -54,9 +55,9 @@ public interface FedoraSession { /** * Get the date this session expires - * @return expiration date + * @return expiration date, if one exists */ - Instant getExpires(); + Optional getExpires(); /** * Get the session identifier diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java index bca9545b4b..ff1575910a 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java @@ -22,12 +22,14 @@ import static java.time.Duration.ofMinutes; import static java.time.Instant.now; import static java.util.Collections.singleton; +import static java.util.Optional.of; import static java.util.UUID.randomUUID; import java.time.Duration; import java.time.Instant; import java.util.Collection; import java.util.Map; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import javax.jcr.RepositoryException; @@ -123,8 +125,8 @@ public Instant getCreated() { } @Override - public Instant getExpires() { - return expires; + public Optional getExpires() { + return of(expires); } @Override diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java index c106132bd6..69d21319f0 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java @@ -72,7 +72,9 @@ public class BatchServiceImpl extends AbstractService implements BatchService { @Scheduled(fixedRate = REAP_INTERVAL) public void removeExpired() { final Set reapable = transactions.entrySet().stream() - .filter(e -> e.getValue().getExpires().isBefore(now())).map(Map.Entry::getKey).collect(toSet()); + .filter(e -> e.getValue().getExpires().isPresent()) + .filter(e -> e.getValue().getExpires().get().isBefore(now())) + .map(Map.Entry::getKey).collect(toSet()); reapable.forEach(key -> { final FedoraSession s = transactions.get(key); if (s != null) { diff --git a/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java b/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java index cca8332694..ce1471eb28 100644 --- a/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java +++ b/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java @@ -18,6 +18,7 @@ package org.fcrepo.kernel.modeshape.services; import static java.time.Instant.now; +import static java.util.Optional.of; import static org.fcrepo.kernel.modeshape.FedoraSessionImpl.FCREPO_TX_ID; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -91,7 +92,7 @@ public void setup() throws Exception { @Test public void testExpiration() { final Instant fiveSecondsAgo = now().minusSeconds(5); - when(mockTx.getExpires()).thenReturn(fiveSecondsAgo); + when(mockTx.getExpires()).thenReturn(of(fiveSecondsAgo)); service.removeExpired(); verify(mockTx).expire(); } @@ -100,7 +101,7 @@ public void testExpiration() { public void testExpirationThrowsRepositoryException() { final Instant fiveSecondsAgo = now().minusSeconds(5); doThrow(new RepositoryRuntimeException("")).when(mockTx).expire(); - when(mockTx.getExpires()).thenReturn(fiveSecondsAgo); + when(mockTx.getExpires()).thenReturn(of(fiveSecondsAgo)); service.removeExpired(); } From 811c8a2ec404b9a8fe3d50bb0df06cb7793db781 Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Wed, 12 Oct 2016 13:03:31 -0400 Subject: [PATCH 12/13] code review --- .../kernel/api/services/BatchService.java | 2 +- .../kernel/modeshape/FedoraSessionImpl.java | 9 ++++++--- .../modeshape/services/BatchServiceImpl.java | 20 ++++++++++--------- .../services/BatchServiceImplTest.java | 2 +- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java index de1f91d15c..eb1ad6c439 100644 --- a/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java +++ b/fcrepo-kernel-api/src/main/java/org/fcrepo/kernel/api/services/BatchService.java @@ -39,7 +39,7 @@ public interface BatchService { void begin(FedoraSession session, String username); /** - * Create a new FedoraSession for the anonymous user and add it to the currently open ones + * Create a new batch operation with a FedoraSession for the anonymous user and add it to the currently open ones * * @param session The session to use for this batch operation */ diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java index ff1575910a..1354311a8e 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraSessionImpl.java @@ -39,6 +39,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.common.annotations.VisibleForTesting; import org.fcrepo.kernel.api.FedoraSession; import org.fcrepo.kernel.api.exception.AccessDeniedException; import org.fcrepo.kernel.api.exception.RepositoryRuntimeException; @@ -51,8 +52,10 @@ public class FedoraSessionImpl implements FedoraSession { // The default timeout is 3 minutes + @VisibleForTesting public static final String DEFAULT_TIMEOUT = Long.toString(ofMinutes(3).toMillis()); + @VisibleForTesting public static final String TIMEOUT_SYSTEM_PROPERTY = "fcrepo.session.timeout"; private final Session jcrSession; @@ -69,7 +72,7 @@ public class FedoraSessionImpl implements FedoraSession { private static final ObjectMapper mapper = new ObjectMapper(); /** - * Create a Fedora session with a JCR session and a username + * Create a Fedora session with a JCR session * @param session the JCR session */ public FedoraSessionImpl(final Session session) { @@ -149,10 +152,10 @@ public Map getNamespaces() { * @param key the data key * @param value the data value * - * Note: while the FedoraSession inteface permits multi-valued + * Note: while the FedoraSession interface permits multi-valued * session data, this implementation constrains that to be single-valued. * That is, calling obj.addSessionData("key", "value1") followed by - * obj.addSessionData("key", "value2") will result in only "value2" being assciated + * obj.addSessionData("key", "value2") will result in only "value2" being associated * with the given key. */ @Override diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java index 69d21319f0..cd7d485c32 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/services/BatchServiceImpl.java @@ -36,6 +36,7 @@ import org.fcrepo.kernel.api.exception.SessionMissingException; import org.fcrepo.kernel.api.services.BatchService; +import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; @@ -60,8 +61,9 @@ public class BatchServiceImpl extends AbstractService implements BatchService { * be either persisted or written to a distributed map or sth, not just this * plain hashmap that follows */ - private static Map transactions = new ConcurrentHashMap<>(); + private static Map sessions = new ConcurrentHashMap<>(); + @VisibleForTesting public static final long REAP_INTERVAL = 1000; /** @@ -71,12 +73,12 @@ public class BatchServiceImpl extends AbstractService implements BatchService { @Override @Scheduled(fixedRate = REAP_INTERVAL) public void removeExpired() { - final Set reapable = transactions.entrySet().stream() + final Set reapable = sessions.entrySet().stream() .filter(e -> e.getValue().getExpires().isPresent()) .filter(e -> e.getValue().getExpires().get().isBefore(now())) .map(Map.Entry::getKey).collect(toSet()); reapable.forEach(key -> { - final FedoraSession s = transactions.get(key); + final FedoraSession s = sessions.get(key); if (s != null) { try { s.expire(); @@ -84,18 +86,18 @@ public void removeExpired() { LOGGER.error("Got exception rolling back expired session {}: {}", s, e.getMessage()); } } - transactions.remove(key); + sessions.remove(key); }); } @Override public void begin(final FedoraSession session, final String username) { - transactions.put(getTxKey(session.getId(), username), session); + sessions.put(getTxKey(session.getId(), username), session); } @Override public FedoraSession getSession(final String sessionId, final String username) { - final FedoraSession session = transactions.get(getTxKey(sessionId, username)); + final FedoraSession session = sessions.get(getTxKey(sessionId, username)); if (session == null) { throw new SessionMissingException("Batch session with id: " + sessionId + " is not available"); } @@ -104,14 +106,14 @@ public FedoraSession getSession(final String sessionId, final String username) { @Override public boolean exists(final String sessionId, final String username) { - return transactions.containsKey(getTxKey(sessionId, username)); + return sessions.containsKey(getTxKey(sessionId, username)); } @Override public void commit(final String sessionId, final String username) { final FedoraSession session = getSession(sessionId, username); session.commit(); - transactions.remove(getTxKey(sessionId, username)); + sessions.remove(getTxKey(sessionId, username)); } @Override @@ -124,7 +126,7 @@ public void refresh(final String sessionId, final String username) { public void abort(final String sessionId, final String username) { final FedoraSession session = getSession(sessionId, username); session.expire(); - transactions.remove(getTxKey(sessionId, username)); + sessions.remove(getTxKey(sessionId, username)); } private static String getTxKey(final String sessionId, final String username) { diff --git a/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java b/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java index ce1471eb28..85563626dc 100644 --- a/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java +++ b/fcrepo-kernel-modeshape/src/test/java/org/fcrepo/kernel/modeshape/services/BatchServiceImplTest.java @@ -80,7 +80,7 @@ public void setup() throws Exception { when(mockTx.getId()).thenReturn(IS_A_TX); when(mockTx.getUserId()).thenReturn(null); final Field txsField = - BatchServiceImpl.class.getDeclaredField("transactions"); + BatchServiceImpl.class.getDeclaredField("sessions"); txsField.setAccessible(true); @SuppressWarnings("unchecked") final Map txs = From eaacdc387aa5b5dea4f43f31f1fb60ffc12c1e1e Mon Sep 17 00:00:00 2001 From: Aaron Coburn Date: Wed, 12 Oct 2016 13:53:58 -0400 Subject: [PATCH 13/13] more code review --- .../java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java index 19fcd71c81..1975884715 100644 --- a/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java +++ b/fcrepo-kernel-modeshape/src/main/java/org/fcrepo/kernel/modeshape/FedoraRepositoryImpl.java @@ -60,7 +60,8 @@ public FedoraSession login(final Object credentials) { throw new RepositoryRuntimeException(ex); } } - throw new IllegalArgumentException("Credentials are of the wrong type"); + throw new ClassCastException("login credentials are not an instance of " + + Credentials.class.getCanonicalName()); } /**