necouchman commented on a change in pull request #698:
URL: https://github.com/apache/guacamole-client/pull/698#discussion_r805229375



##########
File path: 
extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java
##########
@@ -0,0 +1,309 @@
+/*
+ * 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.history.connection;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.net.MalformedURLException;
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.environment.Environment;
+import org.apache.guacamole.environment.LocalEnvironment;
+import org.apache.guacamole.io.GuacamoleReader;
+import org.apache.guacamole.io.ReaderGuacamoleReader;
+import org.apache.guacamole.language.TranslatableMessage;
+import org.apache.guacamole.net.auth.ActivityLog;
+import org.apache.guacamole.net.auth.ConnectionRecord;
+import org.apache.guacamole.net.auth.DelegatingConnectionRecord;
+import org.apache.guacamole.net.auth.FileActivityLog;
+import org.apache.guacamole.properties.FileGuacamoleProperty;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * ConnectionRecord implementation that automatically defines ActivityLogs for
+ * files that relate to the wrapped record.
+ */
+public class HistoryConnectionRecord extends DelegatingConnectionRecord {
+
+    /**
+     * Logger for this class.
+     */
+    private static final Logger logger = 
LoggerFactory.getLogger(HistoryConnectionRecord.class);
+
+    /**
+     * The namespace for URL UUIDs as defined by RFC 4122.
+     */
+    private static final UUID UUID_NAMESPACE_URL = 
UUID.fromString("6ba7b811-9dad-11d1-80b4-00c04fd430c8");
+
+    /**
+     * The filename suffix of typescript timing files.
+     */
+    private static final String TIMING_FILE_SUFFIX = ".timing";
+
+    /**
+     * The default directory to search for associated session recordings, if
+     * not overridden with the "recording-search-path" property.
+     */
+    private static final File DEFAULT_RECORDING_SEARCH_PATH = new 
File("/var/lib/guacamole/recordings");
+
+    /**
+     * The directory to search for associated session recordings. By default,
+     * "/var/lib/guacamole/recordings" will be used.
+     */
+    private static final FileGuacamoleProperty RECORDING_SEARCH_PATH = new 
FileGuacamoleProperty() {
+
+        @Override
+        public String getName() {
+            return "recording-search-path";
+        }
+
+    };
+
+    /**
+     * The recording file associated with the wrapped connection record. This
+     * may be a single file or a directory that may contain any number of
+     * relevant recordings.
+     */
+    private final File recording;
+
+    /**
+     * Creates a new HistoryConnectionRecord that wraps the given
+     * ConnectionRecord, automatically associating ActivityLogs based on
+     * related files (session recordings, typescripts, etc.).
+     *
+     * @param record
+     *     The ConnectionRecord to wrap.
+     *
+     * @throws GuacamoleException
+     *     If the configured path for stored recordings cannot be read.
+     */
+    public HistoryConnectionRecord(ConnectionRecord record) throws 
GuacamoleException {
+        super(record);
+
+        Environment environment = LocalEnvironment.getInstance();
+        File recordingPath = environment.getProperty(RECORDING_SEARCH_PATH,
+                DEFAULT_RECORDING_SEARCH_PATH);
+
+        String uuid = record.getUUID().toString();
+        File recordingFile = new File(recordingPath, uuid);
+        this.recording = recordingFile.canRead() ? recordingFile : null;
+
+    }
+
+    /**
+     * Returns whether the given file appears to be a Guacamole session
+     * recording. As there is no standard extension for session recordings,
+     * this is determined by attempting to read a single Guacamole instruction
+     * from the file.
+     *
+     * @param file
+     *     The file to test.
+     *
+     * @return
+     *     true if the file appears to be a Guacamole session recording, false
+     *     otherwise.
+     */
+    private boolean isSessionRecording(File file) {
+
+        try (Reader reader = new InputStreamReader(new FileInputStream(file), 
StandardCharsets.UTF_8)) {
+
+            GuacamoleReader guacReader = new ReaderGuacamoleReader(reader);
+            if (guacReader.readInstruction() != null)
+                return true;
+
+        }
+        catch (GuacamoleException e) {
+            logger.debug("File \"{}\" does not appear to be a session "
+                    + "recording, as it could not be parsed as Guacamole "
+                    + "protocol data.", file, e);
+        }
+        catch (IOException e) {
+            logger.warn("Possible session recording \"{}\" could not be "
+                    + "identified as it cannot be read: {}", file, 
e.getMessage());
+            logger.debug("Possible session recording \"{}\" could not be 
read.", file, e);
+        }
+
+        return false;
+
+    }
+
+    /**
+     * Returns whether the given file appears to be a typescript (text
+     * recording of a terminal session). As there is no standard extension for
+     * session recordings, this is determined by testing whether there is an
+     * associated timing file. Guacamole will always include a timing file for
+     * its typescripts.
+     *
+     * @param file
+     *     The file to test.
+     *
+     * @return
+     *     true if the file appears to be a typescript, false otherwise.
+     */
+    private boolean isTypescript(File file) {
+        return new File(file.getAbsolutePath() + TIMING_FILE_SUFFIX).exists();
+    }
+
+    /**
+     * Returns whether the given file appears to be a typescript timing file.
+     * Typescript timing files have the standard extension ".timing".
+     *
+     * @param file
+     *     The file to test.
+     *
+     * @return
+     *     true if the file appears to be a typescript timing file, false
+     *     otherwise.
+     */
+    private boolean isTypescriptTiming(File file) {
+        return file.getName().endsWith(TIMING_FILE_SUFFIX);
+    }
+
+    /**
+     * Returns the type of session recording or log contained within the given
+     * file by inspecting its name and contents.
+     *
+     * @param file
+     *     The file to test.
+     *
+     * @return
+     *     The type of session recording or log contained within the given
+     *     file, or null if this cannot be determined.

Review comment:
       Is this true - that `null` will be returned if the type cannot be 
determined? It looks to me like `ActivityLog.Type.SERVER_LOG` will be returned? 
Looking at the below code fro `ActivityLog`, that doesn't look to be `null`?

##########
File path: 
guacamole/src/main/frontend/src/app/rest/types/ConnectionHistoryEntry.js
##########
@@ -38,6 +38,23 @@ angular.module('rest').factory('ConnectionHistoryEntry', 
[function defineConnect
         // Use empty object by default
         template = template || {};
 
+        /**
+         * An arbitrary unique identifier that uniquely identifies this record

Review comment:
       It would be odd to have a unique identifier that did not uniquely 
identify this record.

##########
File path: 
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordMapper.java
##########
@@ -19,113 +19,9 @@
 
 package org.apache.guacamole.auth.jdbc.connection;
 
-import java.util.Collection;
-import java.util.List;
-import org.apache.guacamole.auth.jdbc.base.ActivityRecordSearchTerm;
-import org.apache.guacamole.auth.jdbc.base.ActivityRecordSortPredicate;
-import org.apache.ibatis.annotations.Param;
-import org.apache.guacamole.auth.jdbc.user.UserModel;
+import org.apache.guacamole.auth.jdbc.base.ActivityRecordMapper;
 
 /**
  * Mapper for connection record objects.
  */
-public interface ConnectionRecordMapper {
-
-    /**
-     * Returns a collection of all connection records associated with the
-     * connection having the given identifier.
-     *
-     * @param identifier
-     *     The identifier of the connection whose records are to be retrieved.
-     *
-     * @return
-     *     A collection of all connection records associated with the
-     *     connection having the given identifier. This collection will be
-     *     empty if no such connection exists.
-     */
-    List<ConnectionRecordModel> select(@Param("identifier") String identifier);
-
-    /**
-     * Inserts the given connection record.
-     *
-     * @param record
-     *     The connection record to insert.
-     *
-     * @return
-     *     The number of rows inserted.
-     */
-    int insert(@Param("record") ConnectionRecordModel record);
-
-    /**
-     * Searches for up to <code>limit</code> connection records that contain
-     * the given terms, sorted by the given predicates, regardless of whether
-     * the data they are associated with is is readable by any particular user.
-     * This should only be called on behalf of a system administrator. If
-     * records are needed by a non-administrative user who must have explicit
-     * read rights, use {@link searchReadable()} instead.
-     *
-     * @param identifier
-     *     The optional connection identifier to which records should be 
limited,
-     *     or null if all records should be retrieved.
-     * 
-     * @param terms
-     *     The search terms that must match the returned records.
-     *
-     * @param sortPredicates
-     *     A list of predicates to sort the returned records by, in order of
-     *     priority.
-     *
-     * @param limit
-     *     The maximum number of records that should be returned.
-     *
-     * @return
-     *     The results of the search performed with the given parameters.
-     */
-    List<ConnectionRecordModel> search(@Param("identifier") String identifier,
-            @Param("terms") Collection<ActivityRecordSearchTerm> terms,
-            @Param("sortPredicates") List<ActivityRecordSortPredicate> 
sortPredicates,
-            @Param("limit") int limit);
-
-    /**
-     * Searches for up to <code>limit</code> connection records that contain
-     * the given terms, sorted by the given predicates. Only records that are
-     * associated with data explicitly readable by the given user will be
-     * returned. If records are needed by a system administrator (who, by
-     * definition, does not need explicit read rights), use {@link search()}
-     * instead.
-     *
-     * @param identifier
-     *     The optional connection identifier for which records should be
-     *     retrieved, or null if all readable records should be retrieved.
-     * 
-     * @param user
-     *    The user whose permissions should determine whether a record is
-     *    returned.
-     *
-     * @param terms
-     *     The search terms that must match the returned records.
-     *
-     * @param sortPredicates
-     *     A list of predicates to sort the returned records by, in order of
-     *     priority.
-     *
-     * @param limit
-     *     The maximum number of records that should be returned.
-     *
-     * @param effectiveGroups
-     *     The identifiers of all groups that should be taken into account
-     *     when determining the permissions effectively granted to the user. If
-     *     no groups are given, only permissions directly granted to the user
-     *     will be used.
-     *
-     * @return
-     *     The results of the search performed with the given parameters.
-     */
-    List<ConnectionRecordModel> searchReadable(@Param("identifier") String 
identifier,
-            @Param("user") UserModel user,
-            @Param("terms") Collection<ActivityRecordSearchTerm> terms,
-            @Param("sortPredicates") List<ActivityRecordSortPredicate> 
sortPredicates,
-            @Param("limit") int limit,
-            @Param("effectiveGroups") Collection<String> effectiveGroups);
-
-}
+public interface ConnectionRecordMapper extends 
ActivityRecordMapper<ConnectionRecordModel> {}

Review comment:
       If all the functionality is being moved to `ActivityRecordMapper`, 
should this interface be deprecated? Or will that cause other down-stream 
impacts?

##########
File path: guacamole/src/main/frontend/src/app/rest/types/ActivityLog.js
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.
+ */
+
+/**
+ * Service which defines the ActivityLog class.
+ */
+angular.module('rest').factory('ActivityLog', [function defineActivityLog() {
+
+    /**
+     * The object returned by REST API calls when representing the a log or

Review comment:
       `the a log`

##########
File path: 
guacamole/src/main/java/org/apache/guacamole/rest/history/APIActivityRecord.java
##########
@@ -55,6 +58,30 @@
      */
     private final boolean active;
 
+    /**
+     * The unique identifier assigned to this record, or null if this record
+     * has no such identifier.
+     */
+    private final String identifier;
+    
+    /**
+     * A UUID uniquely identifies this record, or null if no such unique

Review comment:
       `UUID` _that_ `uniquely identifies`....although, given my earlier 
comment even that could be redundant...

##########
File path: 
extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserRecordMapper.java
##########
@@ -19,124 +19,10 @@
 
 package org.apache.guacamole.auth.jdbc.user;
 
-import java.util.Collection;
-import java.util.List;
+import org.apache.guacamole.auth.jdbc.base.ActivityRecordMapper;
 import org.apache.guacamole.auth.jdbc.base.ActivityRecordModel;
-import org.apache.guacamole.auth.jdbc.base.ActivityRecordSearchTerm;
-import org.apache.guacamole.auth.jdbc.base.ActivityRecordSortPredicate;
-import org.apache.ibatis.annotations.Param;
 
 /**
  * Mapper for user login activity records.
  */
-public interface UserRecordMapper {
-
-    /**
-     * Returns a collection of all user login records associated with the user
-     * having the given username.
-     *
-     * @param username
-     *     The username of the user whose login records are to be retrieved.
-     *
-     * @return
-     *     A collection of all user login records associated with the user
-     *     having the given username. This collection will be empty if no such
-     *     user exists.
-     */
-    List<ActivityRecordModel> select(@Param("username") String username);
-
-    /**
-     * Inserts the given user login record.
-     *
-     * @param record
-     *     The user login record to insert.
-     *
-     * @return
-     *     The number of rows inserted.
-     */
-    int insert(@Param("record") ActivityRecordModel record);
-
-    /**
-     * Updates the given user login record.
-     *
-     * @param record
-     *     The user login record to update.
-     *
-     * @return
-     *     The number of rows updated.
-     */
-    int update(@Param("record") ActivityRecordModel record);
-
-    /**
-     * Searches for up to <code>limit</code> user login records that contain
-     * the given terms, sorted by the given predicates, regardless of whether
-     * the data they are associated with is is readable by any particular user.
-     * This should only be called on behalf of a system administrator. If
-     * records are needed by a non-administrative user who must have explicit
-     * read rights, use {@link searchReadable()} instead.
-     *
-     * @param username
-     *     The optional username to which records should be limited, or null
-     *     if all records should be retrieved.
-     * 
-     * @param terms
-     *     The search terms that must match the returned records.
-     *
-     * @param sortPredicates
-     *     A list of predicates to sort the returned records by, in order of
-     *     priority.
-     *
-     * @param limit
-     *     The maximum number of records that should be returned.
-     *
-     * @return
-     *     The results of the search performed with the given parameters.
-     */
-    List<ActivityRecordModel> search(@Param("username") String username,
-            @Param("terms") Collection<ActivityRecordSearchTerm> terms,
-            @Param("sortPredicates") List<ActivityRecordSortPredicate> 
sortPredicates,
-            @Param("limit") int limit);
-
-    /**
-     * Searches for up to <code>limit</code> user login records that contain
-     * the given terms, sorted by the given predicates. Only records that are
-     * associated with data explicitly readable by the given user will be
-     * returned. If records are needed by a system administrator (who, by
-     * definition, does not need explicit read rights), use {@link search()}
-     * instead.
-     *
-     * @param username
-     *     The optional username to which records should be limited, or null
-     *     if all readable records should be retrieved.
-     * 
-     * @param user
-     *     The user whose permissions should determine whether a record is
-     *     returned.
-     *
-     * @param terms
-     *     The search terms that must match the returned records.
-     *
-     * @param sortPredicates
-     *     A list of predicates to sort the returned records by, in order of
-     *     priority.
-     *
-     * @param limit
-     *     The maximum number of records that should be returned.
-     *
-     * @param effectiveGroups
-     *     The identifiers of all groups that should be taken into account
-     *     when determining the permissions effectively granted to the user. If
-     *     no groups are given, only permissions directly granted to the user
-     *     will be used.
-     *
-     * @return
-     *     The results of the search performed with the given parameters.
-     */
-    List<ActivityRecordModel> searchReadable(@Param("username") String 
username,
-            @Param("user") UserModel user,
-            @Param("terms") Collection<ActivityRecordSearchTerm> terms,
-            @Param("sortPredicates") List<ActivityRecordSortPredicate> 
sortPredicates,
-            @Param("limit") int limit,
-            @Param("effectiveGroups") Collection<String> effectiveGroups);
-
-}
+public interface UserRecordMapper extends 
ActivityRecordMapper<ActivityRecordModel> {}

Review comment:
       As with `ConnectionRecordMapper`, should this be deprecated?

##########
File path: 
guacamole/src/main/java/org/apache/guacamole/rest/history/ActivityLogResource.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rest.history;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.ResponseBuilder;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.net.auth.ActivityLog;
+
+/**
+ * A REST resource which exposes the contents of a given ActivityLog.
+ */
+public class ActivityLogResource {
+
+    /**
+     * The ActivityLog whose contents are being exposed.
+     */
+    private final ActivityLog log;
+
+    /**
+     * Creates a new ActivityRecordSetResource which exposes the records within
+     * the given ActivityRecordSet.

Review comment:
       Yum....copy pasta? I think this is an `ActivityLogResource` exposing the 
given `ActivityLog`?

##########
File path: 
extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java
##########
@@ -0,0 +1,309 @@
+/*
+ * 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.history.connection;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.net.MalformedURLException;
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.environment.Environment;
+import org.apache.guacamole.environment.LocalEnvironment;
+import org.apache.guacamole.io.GuacamoleReader;
+import org.apache.guacamole.io.ReaderGuacamoleReader;
+import org.apache.guacamole.language.TranslatableMessage;
+import org.apache.guacamole.net.auth.ActivityLog;
+import org.apache.guacamole.net.auth.ConnectionRecord;
+import org.apache.guacamole.net.auth.DelegatingConnectionRecord;
+import org.apache.guacamole.net.auth.FileActivityLog;
+import org.apache.guacamole.properties.FileGuacamoleProperty;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * ConnectionRecord implementation that automatically defines ActivityLogs for
+ * files that relate to the wrapped record.
+ */
+public class HistoryConnectionRecord extends DelegatingConnectionRecord {
+
+    /**
+     * Logger for this class.
+     */
+    private static final Logger logger = 
LoggerFactory.getLogger(HistoryConnectionRecord.class);
+
+    /**
+     * The namespace for URL UUIDs as defined by RFC 4122.
+     */
+    private static final UUID UUID_NAMESPACE_URL = 
UUID.fromString("6ba7b811-9dad-11d1-80b4-00c04fd430c8");
+
+    /**
+     * The filename suffix of typescript timing files.
+     */
+    private static final String TIMING_FILE_SUFFIX = ".timing";
+
+    /**
+     * The default directory to search for associated session recordings, if
+     * not overridden with the "recording-search-path" property.
+     */
+    private static final File DEFAULT_RECORDING_SEARCH_PATH = new 
File("/var/lib/guacamole/recordings");
+
+    /**
+     * The directory to search for associated session recordings. By default,
+     * "/var/lib/guacamole/recordings" will be used.
+     */
+    private static final FileGuacamoleProperty RECORDING_SEARCH_PATH = new 
FileGuacamoleProperty() {
+
+        @Override
+        public String getName() {
+            return "recording-search-path";
+        }
+
+    };
+
+    /**
+     * The recording file associated with the wrapped connection record. This
+     * may be a single file or a directory that may contain any number of
+     * relevant recordings.
+     */
+    private final File recording;
+
+    /**
+     * Creates a new HistoryConnectionRecord that wraps the given
+     * ConnectionRecord, automatically associating ActivityLogs based on
+     * related files (session recordings, typescripts, etc.).
+     *
+     * @param record
+     *     The ConnectionRecord to wrap.
+     *
+     * @throws GuacamoleException
+     *     If the configured path for stored recordings cannot be read.
+     */
+    public HistoryConnectionRecord(ConnectionRecord record) throws 
GuacamoleException {
+        super(record);
+
+        Environment environment = LocalEnvironment.getInstance();
+        File recordingPath = environment.getProperty(RECORDING_SEARCH_PATH,
+                DEFAULT_RECORDING_SEARCH_PATH);
+
+        String uuid = record.getUUID().toString();
+        File recordingFile = new File(recordingPath, uuid);
+        this.recording = recordingFile.canRead() ? recordingFile : null;
+
+    }
+
+    /**
+     * Returns whether the given file appears to be a Guacamole session
+     * recording. As there is no standard extension for session recordings,
+     * this is determined by attempting to read a single Guacamole instruction
+     * from the file.
+     *
+     * @param file
+     *     The file to test.
+     *
+     * @return
+     *     true if the file appears to be a Guacamole session recording, false
+     *     otherwise.
+     */
+    private boolean isSessionRecording(File file) {
+
+        try (Reader reader = new InputStreamReader(new FileInputStream(file), 
StandardCharsets.UTF_8)) {
+
+            GuacamoleReader guacReader = new ReaderGuacamoleReader(reader);
+            if (guacReader.readInstruction() != null)
+                return true;
+
+        }
+        catch (GuacamoleException e) {
+            logger.debug("File \"{}\" does not appear to be a session "
+                    + "recording, as it could not be parsed as Guacamole "
+                    + "protocol data.", file, e);
+        }
+        catch (IOException e) {
+            logger.warn("Possible session recording \"{}\" could not be "
+                    + "identified as it cannot be read: {}", file, 
e.getMessage());
+            logger.debug("Possible session recording \"{}\" could not be 
read.", file, e);
+        }
+
+        return false;
+
+    }
+
+    /**
+     * Returns whether the given file appears to be a typescript (text
+     * recording of a terminal session). As there is no standard extension for
+     * session recordings, this is determined by testing whether there is an
+     * associated timing file. Guacamole will always include a timing file for
+     * its typescripts.
+     *
+     * @param file
+     *     The file to test.
+     *
+     * @return
+     *     true if the file appears to be a typescript, false otherwise.
+     */
+    private boolean isTypescript(File file) {
+        return new File(file.getAbsolutePath() + TIMING_FILE_SUFFIX).exists();
+    }
+
+    /**
+     * Returns whether the given file appears to be a typescript timing file.
+     * Typescript timing files have the standard extension ".timing".
+     *
+     * @param file
+     *     The file to test.
+     *
+     * @return
+     *     true if the file appears to be a typescript timing file, false
+     *     otherwise.
+     */
+    private boolean isTypescriptTiming(File file) {
+        return file.getName().endsWith(TIMING_FILE_SUFFIX);
+    }
+
+    /**
+     * Returns the type of session recording or log contained within the given
+     * file by inspecting its name and contents.
+     *
+     * @param file
+     *     The file to test.
+     *
+     * @return
+     *     The type of session recording or log contained within the given
+     *     file, or null if this cannot be determined.
+     */
+    private ActivityLog.Type getType(File file) {
+
+        if (isSessionRecording(file))
+            return ActivityLog.Type.GUACAMOLE_SESSION_RECORDING;
+
+        if (isTypescript(file))
+            return ActivityLog.Type.TYPESCRIPT;
+
+        if (isTypescriptTiming(file))
+            return ActivityLog.Type.TYPESCRIPT_TIMING;
+
+        return ActivityLog.Type.SERVER_LOG;
+
+    }
+
+    /**
+     * Returns a new ActivityLog instance representing the session recording or
+     * log contained within the given file. If the type of recording/log cannot
+     * be determined, or if the file is unreadable, null is returned.
+     *
+     * @param file
+     *     The file to produce an ActivityLog instance for.
+     *
+     * @return
+     *     A new ActivityLog instance representing the recording/log contained
+     *     within the given file, or null if the file is unreadable or cannot
+     *     be identified.
+     */
+    private ActivityLog getActivityLog(File file) {
+
+        // Verify file can actually be read
+        if (!file.canRead()) {
+            logger.warn("Ignoring file \"{}\" relevant to connection history "
+                    + "record as it cannot be read.", file);
+            return null;
+        }
+
+        // Determine type of recording/log by inspecting file
+        ActivityLog.Type logType = getType(file);
+        if (logType == null) {
+            logger.warn("Recording/log type of \"{}\" cannot be determined.", 
file);
+            return null;
+        }

Review comment:
       It looks to me like `getType()` will never return `null`, so this should 
actually never be `null`? See comment above about return value.

##########
File path: 
guacamole/src/main/java/org/apache/guacamole/rest/history/ActivityLogResource.java
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.rest.history;
+
+import javax.ws.rs.GET;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.ResponseBuilder;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.net.auth.ActivityLog;
+
+/**
+ * A REST resource which exposes the contents of a given ActivityLog.
+ */
+public class ActivityLogResource {
+
+    /**
+     * The ActivityLog whose contents are being exposed.
+     */
+    private final ActivityLog log;
+
+    /**
+     * Creates a new ActivityRecordSetResource which exposes the records within
+     * the given ActivityRecordSet.
+     *
+     * @param log
+     *     The ActivityLog whose contents should be exposed.
+     */
+    public ActivityLogResource(ActivityLog log) {
+        this.log = log;
+    }
+
+    /**
+     * Returns the raw contents of the underlying ActivityLog. If the size of
+     * the ActivityLog is known, this size is included as the "Content-Length"
+     * of the response.
+     *
+     * @return
+     *     A Response containing the raw contents of the underlying
+     *     ActivityLog.
+     *
+     * @throws GuacamoleException
+     *     If an error prevents retrieving the content of the log or its size.
+     */
+    @GET
+    public Response getContents() throws GuacamoleException {
+
+        // Build base response exposing the raw contents of the underlying log
+        ResponseBuilder response = Response.ok(log.getContent(),
+                log.getType().getContentType());
+
+        // Include size, if knopwn

Review comment:
       Silent "p"?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to