Skip to content

Commit

Permalink
Merge pull request #700 from mozilla/promotion_entity_state_api
Browse files Browse the repository at this point in the history
Storage promotion/demotion and legacy object unpinning for entity state API
  • Loading branch information
johnshaughnessy authored Jul 17, 2023
2 parents 56ff5a8 + 1ee31ea commit 77e39ca
Show file tree
Hide file tree
Showing 6 changed files with 305 additions and 13 deletions.
16 changes: 15 additions & 1 deletion lib/ret/owned_file.ex
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,24 @@ defmodule Ret.OwnedFile do

def set_active(owned_file_uuid, account_id) do
get_by_uuid_and_account(owned_file_uuid, account_id) |> set_state(:active)

case get_by_uuid_and_account(owned_file_uuid, account_id) do
nil ->
{:error, :file_not_found}

owned_file ->
set_state(owned_file, :active)
end
end

def set_inactive(owned_file_uuid, account_id) do
get_by_uuid_and_account(owned_file_uuid, account_id) |> set_state(:inactive)
case get_by_uuid_and_account(owned_file_uuid, account_id) do
nil ->
{:error, :file_not_found}

owned_file ->
set_state(owned_file, :inactive)
end
end

def set_inactive(%OwnedFile{} = owned_file) do
Expand Down
6 changes: 5 additions & 1 deletion lib/ret/storage.ex
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,10 @@ defmodule Ret.Storage do
end
end

def promote(_id, nil, nil, _account) do
{:error, :not_allowed}
end

def promote(nil, _key, _promotion_token, _account) do
{:error, :not_found}
end
Expand Down Expand Up @@ -329,7 +333,7 @@ defmodule Ret.Storage do
end
end

# If an owned file does not have a promotion token associated with it, it can be promoted with any given
# If an owned file does not have a promotion token associated with it, it can be promoted with any given
# promotion token, including nil.
defp check_promotion_token(nil, _token), do: {:ok}
defp check_promotion_token(actual_token, token) when actual_token == token, do: {:ok}
Expand Down
67 changes: 57 additions & 10 deletions lib/ret_web/channels/hub_channel.ex
Original file line number Diff line number Diff line change
Expand Up @@ -467,10 +467,17 @@ defmodule RetWeb.HubChannel do
def handle_in("save_entity_state", params, socket) do
params = parse(params)

with {:ok, hub} <- authorize(socket, :write_entity_state),
{:ok, %{entity: entity}} <- Ret.create_entity(hub, params) do
with {:ok, hub, account} <- authorize(socket, :write_entity_state),
{:ok, %{entity: entity}} <- Ret.create_entity(hub, params),
:ok <- maybe_promote_file(params, account, socket) do
entity = Repo.preload(entity, [:sub_entities])
broadcast!(socket, "entity_state_saved", EntityView.render("show.json", %{entity: entity}))

broadcast!(
socket,
"entity_state_saved",
EntityView.render("show.json", %{entity: entity})
)

{:reply, :ok, socket}
else
{:error, reason} ->
Expand All @@ -481,7 +488,7 @@ defmodule RetWeb.HubChannel do
def handle_in("update_entity_state", %{"update_message" => update_message} = params, socket) do
params = parse(params)

with {:ok, hub} <- authorize(socket, :write_entity_state),
with {:ok, hub, _account} <- authorize(socket, :write_entity_state),
{:ok, _} <- Ret.insert_or_update_sub_entity(hub, params) do
broadcast!(socket, "entity_state_updated", update_message)
{:reply, :ok, socket}
Expand All @@ -491,9 +498,12 @@ defmodule RetWeb.HubChannel do
end
end

def handle_in("delete_entity_state", %{"nid" => nid}, socket) do
with {:ok, hub} <- authorize(socket, :write_entity_state),
{:ok, _} <- Ret.delete_entity(hub.hub_id, nid) do
def handle_in("delete_entity_state", %{"nid" => nid} = payload, socket) do
with {:ok, hub, account} <- authorize(socket, :write_entity_state),
{:ok, _} <- Ret.delete_entity(hub.hub_id, nid),
{:ok, _} <- maybe_set_owned_file_inactive(payload, account) do
RoomObject.perform_unpin(hub, nid)

