You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by mike-jumper <gi...@git.apache.org> on 2017/09/25 20:02:35 UTC
[GitHub] incubator-guacamole-client pull request #191: GUACAMOLE-394: Refactor extens...
GitHub user mike-jumper opened a pull request:
https://github.com/apache/incubator-guacamole-client/pull/191
GUACAMOLE-394: Refactor extension API to define user history
This change:
* Extracts an `ActivityRecord` base interface from the `ConnectionRecord` interface, leveraging that to represent both user and connection history entries.
* Deprecates the connection-specific `ConnectionRecordSet` interface in favor of a new, generic `ActivityRecordSet` interface.
* Adds history retrieval functions for `User` objects, analogous to those already available for `Connection` objects.
* Adds a `getLastActive()` function to both `User` and `Connection` with similar semantics for each.
Existing extensions have been updated accordingly, though the database auth backend has not been updated to actually record these history entries.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mike-jumper/incubator-guacamole-client user-history-api
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-guacamole-client/pull/191.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #191
----
commit cd5d23866db316e5835cd7fcd5fcc3a9c9a3a67e
Author: Michael Jumper <mj...@apache.org>
Date: 2017-09-05T20:45:45Z
GUACAMOLE-394: Separate definition of records and record sets into generalized interface.
commit 26122ebc3eb07934264618c319f93738a230a973
Author: Michael Jumper <mj...@apache.org>
Date: 2017-09-09T20:20:43Z
GUACAMOLE-394: Deprecate ConnectionRecordSet. Refactor accordingly.
commit 5340f553616108283f399a811dc1eace14aa92d2
Author: Michael Jumper <mj...@apache.org>
Date: 2017-09-09T20:43:49Z
GUACAMOLE-394: Add API support for user login/logout records.
commit 700005e8238ec1cba18feb00c98c3a9997380811
Author: Michael Jumper <mj...@apache.org>
Date: 2017-09-12T01:20:53Z
GUACAMOLE-394: Remove UserRecord interface - recording historical auth tokens doesn't make sense, and removing that turns UserRecord into an empty interface.
commit b61f14e4db06bf2d2dfc1fd7e2098e0d96618b82
Author: Michael Jumper <mj...@apache.org>
Date: 2017-09-12T01:33:49Z
GUACAMOLE-394: Add history list at User object level (similar to Connection).
commit 3cd7f453c0a9ba8abd69c76cce8da8a917c0021e
Author: Michael Jumper <mj...@apache.org>
Date: 2017-09-12T01:49:11Z
GUACAMOLE-394: Add getLastActive() function, returning the time that a user/connection was last logged-in / used.
----
---
[GitHub] incubator-guacamole-client pull request #191: GUACAMOLE-394: Refactor extens...
Posted by jmuehlner <gi...@git.apache.org>.
Github user jmuehlner commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/191#discussion_r140896419
--- Diff: guacamole-ext/src/main/java/org/apache/guacamole/net/auth/ActivityRecordSet.java ---
@@ -0,0 +1,128 @@
+/*
+ * 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.net.auth;
+
+import java.util.Collection;
+import org.apache.guacamole.GuacamoleException;
+
+/**
+ * A set of all available records related to a type of activity which has a
+ * defined start and end time, such as a user being logged in or connected, or a
+ * subset of those records.
+ *
+ * @param <RecordType>
+ * The type of ActivityRecord contained within this set.
+ */
+public interface ActivityRecordSet<RecordType extends ActivityRecord> {
+
+ /**
+ * All properties of activity records which can be used as sorting
+ * criteria.
+ */
+ enum SortableProperty {
+
+ /**
+ * The date and time when the activity associated with the record
+ * began.
+ */
+ START_DATE
+
+ };
+
+ /**
+ * Returns all records within this set as a standard Collection.
+ *
+ * @return
+ * A collection containing all records within this set.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while retrieving the records within this set.
+ */
+ Collection<RecordType> asCollection() throws GuacamoleException;
+
+ /**
+ * Returns the subset of records which contain the given value. The
+ * properties and semantics involved with determining whether a particular
+ * record "contains" the given value is implementation dependent. This
--- End diff --
It doesn't look like any implementations actually mutate the `ActivityRecordSet` when `contains()` is called. Do you expect that this will be the case? If so, why?
---
[GitHub] incubator-guacamole-client pull request #191: GUACAMOLE-394: Refactor extens...
Posted by jmuehlner <gi...@git.apache.org>.
Github user jmuehlner commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/191#discussion_r141450769
--- Diff: guacamole-ext/src/main/java/org/apache/guacamole/net/auth/ActivityRecordSet.java ---
@@ -0,0 +1,128 @@
+/*
+ * 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.net.auth;
+
+import java.util.Collection;
+import org.apache.guacamole.GuacamoleException;
+
+/**
+ * A set of all available records related to a type of activity which has a
+ * defined start and end time, such as a user being logged in or connected, or a
+ * subset of those records.
+ *
+ * @param <RecordType>
+ * The type of ActivityRecord contained within this set.
+ */
+public interface ActivityRecordSet<RecordType extends ActivityRecord> {
+
+ /**
+ * All properties of activity records which can be used as sorting
+ * criteria.
+ */
+ enum SortableProperty {
+
+ /**
+ * The date and time when the activity associated with the record
+ * began.
+ */
+ START_DATE
+
+ };
+
+ /**
+ * Returns all records within this set as a standard Collection.
+ *
+ * @return
+ * A collection containing all records within this set.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while retrieving the records within this set.
+ */
+ Collection<RecordType> asCollection() throws GuacamoleException;
+
+ /**
+ * Returns the subset of records which contain the given value. The
+ * properties and semantics involved with determining whether a particular
+ * record "contains" the given value is implementation dependent. This
--- End diff --
Ahhh ok. I gotcha. This makes sense to me.
---
[GitHub] incubator-guacamole-client pull request #191: GUACAMOLE-394: Refactor extens...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/191#discussion_r140905330
--- Diff: guacamole-ext/src/main/java/org/apache/guacamole/net/auth/ActivityRecordSet.java ---
@@ -0,0 +1,128 @@
+/*
+ * 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.net.auth;
+
+import java.util.Collection;
+import org.apache.guacamole.GuacamoleException;
+
+/**
+ * A set of all available records related to a type of activity which has a
+ * defined start and end time, such as a user being logged in or connected, or a
+ * subset of those records.
+ *
+ * @param <RecordType>
+ * The type of ActivityRecord contained within this set.
+ */
+public interface ActivityRecordSet<RecordType extends ActivityRecord> {
+
+ /**
+ * All properties of activity records which can be used as sorting
+ * criteria.
+ */
+ enum SortableProperty {
+
+ /**
+ * The date and time when the activity associated with the record
+ * began.
+ */
+ START_DATE
+
+ };
+
+ /**
+ * Returns all records within this set as a standard Collection.
+ *
+ * @return
+ * A collection containing all records within this set.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while retrieving the records within this set.
+ */
+ Collection<RecordType> asCollection() throws GuacamoleException;
+
+ /**
+ * Returns the subset of records which contain the given value. The
+ * properties and semantics involved with determining whether a particular
+ * record "contains" the given value is implementation dependent. This
--- End diff --
Yep - since most of the `ActivityRecordSet` functions return a possibly-new `ActivityRecordSet` based off the original, it's likely that explicitly allowing these functions to mutate the original will simplify implementations, as well as free implementations to make use of optimizations that rely on shared state.
The JDBC version actually does mutate the original:
https://github.com/apache/incubator-guacamole-client/blob/1c0ee41d0ecd5bc4a3550804b74b73b901e074c2/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/connection/ConnectionRecordSet.java#L77-L82
---
[GitHub] incubator-guacamole-client pull request #191: GUACAMOLE-394: Refactor extens...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-guacamole-client/pull/191
---