GUACAMOLE-926: Set JDBC to batch mode and just do operations one at a time.

This commit is contained in:
James Muehlner
2023-01-18 23:57:02 +00:00
parent 97e99d6fe3
commit 65074385e1
11 changed files with 188 additions and 176 deletions

View File

@@ -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);

View File

@@ -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<ActiveConnection> {
public class ActiveConnectionDirectory extends JDBCDirectory<ActiveConnection> {
/**
* Service for retrieving and manipulating active connections.

View File

@@ -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<ObjectType extends Identifiable>
extends RestrictedObject implements Directory<ObjectType> {
@Transactional
public void tryAtomically(AtomicDirectoryOperation<ObjectType> 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);
}
}

View File

@@ -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<Connection> {
public class ConnectionDirectory extends JDBCDirectory<Connection> {
/**
* Service for managing connection objects.

View File

@@ -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<ConnectionGroup> {
public class ConnectionGroupDirectory extends JDBCDirectory<ConnectionGroup> {
/**
* Service for managing connection group objects.

View File

@@ -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<SharingProfile> {
public class SharingProfileDirectory extends JDBCDirectory<SharingProfile> {
/**
* Service for managing sharing profile objects.

View File

@@ -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<User> {
public class UserDirectory extends JDBCDirectory<User> {
/**
* Service for managing user objects.

View File

@@ -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<UserGroup> {
public class UserGroupDirectory extends JDBCDirectory<UserGroup> {
/**
* Service for managing user group objects.

View File

@@ -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<ObjectType extends Identifiable> {
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<ObjectType> objects)
throws GuacamoleException {
// Add each object individually by default
Iterator<ObjectType> 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<ObjectType extends Identifiable> {
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<ObjectType> objects)
throws GuacamoleException {
// Update each object individually by default
Iterator<ObjectType> 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<ObjectType extends Identifiable> {
*/
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<String> identifiers)
throws GuacamoleException {
// Remove each object individually by default
Iterator<String> 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

View File

@@ -1,12 +0,0 @@
package org.apache.guacamole.rest.directory;
import org.apache.guacamole.GuacamoleException;
public class DirectoryOperationException<InternalType> extends GuacamoleException {
public DirectoryOperationException(String message) {
super(message);
}
}

View File

@@ -417,49 +417,6 @@ public abstract class DirectoryResource<InternalType extends Identifiable, Exter
public void patchObjects(List<APIPatch<ExternalType>> patches)
throws GuacamoleException {
// Objects will be add, updated, and removed atomically
Collection<InternalType> objectsToAdd = new ArrayList<>();
Collection<InternalType> objectsToUpdate = new ArrayList<>();
Collection<String> identifiersToRemove = new ArrayList<>();
// Apply each operation specified within the patch
for (APIPatch<ExternalType> 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<InternalType>() {
@@ -475,21 +432,106 @@ public abstract class DirectoryResource<InternalType extends Identifiable, Exter
"Atomic operations are not supported. " +
"The patch cannot be executed.");
// First, create every object from the patch
directory.add(objectsToAdd);
// Next, update every object from the patch
directory.update(objectsToUpdate);
// Keep a list of all objects that have been successfully
// added, updated, or removed
Collection<InternalType> addedObjects = new ArrayList<>();
Collection<InternalType> updatedObjects = new ArrayList<>();
Collection<String> 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<ExternalType> 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<InternalType> addedIterator = objectsToAdd.iterator();
Iterator<InternalType> addedIterator = addedObjects.iterator();
while (addedIterator.hasNext()) {
InternalType internal = addedIterator.next();
@@ -499,7 +541,7 @@ public abstract class DirectoryResource<InternalType extends Identifiable, Exter
}
// Fire directory success events for each updated object
Iterator<InternalType> updatedIterator = objectsToUpdate.iterator();
Iterator<InternalType> updatedIterator = updatedObjects.iterator();
while (updatedIterator.hasNext()) {
InternalType internal = updatedIterator.next();
@@ -509,7 +551,7 @@ public abstract class DirectoryResource<InternalType extends Identifiable, Exter
}
// Fire directory success events for each removed object
Iterator<String> removedIterator = identifiersToRemove.iterator();
Iterator<String> removedIterator = removedIdentifiers.iterator();
while (removedIterator.hasNext()) {
String identifier = removedIterator.next();
@@ -520,6 +562,12 @@ public abstract class DirectoryResource<InternalType extends Identifiable, Exter
}
});
// TODO: JSON response with failures or success
}
/**
* Creates a new object within the underlying Directory, returning the
* object that was created. The identifier of the created object will be