broadcast!(socket, "entity_state_deleted", %{
"nid" => nid,
"creator" => socket.assigns.session_id
Expand Down Expand Up @@ -950,7 +960,7 @@ defmodule RetWeb.HubChannel do

with {:ok, account} <- account_for_socket(socket),
:ok <- auth(hub, account, :write_entity_state) do
{:ok, hub}
{:ok, hub, account}
end
end

Expand Down Expand Up @@ -1445,14 +1455,51 @@ defmodule RetWeb.HubChannel do
%{nid: nid, root_nid: root_nid, update_message: Jason.encode!(update_message)}
end

defp parse(%{"nid" => nid, "create_message" => create_message, "updates" => updates}) do
defp parse(
%{
"nid" => nid,
"create_message" => create_message,
"updates" => updates
} = params
) do
%{
nid: nid,
create_message: Jason.encode!(create_message),
updates: Enum.map(updates, &parse/1)
updates: Enum.map(updates, &parse/1),
promotion_token: Map.get(params, "promotion_token", nil),
file_id: Map.get(params, "file_id", nil),
file_access_token: Map.get(params, "file_access_token", nil)
}
end

defp maybe_set_owned_file_inactive(%{"file_id" => file_id}, %Account{account_id: account_id}) do
OwnedFile.set_inactive(file_id, account_id)
end

defp maybe_set_owned_file_inactive(_payload, _account) do
{:ok, :no_file}
end

defp maybe_promote_file(%{file_id: nil} = _params, _account, _socket) do
:ok
end

defp maybe_promote_file(params, account, _socket) do
with {:ok, _owned_file} <-
Storage.promote(
params.file_id,
params.file_access_token,
params.promotion_token,
account
) do
OwnedFile.set_active(params.file_id, account.account_id)
:ok
else
{:error, reason} ->
{:error, reason}
end
end

defp reply_error(socket, reason) do
{:reply, {:error, %{reason: reason}}, socket}
end
Expand Down
121 changes: 120 additions & 1 deletion test/ret_web/channels/entity_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ defmodule RetWeb.EntityTest do
import RetWeb.EntityTestUtils, only: [read_json: 1]

alias RetWeb.SessionSocket
alias Ret.{Repo, HubRoleMembership}
alias Ret.{Repo, HubRoleMembership, Storage}

@payload_save_entity_state read_json("save_entity_state_payload.json")
@payload_save_entity_state_2 read_json("save_entity_state_payload_2.json")
@payload_save_entity_state_promotable read_json("save_entity_state_payload_promotable.json")
@payload_save_entity_state_unpromotable read_json("save_entity_state_payload_unpromotable.json")
@payload_update_entity_state read_json("update_entity_state_payload.json")
@payload_delete_entity_state read_json("delete_entity_state_payload.json")
@default_join_params %{"profile" => %{}, "context" => %{}}
Expand Down Expand Up @@ -70,6 +72,52 @@ defmodule RetWeb.EntityTest do
reason: :unauthorized
}
end

test "save_entity_state succeeds if provided correct promotion keys", %{
socket: socket,
hub: hub,
account: account
} do
%HubRoleMembership{hub: hub, account: account} |> Repo.insert!()
temp_file = generate_temp_file("test")

{:ok, _, socket} =
subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(account))

{:ok, uuid} = Storage.store(%Plug.Upload{path: temp_file}, "text/plain", "secret")

updated_map =
@payload_save_entity_state_promotable
|> Map.put("file_id", uuid)
|> Map.put("file_access_token", "secret")

assert_reply push(socket, "save_entity_state", updated_map), :ok
end

test "save_entity_state fails if provided incorrect promotion parameters", %{
socket: socket,
hub: hub,
account: account
} do
%HubRoleMembership{hub: hub, account: account} |> Repo.insert!()
temp_file = generate_temp_file("test2")

{:ok, _, socket} =
subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(account))

{:ok, uuid} = Storage.store(%Plug.Upload{path: temp_file}, "text/plain", "secret")

