From 7eeb7f9229311a62942f929f8740af349777e2c2 Mon Sep 17 00:00:00 2001 From: frcroth Date: Tue, 20 Sep 2022 15:00:18 +0200 Subject: [PATCH] Short links (#6461) Co-authored-by: Philipp Otto Co-authored-by: Florian M --- .circleci/slack-notification.sh | 1 + CHANGELOG.unreleased.md | 1 + MIGRATIONS.unreleased.md | 1 + app/controllers/ShortLinkController.scala | 36 ++++++ app/models/shortlinks/ShortLink.scala | 55 +++++++++ conf/evolutions/088-shortlinks.sql | 14 +++ conf/evolutions/reversions/088-shortlinks.sql | 7 ++ conf/webknossos.latest.routes | 4 + frontend/javascripts/admin/admin_rest_api.ts | 19 ++++ .../components/{redirect.ts => redirect.tsx} | 9 +- frontend/javascripts/libs/error_handling.ts | 11 +- .../view/action-bar/share_modal_view.tsx | 107 +++++++++++++----- .../share_view_dataset_modal_view.tsx | 24 +--- frontend/javascripts/router.tsx | 14 ++- frontend/javascripts/types/api_flow_types.ts | 6 + tools/postgres/schema.sql | 9 +- 16 files changed, 265 insertions(+), 53 deletions(-) create mode 100644 app/controllers/ShortLinkController.scala create mode 100644 app/models/shortlinks/ShortLink.scala create mode 100644 conf/evolutions/088-shortlinks.sql create mode 100644 conf/evolutions/reversions/088-shortlinks.sql rename frontend/javascripts/components/{redirect.ts => redirect.tsx} (72%) diff --git a/.circleci/slack-notification.sh b/.circleci/slack-notification.sh index 319a8006e1c..5e5e1080b24 100755 --- a/.circleci/slack-notification.sh +++ b/.circleci/slack-notification.sh @@ -18,6 +18,7 @@ if [ "${CIRCLE_BRANCH}" == "master" ] ; then author=${author/youri-k/<@youri>} author=${author/Dagobert42/<@Arthur>} author=${author/leowe/<@leo>} + author=${author/frcroth/<@felix.roth>} channel="webknossos-bots" commitmsg="$(git log --format=%s -n 1)" pullregex="(.*)#([0-9]+)(.*)" diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 34431c75d9f..4cad9fa8b08 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -18,6 +18,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released - The proofreading tool now supports merging and splitting (via min-cut) agglomerates by rightclicking a segment (and not a node). Note that there still has to be an active node so that both partners of the operation are defined. [#6464](https://github.com/scalableminds/webknossos/pull/6464) ### Changed +- Sharing links are shortened by default. Within the sharing modal, this shortening behavior can be disabled. [#6461](https://github.com/scalableminds/webknossos/pull/6461) ### Fixed - Fixed sharing button for users who are currently visiting a dataset or annotation which was shared with them. [#6438](https://github.com/scalableminds/webknossos/pull/6438) diff --git a/MIGRATIONS.unreleased.md b/MIGRATIONS.unreleased.md index 15847635042..2730361a005 100644 --- a/MIGRATIONS.unreleased.md +++ b/MIGRATIONS.unreleased.md @@ -9,3 +9,4 @@ User-facing changes are documented in the [changelog](CHANGELOG.released.md). [Commits](https://github.com/scalableminds/webknossos/compare/22.09.0...HEAD) ### Postgres Evolutions: +- [088-shortlinks.sql](conf/evolutions/088-shortlinks.sql) diff --git a/app/controllers/ShortLinkController.scala b/app/controllers/ShortLinkController.scala new file mode 100644 index 00000000000..17b70f876db --- /dev/null +++ b/app/controllers/ShortLinkController.scala @@ -0,0 +1,36 @@ +package controllers + +import com.mohiva.play.silhouette.api.Silhouette +import com.scalableminds.util.tools.FoxImplicits +import models.shortlinks.{ShortLink, ShortLinkDAO} +import oxalis.security.{RandomIDGenerator, WkEnv} +import play.api.libs.json.Json +import play.api.mvc.{Action, AnyContent, PlayBodyParsers} +import utils.{ObjectId, WkConf} + +import javax.inject.Inject +import scala.concurrent.ExecutionContext + +class ShortLinkController @Inject()(shortLinkDAO: ShortLinkDAO, sil: Silhouette[WkEnv], wkConf: WkConf)( + implicit ec: ExecutionContext, + val bodyParsers: PlayBodyParsers) + extends Controller + with FoxImplicits { + + def create: Action[String] = sil.SecuredAction.async(validateJson[String]) { implicit request => + val longLink = request.body + val _id = ObjectId.generate + val key = RandomIDGenerator.generateBlocking(12) + for { + _ <- bool2Fox(longLink.startsWith(wkConf.Http.uri)) ?~> "Could not generate short link: URI does not match" + _ <- shortLinkDAO.insertOne(ShortLink(_id, key, longLink)) ?~> "create.failed" + inserted <- shortLinkDAO.findOne(_id) + } yield Ok(Json.toJson(inserted)) + } + + def getByKey(key: String): Action[AnyContent] = Action.async { implicit request => + for { + shortLink <- shortLinkDAO.findOneByKey(key) + } yield Ok(Json.toJson(shortLink)) + } +} diff --git a/app/models/shortlinks/ShortLink.scala b/app/models/shortlinks/ShortLink.scala new file mode 100644 index 00000000000..079ebfe82ef --- /dev/null +++ b/app/models/shortlinks/ShortLink.scala @@ -0,0 +1,55 @@ +package models.shortlinks + +import com.scalableminds.util.tools.Fox +import com.scalableminds.webknossos.schema.Tables +import com.scalableminds.webknossos.schema.Tables.{Shortlinks, ShortlinksRow} +import play.api.libs.json.{Json, OFormat} +import slick.jdbc.PostgresProfile.api._ +import slick.lifted.Rep +import utils.{ObjectId, SQLClient, SQLDAO} + +import javax.inject.Inject +import scala.concurrent.ExecutionContext + +case class ShortLink(_id: ObjectId, key: String, longLink: String) + +object ShortLink { + implicit val jsonFormat: OFormat[ShortLink] = Json.format[ShortLink] +} + +class ShortLinkDAO @Inject()(sqlClient: SQLClient)(implicit ec: ExecutionContext) + extends SQLDAO[ShortLink, ShortlinksRow, Shortlinks](sqlClient) { + val collection = Shortlinks + + def idColumn(x: Shortlinks): Rep[String] = x._Id + + override def isDeletedColumn(x: Tables.Shortlinks): Rep[Boolean] = false + + def parse(r: ShortlinksRow): Fox[ShortLink] = + Fox.successful( + ShortLink( + ObjectId(r._Id), + r.key, + r.longlink + ) + ) + + def insertOne(sl: ShortLink): Fox[Unit] = + for { + _ <- run(sqlu"""insert into webknossos.shortLinks(_id, key, longlink) + values(${sl._id}, ${sl.key}, ${sl.longLink})""") + } yield () + + def findOne(id: String): Fox[ShortLink] = + for { + r <- run(sql"select #$columns from webknossos.shortLinks where id = $id".as[ShortlinksRow]) + parsed <- parseFirst(r, id) + } yield parsed + + def findOneByKey(key: String): Fox[ShortLink] = + for { + r <- run(sql"select #$columns from webknossos.shortLinks where key = $key".as[ShortlinksRow]) + parsed <- parseFirst(r, key) + } yield parsed + +} diff --git a/conf/evolutions/088-shortlinks.sql b/conf/evolutions/088-shortlinks.sql new file mode 100644 index 00000000000..b722b9baab0 --- /dev/null +++ b/conf/evolutions/088-shortlinks.sql @@ -0,0 +1,14 @@ +START TRANSACTION; + +CREATE TABLE webknossos.shortLinks( + _id CHAR(24) PRIMARY KEY DEFAULT '', + key CHAR(16) NOT NULL UNIQUE, + longLink Text NOT NULL +); + +CREATE INDEX ON webknossos.shortLinks(key); + +UPDATE webknossos.releaseInformation +SET schemaVersion = 88; + +COMMIT TRANSACTION; diff --git a/conf/evolutions/reversions/088-shortlinks.sql b/conf/evolutions/reversions/088-shortlinks.sql new file mode 100644 index 00000000000..97789b8795b --- /dev/null +++ b/conf/evolutions/reversions/088-shortlinks.sql @@ -0,0 +1,7 @@ +START TRANSACTION; + +DROP TABLE webknossos.shortLinks; + +UPDATE webknossos.releaseInformation SET schemaVersion = 87; + +COMMIT TRANSACTION; diff --git a/conf/webknossos.latest.routes b/conf/webknossos.latest.routes index 54267ec1580..1064cccbbea 100644 --- a/conf/webknossos.latest.routes +++ b/conf/webknossos.latest.routes @@ -237,3 +237,7 @@ POST /jobs/:id/status # Publications GET /publications controllers.PublicationController.listPublications() GET /publications/:id controllers.PublicationController.read(id: String) + +# Shortlinks +POST /shortLinks controllers.ShortLinkController.create +GET /shortLinks/byKey/:key controllers.ShortLinkController.getByKey(key: String) diff --git a/frontend/javascripts/admin/admin_rest_api.ts b/frontend/javascripts/admin/admin_rest_api.ts index 296bb88c9f8..63677416871 100644 --- a/frontend/javascripts/admin/admin_rest_api.ts +++ b/frontend/javascripts/admin/admin_rest_api.ts @@ -59,6 +59,7 @@ import type { ServerEditableMapping, APICompoundType, ZarrPrivateLink, + ShortLink, } from "types/api_flow_types"; import { APIAnnotationTypeEnum } from "types/api_flow_types"; import type { Vector3, Vector6 } from "oxalis/constants"; @@ -2293,3 +2294,21 @@ export async function getEdgesForAgglomerateMinCut( ), ); } + +// ### Short links + +export const createShortLink = _.memoize( + (longLink: string): Promise => + Request.sendJSONReceiveJSON("/api/shortLinks", { + method: "POST", + // stringify is necessary because the back-end expects a JSON string + // (i.e., a string which contains quotes at the beginning and end). + // The Request module does not add additional string quotes + // if the data parameter is already a string. + data: JSON.stringify(longLink), + }), +); + +export function getShortLink(key: string): Promise { + return Request.receiveJSON(`/api/shortLinks/byKey/${key}`); +} diff --git a/frontend/javascripts/components/redirect.ts b/frontend/javascripts/components/redirect.tsx similarity index 72% rename from frontend/javascripts/components/redirect.ts rename to frontend/javascripts/components/redirect.tsx index 3f5befd3e8c..afaeb75627e 100644 --- a/frontend/javascripts/components/redirect.ts +++ b/frontend/javascripts/components/redirect.tsx @@ -1,10 +1,11 @@ import type { RouteComponentProps } from "react-router-dom"; import { withRouter } from "react-router-dom"; import React from "react"; + type Props = { redirectTo: () => Promise; history: RouteComponentProps["history"]; - pushToHistory: boolean; + pushToHistory?: boolean; }; class AsyncRedirect extends React.PureComponent { @@ -19,6 +20,12 @@ class AsyncRedirect extends React.PureComponent { async redirect() { const newPath = await this.props.redirectTo(); + if (newPath.startsWith(location.origin)) { + // The link is absolute which react-router does not support + // apparently. See https://stackoverflow.com/questions/42914666/react-router-external-link + location.replace(newPath); + } + if (this.props.pushToHistory) { this.props.history.push(newPath); } else { diff --git a/frontend/javascripts/libs/error_handling.ts b/frontend/javascripts/libs/error_handling.ts index e91516e4f63..7bcfeba593c 100644 --- a/frontend/javascripts/libs/error_handling.ts +++ b/frontend/javascripts/libs/error_handling.ts @@ -125,9 +125,14 @@ class ErrorHandling { window.addEventListener("unhandledrejection", (event) => { // Create our own error for unhandled rejections here to get additional information for [Object object] errors in airbrake const reasonAsString = event.reason instanceof Error ? event.reason.toString() : event.reason; - const wrappedError = event.reason instanceof Error ? event.reason : new Error(event.reason); - wrappedError.message = - UNHANDLED_REJECTION_PREFIX + JSON.stringify(reasonAsString).slice(0, 80); + let wrappedError = event.reason instanceof Error ? event.reason : new Error(event.reason); + wrappedError = { + ...wrappedError, + // The message property is read-only in newer browser versions which is why + // the object is copied shallowly. + message: UNHANDLED_REJECTION_PREFIX + JSON.stringify(reasonAsString).slice(0, 80), + }; + this.notify(wrappedError, { originalError: reasonAsString, }); diff --git a/frontend/javascripts/oxalis/view/action-bar/share_modal_view.tsx b/frontend/javascripts/oxalis/view/action-bar/share_modal_view.tsx index 4c69db52059..65bf73a60c5 100644 --- a/frontend/javascripts/oxalis/view/action-bar/share_modal_view.tsx +++ b/frontend/javascripts/oxalis/view/action-bar/share_modal_view.tsx @@ -1,6 +1,16 @@ -import { Alert, Divider, Radio, Modal, Input, Button, Row, Col, RadioChangeEvent } from "antd"; -import { CopyOutlined, ShareAltOutlined } from "@ant-design/icons"; -import ButtonComponent from "oxalis/view/components/button_component"; +import { + Alert, + Divider, + Radio, + Modal, + Input, + Button, + Row, + Col, + RadioChangeEvent, + Tooltip, +} from "antd"; +import { CompressOutlined, CopyOutlined, ShareAltOutlined } from "@ant-design/icons"; import { useSelector } from "react-redux"; import React, { useState, useEffect } from "react"; import type { @@ -17,6 +27,7 @@ import { sendAnalyticsEvent, setOthersMayEditForAnnotation, getSharingTokenFromUrlParameters, + createShortLink, } from "admin/admin_rest_api"; import TeamSelectionComponent from "dashboard/dataset/team_selection_component"; import Toast from "libs/toast"; @@ -32,6 +43,7 @@ import { import { setShareModalVisibilityAction } from "oxalis/model/actions/ui_actions"; import { ControlModeEnum } from "oxalis/constants"; import { makeComponentLazy } from "libs/react_helpers"; +import { AsyncButton } from "components/async_clickables"; const RadioGroup = Radio.Group; const sharingActiveNode = true; @@ -115,10 +127,12 @@ export function ShareButton(props: { dataset: APIDataset; style?: Record { + const createAndCopySharingUrl = async () => { // Copy the url on-demand as it constantly changes const url = getUrl(sharingToken, includeToken); - copyUrlToClipboard(url); + const shortLink = await createShortLink(url); + + copyUrlToClipboard(`${location.origin}/links/${shortLink.key}`); if (isTraceMode && !annotationIsPublic) { // For public annotations and in dataset view mode, the link will work for all users. @@ -144,10 +158,10 @@ export function ShareButton(props: { dataset: APIDataset; style?: Record} title={messages["tracing.copy_sharing_link"]} - onClick={copySharingUrl} + onClick={createAndCopySharingUrl} style={style} /> ); @@ -314,7 +328,8 @@ function _ShareModalView(props: Props) { Private: "lock", }; const includeToken = !dataset.isPublic && visibility === "Public"; - const url = getUrl(sharingToken, includeToken); + const longUrl = getUrl(sharingToken, includeToken); + return ( - - - - + {messages["tracing.sharing_modal_basic_information"](sharingActiveNode)} @@ -503,5 +501,60 @@ function _ShareModalView(props: Props) { ); } +export function CopyableSharingLink({ + isVisible, + longUrl, +}: { + isVisible: boolean; + longUrl: string; +}) { + const [shortUrl, setShortUrl] = useState(null); + + const [showShortLink, setShowShortLink] = useState(true); + useEffect(() => { + if (!isVisible || !showShortLink) { + // Don't create new shortlinks when the user does not want/need them. + // Set short url to null to avoid keeping a stale value. + setShortUrl(null); + return; + } + + createShortLink(longUrl).then((shortLink) => { + setShortUrl(`${location.origin}/links/${shortLink.key}`); + }); + }, [longUrl, isVisible, showShortLink]); + const linkToCopy = showShortLink ? shortUrl || longUrl : longUrl; + + return ( + + + + + + + + ); +} + const ShareModalView = makeComponentLazy(_ShareModalView); export default ShareModalView; diff --git a/frontend/javascripts/oxalis/view/action-bar/share_view_dataset_modal_view.tsx b/frontend/javascripts/oxalis/view/action-bar/share_view_dataset_modal_view.tsx index a58ec2892ad..256d20a021c 100644 --- a/frontend/javascripts/oxalis/view/action-bar/share_view_dataset_modal_view.tsx +++ b/frontend/javascripts/oxalis/view/action-bar/share_view_dataset_modal_view.tsx @@ -5,7 +5,7 @@ import React from "react"; import { makeComponentLazy } from "libs/react_helpers"; import messages from "messages"; import { OxalisState } from "oxalis/store"; -import { useDatasetSharingToken, getUrl, copyUrlToClipboard } from "./share_modal_view"; +import { useDatasetSharingToken, getUrl, CopyableSharingLink } from "./share_modal_view"; import { useZarrLinkMenu } from "./private_links_view"; const sharingActiveNode = false; @@ -19,7 +19,7 @@ function _ShareViewDatasetModalView(props: Props) { const { isVisible, onOk } = props; const dataset = useSelector((state: OxalisState) => state.dataset); const sharingToken = useDatasetSharingToken(dataset); - const url = getUrl(sharingToken, !dataset.isPublic); + const longUrl = getUrl(sharingToken, !dataset.isPublic); const { baseUrl: zarrBaseUrl, copyLayerUrlMenu } = useZarrLinkMenu(null); @@ -42,24 +42,8 @@ function _ShareViewDatasetModalView(props: Props) { Sharing Link - - - - + +
{ ( - // @ts-expect-error ts-migrate(2322) FIXME: Type '{ redirectTo: () => Promise; }' is n... Remove this comment to see the full error message { const datasetName = match.params.id || ""; @@ -606,6 +606,18 @@ class ReactRouter extends React.Component { + ( + { + const key = match.params.key || ""; + const shortLink = await getShortLink(key); + return shortLink.longLink; + }} + /> + )} + /> {!features().isDemoInstance && } diff --git a/frontend/javascripts/types/api_flow_types.ts b/frontend/javascripts/types/api_flow_types.ts index d0eccca4f6c..5e6ccfb673b 100644 --- a/frontend/javascripts/types/api_flow_types.ts +++ b/frontend/javascripts/types/api_flow_types.ts @@ -702,3 +702,9 @@ export type ZarrPrivateLink = { accessToken: string; expirationDateTime: number | null; }; + +export type ShortLink = { + longLink: string; + key: string; + _id: string; +}; diff --git a/tools/postgres/schema.sql b/tools/postgres/schema.sql index ad79ac08f88..21746f7a0a1 100644 --- a/tools/postgres/schema.sql +++ b/tools/postgres/schema.sql @@ -19,7 +19,7 @@ START TRANSACTION; CREATE TABLE webknossos.releaseInformation ( schemaVersion BIGINT NOT NULL ); -INSERT INTO webknossos.releaseInformation(schemaVersion) values(87); +INSERT INTO webknossos.releaseInformation(schemaVersion) values(88); COMMIT TRANSACTION; @@ -426,6 +426,12 @@ CREATE TABLE webknossos.annotation_privateLinks( isDeleted BOOLEAN NOT NULL DEFAULT false ); +CREATE TABLE webknossos.shortLinks( + _id CHAR(24) PRIMARY KEY DEFAULT '', + key CHAR(16) NOT NULL UNIQUE, + longLink Text NOT NULL +); + CREATE VIEW webknossos.annotations_ AS SELECT * FROM webknossos.annotations WHERE NOT isDeleted; CREATE VIEW webknossos.meshes_ AS SELECT * FROM webknossos.meshes WHERE NOT isDeleted; @@ -482,6 +488,7 @@ CREATE INDEX ON webknossos.projects(name, isDeleted); CREATE INDEX ON webknossos.projects(_team, isDeleted); CREATE INDEX ON webknossos.invites(tokenValue); CREATE INDEX ON webknossos.annotation_privateLinks(accessToken); +CREATE INDEX ON webknossos.shortLinks(key); ALTER TABLE webknossos.annotations ADD CONSTRAINT task_ref FOREIGN KEY(_task) REFERENCES webknossos.tasks(_id) ON DELETE SET NULL DEFERRABLE,