diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java index 5ae0ea53f..7b735bdf4 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java @@ -45,6 +45,7 @@ import org.apache.guacamole.auth.jdbc.security.SaltService; import org.apache.guacamole.auth.jdbc.security.SecureRandomSaltService; import org.apache.guacamole.auth.jdbc.permission.SystemPermissionService; import org.apache.guacamole.auth.jdbc.user.UserService; +import org.apache.ibatis.session.ExecutorType; import org.apache.ibatis.transaction.jdbc.JdbcTransactionFactory; import org.apache.guacamole.auth.jdbc.permission.ConnectionGroupPermissionMapper; import org.apache.guacamole.auth.jdbc.permission.ConnectionGroupPermissionService; @@ -126,6 +127,11 @@ public class JDBCAuthenticationProviderModule extends MyBatisModule { // Transaction factory bindTransactionFactoryType(JdbcTransactionFactory.class); + // Set the JDBC Auth provider to use batch execution when possible + bindConfigurationSetting(configuration -> { + configuration.setDefaultExecutorType(ExecutorType.BATCH); + }); + // Add MyBatis mappers addMapperClass(ConnectionMapper.class); addMapperClass(ConnectionGroupMapper.class); diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/activeconnection/ActiveConnectionDirectory.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/activeconnection/ActiveConnectionDirectory.java index b0d6324aa..28510b738 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/activeconnection/ActiveConnectionDirectory.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/activeconnection/ActiveConnectionDirectory.java @@ -25,16 +25,14 @@ import java.util.Collection; import java.util.Collections; import java.util.Set; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.jdbc.base.RestrictedObject; +import org.apache.guacamole.auth.jdbc.base.JDBCDirectory; import org.apache.guacamole.net.auth.ActiveConnection; -import org.apache.guacamole.net.auth.Directory; /** * Implementation of a Directory which contains all currently-active * connections. */ -public class ActiveConnectionDirectory extends RestrictedObject - implements Directory { +public class ActiveConnectionDirectory extends JDBCDirectory { /** * Service for retrieving and manipulating active connections. diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/JDBCDirectory.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/JDBCDirectory.java new file mode 100644 index 000000000..fcba2f678 --- /dev/null +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/base/JDBCDirectory.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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.apache.guacamole.auth.jdbc.base; + +import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.net.auth.AtomicDirectoryOperation; +import org.apache.guacamole.net.auth.Directory; +import org.apache.guacamole.net.auth.Identifiable; +import org.mybatis.guice.transactional.Transactional; + +/** + * An implementation of Directory that uses database transactions to guarantee + * atomicity for any operations supplied to tryAtomically(). + */ +public abstract class JDBCDirectory + extends RestrictedObject implements Directory { + + @Transactional + public void tryAtomically(AtomicDirectoryOperation operation) + throws GuacamoleException { + + // Execute the operation atomically - the @Transactional annotation + // specifies that the entire operation will be performed in a transaction + operation.executeOperation(true, this); + + } +} diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionDirectory.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionDirectory.java index 52a127df4..3e364f509 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionDirectory.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionDirectory.java @@ -25,17 +25,15 @@ import java.util.Collection; import java.util.Collections; import java.util.Set; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.jdbc.base.RestrictedObject; +import org.apache.guacamole.auth.jdbc.base.JDBCDirectory; import org.apache.guacamole.net.auth.Connection; -import org.apache.guacamole.net.auth.Directory; import org.mybatis.guice.transactional.Transactional; /** * Implementation of the Connection Directory which is driven by an underlying, * arbitrary database. */ -public class ConnectionDirectory extends RestrictedObject - implements Directory { +public class ConnectionDirectory extends JDBCDirectory { /** * Service for managing connection objects. diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connectiongroup/ConnectionGroupDirectory.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connectiongroup/ConnectionGroupDirectory.java index 9f3930597..2e21dc21d 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connectiongroup/ConnectionGroupDirectory.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connectiongroup/ConnectionGroupDirectory.java @@ -25,17 +25,15 @@ import java.util.Collection; import java.util.Collections; import java.util.Set; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.jdbc.base.RestrictedObject; +import org.apache.guacamole.auth.jdbc.base.JDBCDirectory; import org.apache.guacamole.net.auth.ConnectionGroup; -import org.apache.guacamole.net.auth.Directory; import org.mybatis.guice.transactional.Transactional; /** * Implementation of the ConnectionGroup Directory which is driven by an * underlying, arbitrary database. */ -public class ConnectionGroupDirectory extends RestrictedObject - implements Directory { +public class ConnectionGroupDirectory extends JDBCDirectory { /** * Service for managing connection group objects. diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharingprofile/SharingProfileDirectory.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharingprofile/SharingProfileDirectory.java index 632557052..4035ff03f 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharingprofile/SharingProfileDirectory.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/sharingprofile/SharingProfileDirectory.java @@ -24,8 +24,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Set; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.jdbc.base.RestrictedObject; -import org.apache.guacamole.net.auth.Directory; +import org.apache.guacamole.auth.jdbc.base.JDBCDirectory; import org.apache.guacamole.net.auth.SharingProfile; import org.mybatis.guice.transactional.Transactional; @@ -33,8 +32,7 @@ import org.mybatis.guice.transactional.Transactional; * Implementation of the SharingProfile Directory which is driven by an * underlying, arbitrary database. */ -public class SharingProfileDirectory extends RestrictedObject - implements Directory { +public class SharingProfileDirectory extends JDBCDirectory { /** * Service for managing sharing profile objects. diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserDirectory.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserDirectory.java index dffd8e2ec..72d5a4f30 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserDirectory.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserDirectory.java @@ -25,8 +25,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Set; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.jdbc.base.RestrictedObject; -import org.apache.guacamole.net.auth.Directory; +import org.apache.guacamole.auth.jdbc.base.JDBCDirectory; import org.apache.guacamole.net.auth.User; import org.mybatis.guice.transactional.Transactional; @@ -34,8 +33,7 @@ import org.mybatis.guice.transactional.Transactional; * Implementation of the User Directory which is driven by an underlying, * arbitrary database. */ -public class UserDirectory extends RestrictedObject - implements Directory { +public class UserDirectory extends JDBCDirectory { /** * Service for managing user objects. diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupDirectory.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupDirectory.java index 911b8521f..c6bb89572 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupDirectory.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/usergroup/UserGroupDirectory.java @@ -24,8 +24,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Set; import org.apache.guacamole.GuacamoleException; -import org.apache.guacamole.auth.jdbc.base.RestrictedObject; -import org.apache.guacamole.net.auth.Directory; +import org.apache.guacamole.auth.jdbc.base.JDBCDirectory; import org.apache.guacamole.net.auth.UserGroup; import org.mybatis.guice.transactional.Transactional; @@ -33,8 +32,7 @@ import org.mybatis.guice.transactional.Transactional; * Implementation of the UserGroup Directory which is driven by an underlying, * arbitrary database. */ -public class UserGroupDirectory extends RestrictedObject - implements Directory { +public class UserGroupDirectory extends JDBCDirectory { /** * Service for managing user group objects. diff --git a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Directory.java b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Directory.java index ac76fae9d..bc5c52afa 100644 --- a/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Directory.java +++ b/guacamole-ext/src/main/java/org/apache/guacamole/net/auth/Directory.java @@ -20,7 +20,6 @@ package org.apache.guacamole.net.auth; import java.util.Collection; -import java.util.Iterator; import java.util.Set; import org.apache.guacamole.GuacamoleException; @@ -199,29 +198,6 @@ public interface Directory { void add(ObjectType object) throws GuacamoleException; - /** - * Adds the given objects to the overall set. If new identifiers are - * created for any of the the added objects, the identifiers will be - * automatically assigned via setIdentifier(). - * - * @param objects - * The objects to add. - * - * @throws GuacamoleException - * If an error occurs while adding any of the objects, or if adding - * the objects is not allowed. - */ - default void add(Collection objects) - throws GuacamoleException { - - // Add each object individually by default - Iterator iterator = objects.iterator(); - while (iterator.hasNext()) { - add(iterator.next()); - } - - } - /** * Updates the stored object with the data contained in the given object. * @@ -233,25 +209,6 @@ public interface Directory { void update(ObjectType object) throws GuacamoleException; - /** - * Updates the stored objects with the data contained in the given objects. - * - * @param objects The objects which will supply the data for the update. - * - * @throws GuacamoleException If an error occurs while updating the objects, - * or if updating an object is not allowed. - */ - default void update(Collection objects) - throws GuacamoleException { - - // Update each object individually by default - Iterator iterator = objects.iterator(); - while (iterator.hasNext()) { - update(iterator.next()); - } - - } - /** * Removes the object with the given identifier from the overall set. * @@ -262,25 +219,6 @@ public interface Directory { */ void remove(String identifier) throws GuacamoleException; - /** - * Removes all object with any of the given identifier from the overall set. - * - * @param identifiers The identifiers of the objects to remove. - * - * @throws GuacamoleException If an error occurs while removing an object, - * or if removing an object is not allowed. - */ - default void remove(Collection identifiers) - throws GuacamoleException { - - // Remove each object individually by default - Iterator iterator = identifiers.iterator(); - while (iterator.hasNext()) { - remove(iterator.next()); - } - - } - /** * Attempt to perform the provided operation atomically if possible. If the * operation can be performed atomically, the atomic flag will be set to diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryOperationException.java b/guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryOperationException.java deleted file mode 100644 index 3a88d4270..000000000 --- a/guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryOperationException.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.apache.guacamole.rest.directory; - -import org.apache.guacamole.GuacamoleException; - -public class DirectoryOperationException extends GuacamoleException { - - public DirectoryOperationException(String message) { - super(message); - } - - -} diff --git a/guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryResource.java b/guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryResource.java index 8aa464bd0..412068a96 100644 --- a/guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryResource.java +++ b/guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryResource.java @@ -417,49 +417,6 @@ public abstract class DirectoryResource> patches) throws GuacamoleException { - // Objects will be add, updated, and removed atomically - Collection objectsToAdd = new ArrayList<>(); - Collection objectsToUpdate = new ArrayList<>(); - Collection identifiersToRemove = new ArrayList<>(); - - // Apply each operation specified within the patch - for (APIPatch patch : patches) { - - // Retrieve and validate path - String path = patch.getPath(); - if (!path.startsWith("/")) - throw new GuacamoleClientException("Patch paths must start with \"/\"."); - - // Append each provided object to the list, to be added atomically - if(patch.getOp() == APIPatch.Operation.add) { - - // Filter/sanitize object contents - InternalType internal = filterAndTranslate(patch.getValue()); - - // Add to the list of objects to create - objectsToAdd.add(internal); - } - - // Append each provided object to the list, to be updated atomically - else if (patch.getOp() == APIPatch.Operation.replace) { - - // Filter/sanitize object contents - InternalType internal = filterAndTranslate(patch.getValue()); - - // Add to the list of objects to update - objectsToUpdate.add(internal); - } - - // Append each identifier to the list, to be removed atomically - else if (patch.getOp() == APIPatch.Operation.remove) { - - String identifier = path.substring(1); - identifiersToRemove.add(identifier); - - } - - } - // Perform all requested operations atomically directory.tryAtomically(new AtomicDirectoryOperation() { @@ -475,48 +432,139 @@ public abstract class DirectoryResource addedObjects = new ArrayList<>(); + Collection updatedObjects = new ArrayList<>(); + Collection removedIdentifiers = new ArrayList<>(); - // Finally, remove every object from the patch - directory.remove(identifiersToRemove); + // True if any operation in the patch failed. Any failure will + // fail the request, though won't result in immediate stoppage + // since more errors may yet be uncovered. + boolean failed = false; + + // Apply each operation specified within the patch + for (APIPatch patch : patches) { + + // Retrieve and validate path + String path = patch.getPath(); + if (!path.startsWith("/")) + throw new GuacamoleClientException("Patch paths must start with \"/\"."); + + if(patch.getOp() == APIPatch.Operation.add) { + + // Filter/sanitize object contents + InternalType internal = filterAndTranslate(patch.getValue()); + + try { + + // Attempt to add the new object + directory.add(internal); + + // Add the object to the list if addition was successful + addedObjects.add(internal); + + } + catch (GuacamoleException | RuntimeException | Error e) { + fireDirectoryFailureEvent( + DirectoryEvent.Operation.ADD, + internal.getIdentifier(), internal, e); + + // TODO: Save the error for later inclusion in a big JSON error response + failed = true; + } + + } + + else if (patch.getOp() == APIPatch.Operation.replace) { + + // Filter/sanitize object contents + InternalType internal = filterAndTranslate(patch.getValue()); + + try { + + // Attempt to update the object + directory.update(internal); + + // Add the object to the list if the update was successful + updatedObjects.add(internal); + } + catch (GuacamoleException | RuntimeException | Error e) { + fireDirectoryFailureEvent( + DirectoryEvent.Operation.UPDATE, + internal.getIdentifier(), internal, e); + + // TODO: Save the error for later inclusion in a big JSON error response + failed = true; + } + } + + // Append each identifier to the list, to be removed atomically + else if (patch.getOp() == APIPatch.Operation.remove) { + + String identifier = path.substring(1); + + try { + + // Attempt to remove the object + directory.remove(identifier); + + // Add the object to the list if the removal was successful + removedIdentifiers.add(identifier); + } + catch (GuacamoleException | RuntimeException | Error e) { + fireDirectoryFailureEvent( + DirectoryEvent.Operation.UPDATE, identifier, null, e); + + // TODO: Save the error for later inclusion in a big JSON error response + failed = true; + } + } + + } + + // If any operation failed, fail now + if (failed) { + throw new GuacamoleClientException( + "oh noes the patch batch failed"); + } + + // Fire directory success events for each created object + Iterator addedIterator = addedObjects.iterator(); + while (addedIterator.hasNext()) { + + InternalType internal = addedIterator.next(); + fireDirectorySuccessEvent( + DirectoryEvent.Operation.ADD, internal.getIdentifier(), internal); + + } + + // Fire directory success events for each updated object + Iterator updatedIterator = updatedObjects.iterator(); + while (updatedIterator.hasNext()) { + + InternalType internal = updatedIterator.next(); + fireDirectorySuccessEvent( + DirectoryEvent.Operation.UPDATE, internal.getIdentifier(), internal); + + } + + // Fire directory success events for each removed object + Iterator removedIterator = removedIdentifiers.iterator(); + while (removedIterator.hasNext()) { + + String identifier = removedIterator.next(); + fireDirectorySuccessEvent( + DirectoryEvent.Operation.UPDATE, identifier, null); + + } } }); - // Fire directory success events for each created object - Iterator addedIterator = objectsToAdd.iterator(); - while (addedIterator.hasNext()) { - - InternalType internal = addedIterator.next(); - fireDirectorySuccessEvent( - DirectoryEvent.Operation.ADD, internal.getIdentifier(), internal); - - } - - // Fire directory success events for each updated object - Iterator updatedIterator = objectsToUpdate.iterator(); - while (updatedIterator.hasNext()) { - - InternalType internal = updatedIterator.next(); - fireDirectorySuccessEvent( - DirectoryEvent.Operation.UPDATE, internal.getIdentifier(), internal); - - } - - // Fire directory success events for each removed object - Iterator removedIterator = identifiersToRemove.iterator(); - while (removedIterator.hasNext()) { - - String identifier = removedIterator.next(); - fireDirectorySuccessEvent( - DirectoryEvent.Operation.UPDATE, identifier, null); - - } + // TODO: JSON response with failures or success }