updated_map =
@payload_save_entity_state_unpromotable
|> Map.put("file_id", uuid)
|> Map.put("file_access_token", " not_secret")

assert_reply push(socket, "save_entity_state", updated_map),
:error,
%{
reason: :not_allowed
}
end
end

test "update_entity_state overwrites previous entity state", %{
Expand All @@ -94,6 +142,77 @@ defmodule RetWeb.EntityTest do
1
end

test "delete_entity_state replies with error if entity does not exist", %{
socket: socket,
hub: hub,
account: account
} do
%HubRoleMembership{hub: hub, account: account} |> Repo.insert!()

{:ok, _, socket} =
subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(account))

non_existent_entity_payload = Map.put(@payload_delete_entity_state, "nid", "non_existent_nid")

assert_reply push(socket, "delete_entity_state", non_existent_entity_payload), :error, %{
reason: :entity_state_does_not_exist
}
end

test "delete_entity_state replies with error if owned file does not exist", %{
socket: socket,
hub: hub,
account: account
} do
%HubRoleMembership{hub: hub, account: account} |> Repo.insert!()

{:ok, _, socket} =
subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(account))

push(socket, "save_entity_state", @payload_save_entity_state)

non_existent_file_payload =
Map.put(@payload_delete_entity_state, "file_id", "non_existent_file_id")

assert_reply push(socket, "delete_entity_state", non_existent_file_payload), :error, %{
reason: :file_not_found
}
end

test "delete_entity_state replies with error if not authorized", %{
socket: socket,
hub: hub
} do
{:ok, _, socket} = subscribe_and_join(socket, "hub:#{hub.hub_sid}", @default_join_params)

assert_reply push(socket, "delete_entity_state", @payload_delete_entity_state), :error, %{
reason: :not_logged_in
}
end

test "delete_entity_state deletes the entity and deactivates the owned file if file_id is present",
%{
socket: socket,
hub: hub,
account: account,
owned_file: owned_file
} do
%HubRoleMembership{hub: hub, account: account} |> Repo.insert!()

{:ok, _, socket} =
subscribe_and_join(socket, "hub:#{hub.hub_sid}", join_params_for_account(account))

push(socket, "save_entity_state", @payload_save_entity_state)

payload_with_file_id =
Map.put(@payload_delete_entity_state, "file_id", owned_file.owned_file_uuid)

assert_reply push(socket, "delete_entity_state", payload_with_file_id), :ok

updated_file = owned_file |> Repo.reload()
assert updated_file.state == :inactive
end

test "delete_entity_state deletes the entity with the matching nid", %{
socket: socket,
hub: hub,
Expand Down
54 changes: 54 additions & 0 deletions test/support/utils/save_entity_state_payload_promotable.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
{
"nid": "hvpg97x",
"create_message": {
"version": 1,
"networkId": "hvpg97x",
"prefabName": "media",
"initialData": {
"src": "https://hubs.local:4000/files/101ac987-61f4-4645-b512-def3f0336fdf.jpg?token=2bbfa5ff552a159cff50b8bc212a477c",
"recenter": true,
"resize": true,
"animateLoad": true,
"fileId": "101ac987-61f4-4645-b512-def3f0336fdf",
"isObjectMenuTarget": true
}
},
"updates": [
{
"root_nid": "hvpg97x",
"nid": "hvpg97x",
"update_message": {
"nid": "hvpg97x",
"lastOwnerTime": 1157850816,
"timestamp": 1157852116,
"owner": "9eac3551-20d7-4b04-80fc-2009b5e8fe10",
"data": {
"networked-transform": {
"version": 1,
"data": {
"position": [
44.335304260253906,
1.4967195987701416,
-6.183996677398682
],
"rotation": [
-0.03446304053068161,
0.6161360144615173,
0.026999054476618767,
0.7864221334457397
],
"scale": [
1,
1,
1
]
}
}
}
}
}
],
"file_id": "101ac987-61f4-4645-b512-def3f0336fdf",
"file_access_token": "2bbfa5ff552a159cff50b8bc212a477c",
"promotion_token": "1701f2fc68081168c7b19ebfabc76dc6"
}
Loading

0 comments on commit 77e39ca

Please sign in to comment.