refactor(media): use UUID instead of ID for media retrieval in GraphQL

Fixes #1760
This commit is contained in:
Massedil
2025-05-21 23:05:09 +02:00
committed by setop
parent 1b2c55508e
commit a50681c9ac
27 changed files with 171 additions and 103 deletions

View File

@@ -54,7 +54,7 @@ defmodule Mobilizon.GraphQL.API.Events do
end
defp process_picture(nil, _), do: nil
defp process_picture(%{media_id: _picture_id} = args, _), do: args
defp process_picture(%{media_uuid: _picture_id} = args, _), do: args
defp process_picture(%{media: media}, %Actor{id: actor_id}) do
# case url

View File

@@ -30,6 +30,16 @@ defmodule Mobilizon.GraphQL.Resolvers.Media do
{:ok, Enum.map(medias, &transform_media/1)}
end
def get_media_by_uuid(_parent, %{uuid: uuid}, _resolution) do
case Medias.get_media_by_uuid(uuid) do
%Media{} = media ->
{:ok, transform_media(media)}
nil ->
{:error, :not_found}
end
end
@spec do_fetch_media(nil) :: {:error, nil}
defp do_fetch_media(nil), do: {:error, nil}
@@ -103,6 +113,22 @@ defmodule Mobilizon.GraphQL.Resolvers.Media do
def remove_media(_parent, _args, _resolution), do: {:error, :unauthenticated}
def remove_media_by_uuid(_parent, %{uuid: media_uuid}, %{
context: %{current_user: %User{} = user}
}) do
with {:media, %Media{actor_id: actor_id} = media} <-
{:media, Medias.get_media_by_uuid(media_uuid)},
{:is_owned, %Actor{} = _actor} <- User.owns_actor(user, actor_id) do
Medias.delete_media(media)
else
{:media, nil} -> {:error, :not_found}
{:is_owned, _} -> {:error, :unauthorized}
{:error, :enofile} -> {:error, "File not found"}
end
end
def remove_media_by_uuid(_parent, _args, _resolution), do: {:error, :unauthenticated}
@doc """
Return the total media size for an actor
"""
@@ -140,11 +166,12 @@ defmodule Mobilizon.GraphQL.Resolvers.Media do
@spec transform_media(Media.t() | nil) :: map() | nil
def transform_media(nil), do: nil
def transform_media(%Media{id: id, file: file, metadata: metadata}) do
def transform_media(%Media{id: id, uuid: uuid, file: file, metadata: metadata}) do
%{
name: file.name,
url: file.url,
id: id,
uuid: uuid,
content_type: file.content_type,
size: file.size,
metadata: metadata

View File

@@ -63,6 +63,12 @@ defmodule Mobilizon.GraphQL.Schema do
field(:id, :id)
end
@desc "A struct containing the uuid of the deleted object"
object :deleted_object_by_uuid do
meta(:authorize, :all)
field(:uuid, :uuid)
end
@desc "A JWT and the associated user ID"
object :login do
meta(:authorize, :all)

View File

@@ -12,7 +12,7 @@ defmodule Mobilizon.GraphQL.Schema.MediaType do
@desc "A media"
object :media do
meta(:authorize, :all)
field(:id, :id, description: "The media's ID")
field(:uuid, :uuid, description: "The media's UUID")
field(:alt, :string, description: "The media's alternative text")
field(:name, :string, description: "The media's name")
field(:url, :string, description: "The media's full URL")
@@ -44,8 +44,8 @@ defmodule Mobilizon.GraphQL.Schema.MediaType do
input_object :media_input do
# Either a full media object
field(:media, :media_input_object, description: "A full media attached")
# Or directly the ID of an existing media
field(:media_id, :id, description: "The ID of an existing media")
# Or directly the UUID of an existing media
field(:media_uuid, :uuid, description: "The UUID of an existing media")
end
@desc "An attached media"
@@ -58,11 +58,11 @@ defmodule Mobilizon.GraphQL.Schema.MediaType do
end
object :media_queries do
@desc "Get a media"
@desc "Get a media by uuid"
field :media, :media do
arg(:id, non_null(:id), description: "The media ID")
arg(:uuid, non_null(:uuid), description: "The media UUID")
middleware(Rajska.QueryAuthorization, permit: :all)
resolve(&Media.media/3)
resolve(&Media.get_media_by_uuid/3)
end
end
@@ -89,16 +89,17 @@ defmodule Mobilizon.GraphQL.Schema.MediaType do
@desc """
Remove a media
"""
field :remove_media, :deleted_object do
arg(:id, non_null(:id), description: "The media's ID")
field :remove_media, :deleted_object_by_uuid do
arg(:uuid, non_null(:uuid), description: "The media's UUID")
middleware(Rajska.QueryAuthorization,
permit: :user,
scope: Mobilizon.Medias.Media,
rule: :"write:media:remove"
rule: :"write:media:remove",
args: :uuid
)
resolve(&Media.remove_media/3)
resolve(&Media.remove_media_by_uuid/3)
end
end

View File

@@ -258,9 +258,14 @@ defmodule Mobilizon.Events.Event do
# In case the provided picture is an existing one
@spec put_picture(Changeset.t(), map) :: Changeset.t()
defp put_picture(%Changeset{} = changeset, %{picture: %{media_id: id} = _picture}) do
%Media{} = picture = Medias.get_media!(id)
put_assoc(changeset, :picture, picture)
defp put_picture(%Changeset{} = changeset, %{picture: %{media_uuid: uuid} = _picture}) do
case Medias.get_media_by_uuid(uuid) do
%Media{} = picture ->
put_assoc(changeset, :picture, picture)
nil ->
add_error(changeset, :picture, "Media with UUID #{uuid} not found")
end
end
# In case it's a new picture

View File

@@ -5,7 +5,7 @@ defmodule Mobilizon.Medias.Media do
use Ecto.Schema
import Ecto.Changeset, only: [cast: 3, cast_embed: 2]
import Ecto.Changeset
alias Mobilizon.Actors.Actor
alias Mobilizon.Discussions.Comment
@@ -15,14 +15,19 @@ defmodule Mobilizon.Medias.Media do
alias Mobilizon.Posts.Post
@type t :: %__MODULE__{
uuid: Ecto.UUID.t(),
file: File.t(),
metadata: Metadata.t(),
actor: Actor.t()
}
@attrs [:actor_id]
@attrs [:actor_id, :uuid]
schema "medias" do
# We need read_after_writes because the uuid is generated by
# the database gen_random_uuid() function
field(:uuid, Ecto.UUID, read_after_writes: true)
embeds_one(:file, File, on_replace: :update)
embeds_one(:metadata, Metadata, on_replace: :update)
@@ -44,5 +49,6 @@ defmodule Mobilizon.Medias.Media do
|> cast(attrs, @attrs)
|> cast_embed(:file)
|> cast_embed(:metadata)
|> unique_constraint(:uuid)
end
end

View File

@@ -29,6 +29,12 @@ defmodule Mobilizon.Medias do
@spec get_media!(integer | String.t()) :: Media.t()
def get_media!(id), do: Repo.get!(Media, id)
@doc """
Get a single media by uuid.
"""
@spec get_media_by_uuid(String.t()) :: Media.t() | nil
def get_media_by_uuid(uuid), do: Repo.get_by(Media, uuid: uuid)
@doc """
Get a media by its URL.
"""

View File

@@ -0,0 +1,12 @@
defmodule Mobilizon.Storage.Repo.Migrations.AddUuidToMedia do
use Ecto.Migration
def change do
# Create the new uuid for medias
alter table(:medias) do
add :uuid, :uuid, default: fragment("gen_random_uuid()"), null: false
end
create(unique_index(:medias, [:uuid]))
end
end

View File

@@ -4,7 +4,7 @@ export const ACTOR_FRAGMENT = gql`
fragment ActorFragment on Actor {
id
avatar {
id
uuid
url
}
type
@@ -26,12 +26,12 @@ export const FETCH_PERSON = gql`
...ActorFragment
suspended
avatar {
id
uuid
name
url
}
banner {
id
uuid
url
}
feedTokens {
@@ -57,12 +57,12 @@ export const GET_PERSON = gql`
suspended
mediaSize
avatar {
id
uuid
name
url
}
banner {
id
uuid
url
}
feedTokens {
@@ -118,7 +118,7 @@ export const PERSON_FRAGMENT = gql`
fragment PersonFragment on Person {
id
avatar {
id
uuid
url
}
type
@@ -215,7 +215,7 @@ export const LOGGED_USER_DRAFTS = gql`
title
draft
picture {
id
uuid
url
alt
}
@@ -266,7 +266,7 @@ export const LOGGED_USER_MEMBERSHIPS = gql`
id
title
picture {
id
uuid
url
}
}

View File

@@ -14,7 +14,7 @@ export const DASHBOARD = gql`
...EventOptions
}
picture {
id
uuid
alt
url
}
@@ -201,17 +201,17 @@ export const ADMIN_SETTINGS_FRAGMENT = gql`
instanceSlogan
contact
instanceLogo {
id
uuid
url
name
}
instanceFavicon {
id
uuid
url
name
}
defaultPicture {
id
uuid
url
name
}

View File

@@ -20,7 +20,7 @@ export const CONFIG = gql`
url
}
defaultPicture {
id
uuid
url
name
metadata {
@@ -502,7 +502,7 @@ export const DEFAULT_PICTURE = gql`
query DefaultPicture {
config {
defaultPicture {
id
uuid
url
name
metadata {

View File

@@ -29,7 +29,7 @@ export const CONVERSATION_QUERY_FRAGMENT = gql`
id
}
picture {
id
uuid
url
name
metadata {

View File

@@ -27,7 +27,7 @@ const FULL_EVENT_FRAGMENT = gql`
language
category
picture {
id
uuid
url
name
metadata {
@@ -135,7 +135,7 @@ export const FETCH_EVENTS = gql`
insertedAt
language
picture {
id
uuid
url
}
publishAt
@@ -454,8 +454,8 @@ export const FETCH_GROUP_EVENTS = gql`
...AdressFragment
}
picture {
uuid
url
id
}
}
total

View File

@@ -29,11 +29,11 @@ export const LIST_GROUPS = gql`
...ActorFragment
suspended
avatar {
id
uuid
url
}
banner {
id
uuid
url
}
organizedEvents {
@@ -74,7 +74,7 @@ export const GROUP_VERY_BASIC_FIELDS_FRAGMENTS = gql`
url
}
avatar {
id
uuid
url
name
metadata {
@@ -84,7 +84,7 @@ export const GROUP_VERY_BASIC_FIELDS_FRAGMENTS = gql`
}
}
banner {
id
uuid
url
name
metadata {
@@ -118,7 +118,7 @@ export const GROUP_BASIC_FIELDS_FRAGMENTS = gql`
url
}
avatar {
id
uuid
url
name
metadata {
@@ -128,7 +128,7 @@ export const GROUP_BASIC_FIELDS_FRAGMENTS = gql`
}
}
banner {
id
uuid
url
name
metadata {
@@ -167,7 +167,7 @@ export const GROUP_BASIC_FIELDS_FRAGMENTS = gql`
...ActorFragment
}
picture {
id
uuid
url
}
physicalAddress {
@@ -315,7 +315,7 @@ export const CREATE_GROUP = gql`
) {
...ActorFragment
banner {
id
uuid
url
}
}
@@ -441,7 +441,7 @@ export const GROUP_TIMELINE = gql`
originId
}
banner {
id
uuid
url
}
}

View File

@@ -31,7 +31,7 @@ export const HOME_USER_QUERIES = gql`
uuid
title
picture {
id
uuid
url
alt
}

View File

@@ -27,7 +27,7 @@ export const LOGGED_USER_PARTICIPATIONS = gql`
url
title
picture {
id
uuid
url
alt
}
@@ -94,7 +94,7 @@ export const LOGGED_USER_UPCOMING_EVENTS = gql`
url
title
picture {
id
uuid
url
alt
}

View File

@@ -26,7 +26,7 @@ export const POST_FRAGMENT = gql`
...TagFragment
}
picture {
id
uuid
url
name
metadata {
@@ -59,7 +59,7 @@ export const POST_BASIC_FIELDS = gql`
visibility
language
picture {
id
uuid
url
name
}

View File

@@ -25,7 +25,7 @@ export const REPORTS = gql`
uuid
title
picture {
id
uuid
url
}
}
@@ -61,7 +61,7 @@ const REPORT_FRAGMENT = gql`
description
beginsOn
picture {
id
uuid
url
}
organizerActor {

View File

@@ -8,7 +8,7 @@ export const GROUP_RESULT_FRAGMENT = gql`
fragment GroupResultFragment on GroupSearchResult {
id
avatar {
id
uuid
url
}
type
@@ -72,7 +72,7 @@ export const SEARCH_EVENTS_AND_GROUPS = gql`
endsOn
longEvent
picture {
id
uuid
url
}
url
@@ -116,7 +116,7 @@ export const SEARCH_EVENTS_AND_GROUPS = gql`
__typename
id
avatar {
id
uuid
url
}
type
@@ -127,7 +127,7 @@ export const SEARCH_EVENTS_AND_GROUPS = gql`
url
...GroupResultFragment
banner {
id
uuid
url
}
followersCount
@@ -178,7 +178,7 @@ export const SEARCH_EVENTS = gql`
uuid
beginsOn
picture {
id
uuid
url
}
status
@@ -229,7 +229,7 @@ export const SEARCH_CALENDAR_EVENTS = gql`
beginsOn
endsOn
picture {
id
uuid
url
}
status
@@ -277,7 +277,7 @@ export const SEARCH_GROUPS = gql`
elements {
...ActorFragment
banner {
id
uuid
url
}
membersCount
@@ -317,7 +317,7 @@ export const SEARCH_PERSON_AND_GROUPS = gql`
elements {
...ActorFragment
banner {
id
uuid
url
}
membersCount
@@ -342,7 +342,7 @@ export const INTERACT = gql`
beginsOn
status
picture {
id
uuid
url
}
tags {

View File

@@ -3,16 +3,8 @@ import gql from "graphql-tag";
export const UPLOAD_MEDIA = gql`
mutation UploadMedia($file: Upload!, $alt: String, $name: String!) {
uploadMedia(file: $file, alt: $alt, name: $name) {
uuid
url
id
}
}
`;
export const REMOVE_MEDIA = gql`
mutation RemoveMedia($id: ID!) {
removeMedia(id: $id) {
id
}
}
`;

View File

@@ -1,7 +1,7 @@
import type { Ref } from "vue";
export interface IMedia {
id: string;
uuid: string;
url: string;
name: string;
alt: string;

View File

@@ -48,7 +48,7 @@ defmodule Mobilizon.GraphQL.Resolvers.ActivityTest do
name
domain
avatar {
id
uuid
url
}
}
@@ -73,7 +73,7 @@ defmodule Mobilizon.GraphQL.Resolvers.ActivityTest do
preferredUsername
domain
avatar {
id
uuid
url
}
}
@@ -101,10 +101,10 @@ defmodule Mobilizon.GraphQL.Resolvers.ActivityTest do
id
}
banner {
id
uuid
}
avatar {
id
uuid
}
}
}

View File

@@ -619,7 +619,7 @@ defmodule Mobilizon.Web.Resolvers.EventTest do
alt: $alt
file: $file
) {
id
uuid
url
name
content_type
@@ -628,7 +628,7 @@ defmodule Mobilizon.Web.Resolvers.EventTest do
}
"""
test "create_event/3 creates an event with an picture ID", %{
test "create_event/3 creates an event with an picture UUID", %{
conn: conn,
actor: actor,
user: user
@@ -656,8 +656,8 @@ defmodule Mobilizon.Web.Resolvers.EventTest do
assert res["errors"] == nil
assert res["data"]["uploadMedia"]["name"] == media.name
media_id = res["data"]["uploadMedia"]["id"]
assert media_id !== "" and not is_nil(media_id)
media_uuid = res["data"]["uploadMedia"]["uuid"]
assert media_uuid !== "" and not is_nil(media_uuid)
res =
conn
@@ -672,7 +672,7 @@ defmodule Mobilizon.Web.Resolvers.EventTest do
organizer_actor_id: "#{actor.id}",
category: "PARTY",
picture: %{
media_id: "#{media_id}"
media_uuid: "#{media_uuid}"
}
}
)

View File

@@ -45,7 +45,7 @@ defmodule Mobilizon.Web.Resolvers.FollowerTest do
name
domain
avatar {
id
uuid
url
}
}

View File

@@ -36,7 +36,7 @@ defmodule Mobilizon.Web.Resolvers.GroupTest do
preferredUsername
type
banner {
id
uuid
url
}
}

View File

@@ -20,9 +20,9 @@ defmodule Mobilizon.GraphQL.Resolvers.MediaTest do
end
@media_query """
query Media($id: ID!) {
media(id: $id) {
id
query Media($uuid: UUID!) {
media(uuid: $uuid) {
uuid
name,
alt,
url,
@@ -49,11 +49,11 @@ defmodule Mobilizon.GraphQL.Resolvers.MediaTest do
describe "Resolver: Get media" do
test "media/3 returns the information on a media", %{conn: conn} do
%Media{id: id} = media = insert(:media)
%Media{uuid: uuid} = media = insert(:media)
res =
conn
|> AbsintheHelpers.graphql_query(query: @media_query, variables: %{id: id})
|> AbsintheHelpers.graphql_query(query: @media_query, variables: %{uuid: uuid})
assert res["data"]["media"]["name"] == media.file.name
@@ -68,7 +68,10 @@ defmodule Mobilizon.GraphQL.Resolvers.MediaTest do
test "media/3 returns nothing on a non-existent media", %{conn: conn} do
res =
conn
|> AbsintheHelpers.graphql_query(query: @media_query, variables: %{id: 3})
|> AbsintheHelpers.graphql_query(
query: @media_query,
variables: %{uuid: "bad0bad0-bad0-bad0-bad0-bad0bad0bad0"}
)
assert hd(res["errors"])["message"] == "Resource not found"
assert hd(res["errors"])["status_code"] == 404
@@ -131,30 +134,30 @@ defmodule Mobilizon.GraphQL.Resolvers.MediaTest do
describe "Resolver: Remove media" do
@remove_media_mutation """
mutation RemoveMedia($id: ID!) {
removeMedia(id: $id) {
id
mutation RemoveMedia($uuid: UUID!) {
removeMedia(uuid: $uuid) {
uuid
}
}
"""
test "Removes a previously uploaded media", %{conn: conn, user: user, actor: actor} do
%Media{id: media_id} = insert(:media, actor: actor)
%Media{uuid: media_uuid} = insert(:media, actor: actor)
res =
conn
|> auth_conn(user)
|> AbsintheHelpers.graphql_query(
query: @remove_media_mutation,
variables: %{id: media_id}
variables: %{uuid: media_uuid}
)
assert res["errors"] == nil
assert res["data"]["removeMedia"]["id"] == to_string(media_id)
assert res["data"]["removeMedia"]["uuid"] == to_string(media_uuid)
res =
conn
|> AbsintheHelpers.graphql_query(query: @media_query, variables: %{id: media_id})
|> AbsintheHelpers.graphql_query(query: @media_query, variables: %{uuid: media_uuid})
assert hd(res["errors"])["message"] == "Resource not found"
assert hd(res["errors"])["status_code"] == 404
@@ -166,7 +169,7 @@ defmodule Mobilizon.GraphQL.Resolvers.MediaTest do
|> auth_conn(user)
|> AbsintheHelpers.graphql_query(
query: @remove_media_mutation,
variables: %{id: 400}
variables: %{uuid: "bad0bad0-bad0-bad0-bad0-bad0bad0bad0"}
)
assert hd(res["errors"])["message"] == "Resource not found"
@@ -174,13 +177,13 @@ defmodule Mobilizon.GraphQL.Resolvers.MediaTest do
end
test "Removes nothing if media if not logged-in", %{conn: conn, actor: actor} do
%Media{id: media_id} = insert(:media, actor: actor)
%Media{uuid: media_uuid} = insert(:media, actor: actor)
res =
conn
|> AbsintheHelpers.graphql_query(
query: @remove_media_mutation,
variables: %{id: media_id}
variables: %{uuid: media_uuid}
)
assert hd(res["errors"])["message"] == "You need to be logged in"

View File

@@ -11,6 +11,12 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
alias Mobilizon.Web.Endpoint
# TODO There is a bug in the GraphQL API with person queries
# You can ask for a media (avatar and banner) normally with metadata and uuid accessible,
# but you obtain a File (so metadata and uuid are not accessible)
# Until this issue persist, we can't test uuid or metadata retrivial for person queries
# https://framagit.org/kaihuri/mobilizon/-/issues/1757
@non_existent_username "nonexistent"
@get_person_query """
@@ -161,11 +167,9 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
id
preferredUsername
avatar {
id,
url
},
banner {
id,
name,
url
}
@@ -281,7 +285,11 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
assert res["data"]["createPerson"]["preferredUsername"] ==
"new_identity"
assert res["data"]["createPerson"]["banner"]["id"]
# TODO see the top comment and this bug :
# https://framagit.org/kaihuri/mobilizon/-/issues/1757
# assert res["data"]["createPerson"]["banner"]["uuid"]
# assert res["data"]["createPerson"]["banner"]["metadata"]
# assert res["data"]["createPerson"]["banner"]["metadata"]["width"]
assert res["data"]["createPerson"]["banner"]["name"] ==
"The beautiful atlantic way"
@@ -334,11 +342,9 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
name,
summary,
avatar {
id,
url
},
banner {
id,
name,
url
}
@@ -376,7 +382,11 @@ defmodule Mobilizon.GraphQL.Resolvers.PersonTest do
assert res_person["name"] == "riri updated"
assert res_person["summary"] == "summary updated"
assert res_person["banner"]["id"]
# TODO see the top comment and this bug :
# https://framagit.org/kaihuri/mobilizon/-/issues/1757
# assert res_person["banner"]["uuid"]
# assert res_person["banner"]["metadata"]
# assert res_person["banner"]["metadata"]["width"]
assert res_person["banner"]["name"] == "The beautiful atlantic way"
assert res_person["banner"]["url"] =~ Endpoint.url() <> "/media/"
end