Merge branch 'detect-images-in-body' into 'master'

Track usage of media files and add a job to clean them

See merge request framasoft/mobilizon!727

CHANGELOG.md

@@ -6,6 +6,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+**This release adds new migrations, be sure to run them before restarting Mobilizon**
+
+**This release has a repair step, be sure to run the command right after restarting Mobilizon**
+
+### Special operations
+
+* **Reattach media files to their entity.**
+  When media files were uploaded and added in events and posts bodies, they were only attached to the profile that uploaded them, not to the event or post. This task attaches them back to their entity so that the command to clean orphan media files doesn't remove them.
+
+  * Source install
+    `MIX_ENV=prod mix mobilizon.maintenance.fix_unattached_media_in_body`
+  * Docker
+    `docker-compose exec mobilizon mobilizon_ctl maintenance.fix_unattached_media_in_body`
+
+### Added
+
+- **Add a command to clean orphan media files**. There's a `--dry-run` option to see what files would have been deleted.  
+  **Make sure all media files have been reattached properly (see above) before running this command.**  
+  In 1.1.0 a scheduled job will be enabled to clear orphan media files automatically after a while.
+
+### Fixed
+
+- Fix inline media that weren't being tracked, so that they are not considered orphans media files.
+
 ## 1.0.2 - 2020-11-15
 
 **This release adds new migrations, be sure to run them before restarting Mobilizon**

config/config.exs

@@ -28,6 +28,8 @@ config :mobilizon, :instance,
   upload_limit: 10_000_000,
   avatar_upload_limit: 2_000_000,
   banner_upload_limit: 4_000_000,
+  remove_orphan_uploads: true,
+  orphan_upload_grace_period_hours: 48,
   email_from: "noreply@localhost",
   email_reply_to: "noreply@localhost"
 
@@ -250,6 +252,8 @@ config :mobilizon, Oban,
   crontab: [
     {"@hourly", Mobilizon.Service.Workers.BuildSiteMap, queue: :background},
     {"17 * * * *", Mobilizon.Service.Workers.RefreshGroups, queue: :background}
+    # To be activated in Mobilizon 1.2
+    # {"@hourly", Mobilizon.Service.Workers.CleanOrphanMediaWorker, queue: :background}
   ]
 
 config :mobilizon, :rich_media,

js/src/components/Editor.vue

@@ -212,7 +212,7 @@ import { SEARCH_PERSONS } from "../graphql/search";
 import { Actor, IActor, IPerson } from "../types/actor";
 import Image from "./Editor/Image";
 import MaxSize from "./Editor/MaxSize";
-import { UPLOAD_PICTURE } from "../graphql/upload";
+import { UPLOAD_MEDIA } from "../graphql/upload";
 import { listenFileUpload } from "../utils/upload";
 import { CURRENT_ACTOR_CLIENT } from "../graphql/actor";
 import { IComment } from "../types/comment.model";
@@ -395,7 +395,15 @@ export default class EditorComponent extends Vue {
         new Image(),
         new MaxSize({ maxSize: this.maxSize }),
       ],
-      onUpdate: ({ getHTML }: { getHTML: Function }) => {
+      onUpdate: ({
+        getHTML,
+        transaction,
+        getJSON,
+      }: {
+        getHTML: Function;
+        getJSON: Function;
+        transaction: unknown;
+      }) => {
         this.$emit("input", getHTML());
       },
     });
@@ -526,14 +534,14 @@ export default class EditorComponent extends Vue {
     const image = await listenFileUpload();
     try {
       const { data } = await this.$apollo.mutate({
-        mutation: UPLOAD_PICTURE,
+        mutation: UPLOAD_MEDIA,
         variables: {
           file: image,
           name: image.name,
         },
       });
-      if (data.uploadPicture && data.uploadPicture.url) {
-        command({ src: data.uploadPicture.url });
+      if (data.uploadMedia && data.uploadMedia.url) {
+        command({ src: data.uploadMedia.url, "data-media-id": data.uploadMedia.id });
       }
     } catch (error) {
       console.error(error);

js/src/components/Editor/Image.ts

@@ -1,6 +1,7 @@
+// eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-nocheck
 import { Node } from "tiptap";
-import { UPLOAD_PICTURE } from "@/graphql/upload";
+import { UPLOAD_MEDIA } from "@/graphql/upload";
 import apolloProvider from "@/vue-apollo";
 import ApolloClient from "apollo-client";
 import { NormalizedCacheObject } from "apollo-cache-inmemory";
@@ -27,16 +28,18 @@ export default class Image extends Node {
         title: {
           default: null,
         },
+        "data-media-id": {},
       },
       group: "inline",
       draggable: true,
       parseDOM: [
         {
-          tag: "img[src]",
+          tag: "img",
           getAttrs: (dom: any) => ({
             src: dom.getAttribute("src"),
             title: dom.getAttribute("title"),
             alt: dom.getAttribute("alt"),
+            "data-media-id": dom.getAttribute("data-media-id"),
           }),
         },
       ],
@@ -92,13 +95,16 @@ export default class Image extends Node {
               try {
                 images.forEach(async (image) => {
                   const { data } = await client.mutate({
-                    mutation: UPLOAD_PICTURE,
+                    mutation: UPLOAD_MEDIA,
                     variables: {
                       file: image,
                       name: image.name,
                     },
                   });
-                  const node = schema.nodes.image.create({ src: data.uploadPicture.url });
+                  const node = schema.nodes.image.create({
+                    src: data.uploadMedia.url,
+                    "data-media-id": data.uploadMedia.id,
+                  });
                   const transaction = view.state.tr.insert(coordinates.pos, node);
                   view.dispatch(transaction);
                 });

js/src/components/PictureUpload.vue

@@ -60,14 +60,14 @@ figure.image {
 </style>
 
 <script lang="ts">
-import { IPicture } from "@/types/picture.model";
+import { IMedia } from "@/types/media.model";
 import { Component, Model, Prop, Vue, Watch } from "vue-property-decorator";
 
 @Component
 export default class PictureUpload extends Vue {
   @Model("change", { type: File }) readonly pictureFile!: File;
 
-  @Prop({ type: Object, required: false }) defaultImage!: IPicture;
+  @Prop({ type: Object, required: false }) defaultImage!: IMedia;
 
   @Prop({ type: String, required: false, default: "image/gif,image/png,image/jpeg,image/webp" })
   accept!: string;
@@ -100,7 +100,7 @@ export default class PictureUpload extends Vue {
   }
 
   @Watch("defaultImage")
-  onDefaultImageChange(defaultImage: IPicture): void {
+  onDefaultImageChange(defaultImage: IMedia): void {
     console.log("onDefaultImageChange", defaultImage);
     this.imageSrc = defaultImage ? defaultImage.url : null;
   }

js/src/graphql/actor.ts

@@ -421,7 +421,7 @@ export const CREATE_PERSON = gql`
     $preferredUsername: String!
     $name: String!
     $summary: String
-    $avatar: PictureInput
+    $avatar: MediaInput
   ) {
     createPerson(
       preferredUsername: $preferredUsername
@@ -442,7 +442,7 @@ export const CREATE_PERSON = gql`
 `;
 
 export const UPDATE_PERSON = gql`
-  mutation UpdatePerson($id: ID!, $name: String, $summary: String, $avatar: PictureInput) {
+  mutation UpdatePerson($id: ID!, $name: String, $summary: String, $avatar: MediaInput) {
     updatePerson(id: $id, name: $name, summary: $summary, avatar: $avatar) {
       id
       preferredUsername

js/src/graphql/event.ts

@@ -244,7 +244,7 @@ export const CREATE_EVENT = gql`
     $joinOptions: EventJoinOptions,
     $draft: Boolean,
     $tags: [String],
-    $picture: PictureInput,
+    $picture: MediaInput,
     $onlineAddress: String,
     $phoneAddress: String,
     $category: String,
@@ -355,7 +355,7 @@ export const EDIT_EVENT = gql`
     $joinOptions: EventJoinOptions,
     $draft: Boolean,
     $tags: [String],
-    $picture: PictureInput,
+    $picture: MediaInput,
     $onlineAddress: String,
     $phoneAddress: String,
     $organizerActorId: ID,

js/src/graphql/group.ts

@@ -227,8 +227,8 @@ export const CREATE_GROUP = gql`
     $preferredUsername: String!
     $name: String!
     $summary: String
-    $avatar: PictureInput
-    $banner: PictureInput
+    $avatar: MediaInput
+    $banner: MediaInput
   ) {
     createGroup(
       preferredUsername: $preferredUsername
@@ -259,8 +259,8 @@ export const UPDATE_GROUP = gql`
     $id: ID!
     $name: String
     $summary: String
-    $avatar: PictureInput
-    $banner: PictureInput
+    $avatar: MediaInput
+    $banner: MediaInput
     $visibility: GroupVisibility
     $openness: Openness
     $physicalAddress: AddressInput

js/src/graphql/post.ts

@@ -119,7 +119,7 @@ export const CREATE_POST = gql`
     $visibility: PostVisibility
     $draft: Boolean
     $tags: [String]
-    $picture: PictureInput
+    $picture: MediaInput
   ) {
     createPost(
       title: $title
@@ -145,7 +145,7 @@ export const UPDATE_POST = gql`
     $visibility: PostVisibility
     $draft: Boolean
     $tags: [String]
-    $picture: PictureInput
+    $picture: MediaInput
   ) {
     updatePost(
       id: $id

js/src/graphql/upload.ts

@@ -1,17 +1,17 @@
 import gql from "graphql-tag";
 
-export const UPLOAD_PICTURE = gql`
-  mutation UploadPicture($file: Upload!, $alt: String, $name: String!) {
-    uploadPicture(file: $file, alt: $alt, name: $name) {
+export const UPLOAD_MEDIA = gql`
+  mutation UploadMedia($file: Upload!, $alt: String, $name: String!) {
+    uploadMedia(file: $file, alt: $alt, name: $name) {
       url
       id
     }
   }
 `;
 
-export const REMOVE_PICTURE = gql`
-  mutation RemovePicture($id: ID!) {
-    removePicture(id: $id) {
+export const REMOVE_MEDIA = gql`
+  mutation RemoveMedia($id: ID!) {
+    removeMedia(id: $id) {
       id
     }
   }

js/src/types/actor/actor.model.ts

@@ -1,4 +1,4 @@
-import { IPicture } from "@/types/picture.model";
+import { IMedia } from "@/types/media.model";
 
 export enum ActorType {
   PERSON = "PERSON",
@@ -17,17 +17,17 @@ export interface IActor {
   summary: string;
   preferredUsername: string;
   suspended: boolean;
-  avatar?: IPicture | null;
-  banner?: IPicture | null;
+  avatar?: IMedia | null;
+  banner?: IMedia | null;
   type: ActorType;
 }
 
 export class Actor implements IActor {
   id?: string;
 
-  avatar: IPicture | null = null;
+  avatar: IMedia | null = null;
 
-  banner: IPicture | null = null;
+  banner: IMedia | null = null;
 
   domain: string | null = null;
 

js/src/types/event.model.ts

@@ -1,6 +1,6 @@
 import { Address, IAddress } from "@/types/address.model";
 import { ITag } from "@/types/tag.model";
-import { IPicture } from "@/types/picture.model";
+import { IMedia } from "@/types/media.model";
 import { IComment } from "@/types/comment.model";
 import { Paginate } from "@/types/paginate";
 import { Actor, Group, IActor, IGroup, IPerson } from "./actor";
@@ -69,7 +69,7 @@ interface IEventEditJSON {
   visibility: EventVisibility;
   joinOptions: EventJoinOptions;
   draft: boolean;
-  picture?: IPicture | { pictureId: string } | null;
+  picture?: IMedia | { mediaId: string } | null;
   attributedToId: string | null;
   onlineAddress?: string;
   phoneAddress?: string;
@@ -96,7 +96,7 @@ export interface IEvent {
   joinOptions: EventJoinOptions;
   draft: boolean;
 
-  picture: IPicture | null;
+  picture: IMedia | null;
 
   organizerActor?: IActor;
   attributedTo?: IGroup;
@@ -142,7 +142,7 @@ export class EventModel implements IEvent {
 
   physicalAddress?: IAddress;
 
-  picture: IPicture | null = null;
+  picture: IMedia | null = null;
 
   visibility = EventVisibility.PUBLIC;
 

js/src/types/media.model.ts

@@ -1,11 +1,11 @@
-export interface IPicture {
+export interface IMedia {
   id: string;
   url: string;
   name: string;
   alt: string;
 }
 
-export interface IPictureUpload {
+export interface IMediaUpload {
   file: File;
   name: string;
   alt: string | null;

js/src/types/post.model.ts

@@ -1,5 +1,5 @@
 import { ITag } from "./tag.model";
-import { IPicture } from "./picture.model";
+import { IMedia } from "./media.model";
 import { IActor } from "./actor";
 
 export enum PostVisibility {
@@ -17,7 +17,7 @@ export interface IPost {
   title: string;
   body: string;
   tags?: ITag[];
-  picture?: IPicture | null;
+  picture?: IMedia | null;
   draft: boolean;
   visibility: PostVisibility;
   author?: IActor;

js/src/utils/image.ts

@@ -1,6 +1,6 @@
-import { IPicture } from "@/types/picture.model";
+import { IMedia } from "@/types/media.model";
 
-export async function buildFileFromIPicture(obj: IPicture | null | undefined): Promise<File | null> {
+export async function buildFileFromIMedia(obj: IMedia | null | undefined): Promise<File | null> {
   if (!obj) return Promise.resolve(null);
 
   const response = await fetch(obj.url);
@@ -14,7 +14,7 @@ export function buildFileVariable(file: File | null, name: string, alt?: string)
 
   return {
     [name]: {
-      picture: {
+      media: {
         name: file.name,
         alt: alt || file.name,
         file,

js/src/views/Account/children/EditIdentity.vue

@@ -124,7 +124,6 @@ h1 {
 <script lang="ts">
 import { Component, Prop, Watch } from "vue-property-decorator";
 import { mixins } from "vue-class-component";
-import { IPicture } from "@/types/picture.model";
 import {
   CREATE_PERSON,
   CURRENT_ACTOR_CLIENT,
@@ -137,7 +136,7 @@ import { IPerson, Person } from "../../../types/actor";
 import PictureUpload from "../../../components/PictureUpload.vue";
 import { MOBILIZON_INSTANCE_HOST } from "../../../api/_entrypoint";
 import RouteName from "../../../router/name";
-import { buildFileFromIPicture, buildFileVariable } from "../../../utils/image";
+import { buildFileVariable } from "../../../utils/image";
 import { changeIdentity } from "../../../utils/auth";
 import identityEditionMixin from "../../../mixins/identityEdition";
 

js/src/views/Event/Edit.vue

@@ -377,7 +377,7 @@ import {
 import { IPerson, Person, displayNameAndUsername } from "../../types/actor";
 import { TAGS } from "../../graphql/tags";
 import { ITag } from "../../types/tag.model";
-import { buildFileFromIPicture, buildFileVariable, readFileAsync } from "../../utils/image";
+import { buildFileFromIMedia, buildFileVariable, readFileAsync } from "../../utils/image";
 import RouteName from "../../router/name";
 import "intersection-observer";
 import { CONFIG } from "../../graphql/config";
@@ -517,7 +517,7 @@ export default class EditEvent extends Vue {
     );
     this.observer.observe(this.$refs.bottomObserver as Element);
 
-    this.pictureFile = await buildFileFromIPicture(this.event.picture);
+    this.pictureFile = await buildFileFromIMedia(this.event.picture);
     this.limitedPlaces = this.event.options.maximumAttendeeCapacity > 0;
     if (!(this.isUpdate || this.isDuplicate)) {
       this.initializeEvent();
@@ -775,11 +775,11 @@ export default class EditEvent extends Vue {
 
     try {
       if (this.event.picture && this.pictureFile) {
-        const oldPictureFile = (await buildFileFromIPicture(this.event.picture)) as File;
+        const oldPictureFile = (await buildFileFromIMedia(this.event.picture)) as File;
         const oldPictureFileContent = await readFileAsync(oldPictureFile);
         const newPictureFileContent = await readFileAsync(this.pictureFile as File);
         if (oldPictureFileContent === newPictureFileContent) {
-          res.picture = { pictureId: this.event.picture.id };
+          res.picture = { mediaId: this.event.picture.id };
         }
       }
     } catch (e) {

js/src/views/Group/GroupSettings.vue

@@ -246,7 +246,7 @@ export default class GroupSettings extends mixins(GroupMixin) {
     if (this.avatarFile) {
       avatarObj = {
         avatar: {
-          picture: {
+          media: {
             name: this.avatarFile.name,
             alt: `${this.group.preferredUsername}'s avatar`,
             file: this.avatarFile,
@@ -258,7 +258,7 @@ export default class GroupSettings extends mixins(GroupMixin) {
     if (this.bannerFile) {
       bannerObj = {
         banner: {
-          picture: {
+          media: {
             name: this.bannerFile.name,
             alt: `${this.group.preferredUsername}'s banner`,
             file: this.bannerFile,

js/src/views/Posts/Edit.vue

@@ -103,7 +103,7 @@
 import { Component, Prop } from "vue-property-decorator";
 import { mixins } from "vue-class-component";
 import { FETCH_GROUP } from "@/graphql/group";
-import { buildFileFromIPicture, readFileAsync } from "@/utils/image";
+import { buildFileFromIMedia, readFileAsync } from "@/utils/image";
 import GroupMixin from "@/mixins/group";
 import { TAGS } from "../../graphql/tags";
 import { CONFIG } from "../../graphql/config";
@@ -188,7 +188,7 @@ export default class EditPost extends mixins(GroupMixin) {
   errors: Record<string, unknown> = {};
 
   async mounted(): Promise<void> {
-    this.pictureFile = await buildFileFromIPicture(this.post.picture);
+    this.pictureFile = await buildFileFromIMedia(this.post.picture);
   }
 
   // eslint-disable-next-line consistent-return
@@ -277,11 +277,11 @@ export default class EditPost extends mixins(GroupMixin) {
     }
     try {
       if (this.post.picture) {
-        const oldPictureFile = (await buildFileFromIPicture(this.post.picture)) as File;
+        const oldPictureFile = (await buildFileFromIMedia(this.post.picture)) as File;
         const oldPictureFileContent = await readFileAsync(oldPictureFile);
         const newPictureFileContent = await readFileAsync(this.pictureFile as File);
         if (oldPictureFileContent === newPictureFileContent) {
-          obj.picture = { pictureId: this.post.picture.id };
+          obj.picture = { mediaId: this.post.picture.id };
         }
       }
     } catch (e) {

lib/federation/activity_pub/utils.ex

@@ -10,7 +10,7 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
 
   alias Mobilizon.Actors
   alias Mobilizon.Actors.Actor
-  alias Mobilizon.Media.Picture
+  alias Mobilizon.Medias.Media
 
   alias Mobilizon.Federation.ActivityPub
   alias Mobilizon.Federation.ActivityPub.{Activity, Federator, Relay}
@@ -333,50 +333,50 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
   Return AS Link data from
 
   * a `Plug.Upload` struct, stored an returned
-  * a `Picture`, directly returned
-  * a map containing picture information, stored, saved and returned
+  * a `Media`, directly returned
+  * a map containing media information, stored, saved and returned
 
-  Save picture data from %Plug.Upload{} and return AS Link data.
+  Save media data from %Plug.Upload{} and return AS Link data.
   """
-  def make_picture_data(%Plug.Upload{} = picture, opts) do
-    case Mobilizon.Web.Upload.store(picture, opts) do
-      {:ok, picture} ->
-        picture
+  def make_media_data(%Plug.Upload{} = media, opts) do
+    case Mobilizon.Web.Upload.store(media, opts) do
+      {:ok, media} ->
+        media
 
       _ ->
         nil
     end
   end
 
-  def make_picture_data(%Picture{} = picture) do
-    Converter.Picture.model_to_as(picture)
+  def make_media_data(%Media{} = media) do
+    Converter.Media.model_to_as(media)
   end
 
-  def make_picture_data(picture) when is_map(picture) do
+  def make_media_data(media) when is_map(media) do
     with {:ok, %{"url" => [%{"href" => url, "mediaType" => content_type}], "size" => size}} <-
-           Mobilizon.Web.Upload.store(picture.file),
-         {:picture_exists, nil} <- {:picture_exists, Mobilizon.Media.get_picture_by_url(url)},
-         {:ok, %Picture{file: _file} = picture} <-
-           Mobilizon.Media.create_picture(%{
+           Mobilizon.Web.Upload.store(media.file),
+         {:media_exists, nil} <- {:media_exists, Mobilizon.Medias.get_media_by_url(url)},
+         {:ok, %Media{file: _file} = media} <-
+           Mobilizon.Medias.create_media(%{
              "file" => %{
                "url" => url,
-               "name" => picture.name,
+               "name" => media.name,
                "content_type" => content_type,
                "size" => size
              },
-             "actor_id" => picture.actor_id
+             "actor_id" => media.actor_id
            }) do
-      Converter.Picture.model_to_as(picture)
+      Converter.Media.model_to_as(media)
     else
-      {:picture_exists, %Picture{file: _file} = picture} ->
-        Converter.Picture.model_to_as(picture)
+      {:media_exists, %Media{file: _file} = media} ->
+        Converter.Media.model_to_as(media)
 
       err ->
         err
     end
   end
 
-  def make_picture_data(nil), do: nil
+  def make_media_data(nil), do: nil
 
   @doc """
   Make announce activity data for the given actor and object

lib/federation/activity_stream/converter/event.ex

@@ -10,11 +10,11 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Event do
   alias Mobilizon.Addresses
   alias Mobilizon.Addresses.Address
   alias Mobilizon.Events.Event, as: EventModel
-  alias Mobilizon.Media.Picture
+  alias Mobilizon.Medias.Media
 
   alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
   alias Mobilizon.Federation.ActivityStream.Converter.Address, as: AddressConverter
-  alias Mobilizon.Federation.ActivityStream.Converter.Picture, as: PictureConverter
+  alias Mobilizon.Federation.ActivityStream.Converter.Media, as: MediaConverter
 
   import Mobilizon.Federation.ActivityStream.Converter.Utils,
     only: [
@@ -55,10 +55,10 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Event do
 
       picture_id =
         with true <- length(attachments) > 0,
-             {:ok, %Picture{id: picture_id}} <-
+             {:ok, %Media{id: picture_id}} <-
                attachments
                |> hd()
-               |> PictureConverter.find_or_create_picture(actor_id) do
+               |> MediaConverter.find_or_create_media(actor_id) do
           picture_id
         else
           _err ->
@@ -239,7 +239,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Event do
           res,
           "attachment",
           [],
-          &(&1 ++ [PictureConverter.model_to_as(event.picture)])
+          &(&1 ++ [MediaConverter.model_to_as(event.picture)])
         )
   end
 

lib/federation/activity_stream/converter/media.ex

@@ -0,0 +1,63 @@
+defmodule Mobilizon.Federation.ActivityStream.Converter.Media do
+  @moduledoc """
+  Media converter.
+
+  This module allows to convert events from ActivityStream format to our own
+  internal one, and back.
+  """
+
+  alias Mobilizon.Medias
+  alias Mobilizon.Medias.Media, as: MediaModel
+
+  alias Mobilizon.Web.Upload
+
+  @http_options [
+    ssl: [{:versions, [:"tlsv1.2"]}]
+  ]
+
+  @doc """
+  Convert a media struct to an ActivityStream representation.
+  """
+  @spec model_to_as(MediaModel.t()) :: map
+  def model_to_as(%MediaModel{file: file}) do
+    %{
+      "type" => "Document",
+      "mediaType" => file.content_type,
+      "url" => file.url,
+      "name" => file.name
+    }
+  end
+
+  @doc """
+  Save media data from raw data and return AS Link data.
+  """
+  def find_or_create_media(%{"type" => "Link", "href" => url}, actor_id),
+    do: find_or_create_media(url, actor_id)
+
+  def find_or_create_media(
+        %{"type" => "Document", "url" => media_url, "name" => name},
+        actor_id
+      )
+      when is_bitstring(media_url) do
+    with {:ok, %{body: body}} <- Tesla.get(media_url, opts: @http_options),
+         {:ok, %{name: name, url: url, content_type: content_type, size: size}} <-
+           Upload.store(%{body: body, name: name}),
+         {:media_exists, nil} <- {:media_exists, Medias.get_media_by_url(url)} do
+      Medias.create_media(%{
+        "file" => %{
+          "url" => url,
+          "name" => name,
+          "content_type" => content_type,
+          "size" => size
+        },
+        "actor_id" => actor_id
+      })
+    else
+      {:media_exists, %MediaModel{file: _file} = media} ->
+        {:ok, media}
+
+      err ->
+        err
+    end
+  end
+end

lib/federation/activity_stream/converter/picture.ex

@@ -1,63 +0,0 @@
-defmodule Mobilizon.Federation.ActivityStream.Converter.Picture do
-  @moduledoc """
-  Picture converter.
-
-  This module allows to convert events from ActivityStream format to our own
-  internal one, and back.
-  """
-
-  alias Mobilizon.Media
-  alias Mobilizon.Media.Picture, as: PictureModel
-
-  alias Mobilizon.Web.Upload
-
-  @http_options [
-    ssl: [{:versions, [:"tlsv1.2"]}]
-  ]
-
-  @doc """
-  Convert a picture struct to an ActivityStream representation.
-  """
-  @spec model_to_as(PictureModel.t()) :: map
-  def model_to_as(%PictureModel{file: file}) do
-    %{
-      "type" => "Document",
-      "mediaType" => file.content_type,
-      "url" => file.url,
-      "name" => file.name
-    }
-  end
-
-  @doc """
-  Save picture data from raw data and return AS Link data.
-  """
-  def find_or_create_picture(%{"type" => "Link", "href" => url}, actor_id),
-    do: find_or_create_picture(url, actor_id)
-
-  def find_or_create_picture(
-        %{"type" => "Document", "url" => picture_url, "name" => name},
-        actor_id
-      )
-      when is_bitstring(picture_url) do
-    with {:ok, %{body: body}} <- Tesla.get(picture_url, opts: @http_options),
-         {:ok, %{name: name, url: url, content_type: content_type, size: size}} <-
-           Upload.store(%{body: body, name: name}),
-         {:picture_exists, nil} <- {:picture_exists, Media.get_picture_by_url(url)} do
-      Media.create_picture(%{
-        "file" => %{
-          "url" => url,
-          "name" => name,
-          "content_type" => content_type,
-          "size" => size
-        },
-        "actor_id" => actor_id
-      })
-    else
-      {:picture_exists, %PictureModel{file: _file} = picture} ->
-        {:ok, picture}
-
-      err ->
-        err
-    end
-  end
-end

lib/graphql/api/comments.ex

@@ -1,34 +1,59 @@
 defmodule Mobilizon.GraphQL.API.Comments do
   @moduledoc """
-  API for Comments.
+  API for discussions and comments.
   """
 
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Discussions.Comment
   alias Mobilizon.Federation.ActivityPub
   alias Mobilizon.Federation.ActivityPub.Activity
+  alias Mobilizon.GraphQL.API.Utils
 
   @doc """
   Create a comment
-
-  Creates a comment from an actor
   """
   @spec create_comment(map) :: {:ok, Activity.t(), Comment.t()} | any
   def create_comment(args) do
+    args = extract_pictures_from_comment_body(args)
     ActivityPub.create(:comment, args, true)
   end
 
+  @doc """
+  Updates a comment
+  """
+  @spec update_comment(Comment.t(), map()) :: {:ok, Activity.t(), Comment.t()} | any
   def update_comment(%Comment{} = comment, args) do
+    args = extract_pictures_from_comment_body(args)
     ActivityPub.update(comment, args, true)
   end
 
   @doc """
   Deletes a comment
-
-  Deletes a comment from an actor
   """
   @spec delete_comment(Comment.t(), Actor.t()) :: {:ok, Activity.t(), Comment.t()} | any
   def delete_comment(%Comment{} = comment, %Actor{} = actor) do
     ActivityPub.delete(comment, actor, true)
   end
+
+  @doc """
+  Creates a discussion (or reply to a discussion)
+  """
+  @spec create_discussion(map()) :: map()
+  def create_discussion(args) do
+    args = extract_pictures_from_comment_body(args)
+
+    ActivityPub.create(
+      :discussion,
+      args,
+      true
+    )
+  end
+
+  @spec extract_pictures_from_comment_body(map()) :: map()
+  defp extract_pictures_from_comment_body(%{text: text, actor_id: actor_id} = args) do
+    pictures = Utils.extract_pictures_from_body(text, actor_id)
+    Map.put(args, :media, pictures)
+  end
+
+  defp extract_pictures_from_comment_body(args), do: args
 end

lib/graphql/api/events.ex

@@ -8,6 +8,7 @@ defmodule Mobilizon.GraphQL.API.Events do
 
   alias Mobilizon.Federation.ActivityPub
   alias Mobilizon.Federation.ActivityPub.{Activity, Utils}
+  alias Mobilizon.GraphQL.API.Utils, as: APIUtils
 
   @doc """
   Create an event
@@ -15,6 +16,7 @@ defmodule Mobilizon.GraphQL.API.Events do
   @spec create_event(map) :: {:ok, Activity.t(), Event.t()} | any
   def create_event(args) do
     with organizer_actor <- Map.get(args, :organizer_actor),
+         args <- extract_pictures_from_event_body(args, organizer_actor),
          args <-
            Map.update(args, :picture, nil, fn picture ->
              process_picture(picture, organizer_actor)
@@ -30,6 +32,7 @@ defmodule Mobilizon.GraphQL.API.Events do
   @spec update_event(map, Event.t()) :: {:ok, Activity.t(), Event.t()} | any
   def update_event(args, %Event{} = event) do
     with organizer_actor <- Map.get(args, :organizer_actor),
+         args <- extract_pictures_from_event_body(args, organizer_actor),
          args <-
            Map.update(args, :picture, nil, fn picture ->
              process_picture(picture, organizer_actor)
@@ -40,23 +43,32 @@ defmodule Mobilizon.GraphQL.API.Events do
 
   @doc """
   Trigger the deletion of an event
-
-  If the event is deleted by
   """
   def delete_event(%Event{} = event, %Actor{} = actor, federate \\ true) do
     ActivityPub.delete(event, actor, federate)
   end
 
   defp process_picture(nil, _), do: nil
-  defp process_picture(%{picture_id: _picture_id} = args, _), do: args
+  defp process_picture(%{media_id: _picture_id} = args, _), do: args
 
-  defp process_picture(%{picture: picture}, %Actor{id: actor_id}) do
+  defp process_picture(%{media: media}, %Actor{id: actor_id}) do
     %{
       file:
-        picture
+        media
         |> Map.get(:file)
-        |> Utils.make_picture_data(description: Map.get(picture, :name)),
+        |> Utils.make_media_data(description: Map.get(media, :name)),
       actor_id: actor_id
     }
   end
+
+  @spec extract_pictures_from_event_body(map(), Actor.t()) :: map()
+  defp extract_pictures_from_event_body(
+         %{description: description} = args,
+         %Actor{id: organizer_actor_id}
+       ) do
+    pictures = APIUtils.extract_pictures_from_body(description, organizer_actor_id)
+    Map.put(args, :media, pictures)
+  end
+
+  defp extract_pictures_from_event_body(args, _), do: args
 end

lib/graphql/api/utils.ex

@@ -3,7 +3,8 @@ defmodule Mobilizon.GraphQL.API.Utils do
   Utils for API.
   """
 
-  alias Mobilizon.Config
+  alias Mobilizon.{Config, Medias}
+  alias Mobilizon.Medias.Media
   alias Mobilizon.Service.Formatter
 
   @doc """
@@ -40,4 +41,41 @@ defmodule Mobilizon.GraphQL.API.Utils do
       {:error, "Comment must be up to #{max_size} characters"}
     end
   end
+
+  @doc """
+  Use the data-media-id attributes to extract media from body text
+  """
+  @spec extract_pictures_from_body(String.t(), integer() | String.t()) :: list(Media.t())
+  def extract_pictures_from_body(body, actor_id) do
+    body
+    |> do_extract_pictures_from_body()
+    |> Enum.map(&fetch_picture(&1, actor_id))
+    |> Enum.filter(& &1)
+  end
+
+  @spec do_extract_pictures_from_body(String.t()) :: list(String.t())
+  defp do_extract_pictures_from_body(body) when is_nil(body) or body == "", do: []
+
+  defp do_extract_pictures_from_body(body) do
+    {:ok, document} = Floki.parse_document(body)
+
+    document
+    |> Floki.attribute("img", "data-media-id")
+  end
+
+  @spec fetch_picture(String.t() | integer(), String.t() | integer()) :: Media.t() | nil
+  defp fetch_picture(id, actor_id) do
+    with %Media{actor_id: media_actor_id} = media <- Medias.get_media(id),
+         {:owns_media, true} <-
+           {:owns_media, check_actor_owns_media?(actor_id, media_actor_id)} do
+      media
+    else
+      _ -> nil
+    end
+  end
+
+  @spec check_actor_owns_media?(integer() | String.t(), integer() | String.t()) :: boolean()
+  defp check_actor_owns_media?(actor_id, media_actor_id) do
+    actor_id == media_actor_id || Mobilizon.Actors.is_member?(media_actor_id, actor_id)
+  end
 end

lib/graphql/resolvers/discussion.ex

@@ -7,6 +7,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Discussion do
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Discussions.{Comment, Discussion}
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.GraphQL.API.Comments
   alias Mobilizon.Storage.Page
   alias Mobilizon.Users.User
   import Mobilizon.Web.Gettext
@@ -94,17 +95,13 @@ defmodule Mobilizon.GraphQL.Resolvers.Discussion do
     with {:actor, %Actor{id: creator_id} = _actor} <- {:actor, Users.get_actor_for_user(user)},
          {:member, true} <- {:member, Actors.is_member?(creator_id, group_id)},
          {:ok, _activity, %Discussion{} = discussion} <-
-           ActivityPub.create(
-             :discussion,
-             %{
-               title: title,
-               text: text,
-               actor_id: group_id,
-               creator_id: creator_id,
-               attributed_to_id: group_id
-             },
-             true
-           ) do
+           Comments.create_discussion(%{
+             title: title,
+             text: text,
+             actor_id: group_id,
+             creator_id: creator_id,
+             attributed_to_id: group_id
+           }) do
       {:ok, discussion}
     else
       {:member, false} ->
@@ -134,19 +131,15 @@ defmodule Mobilizon.GraphQL.Resolvers.Discussion do
            {:no_discussion, Discussions.get_discussion(discussion_id)},
          {:member, true} <- {:member, Actors.is_member?(creator_id, actor_id)},
          {:ok, _activity, %Discussion{} = discussion} <-
-           ActivityPub.create(
-             :discussion,
-             %{
-               text: text,
-               discussion_id: discussion_id,
-               actor_id: creator_id,
-               attributed_to_id: actor_id,
-               in_reply_to_comment_id: last_comment_id,
-               origin_comment_id:
-                 origin_comment_id || previous_in_reply_to_comment_id || last_comment_id
-             },
-             true
-           ) do
+           Comments.create_discussion(%{
+             text: text,
+             discussion_id: discussion_id,
+             actor_id: creator_id,
+             attributed_to_id: actor_id,
+             in_reply_to_comment_id: last_comment_id,
+             origin_comment_id:
+               origin_comment_id || previous_in_reply_to_comment_id || last_comment_id
+           }) do
       {:ok, discussion}
     end
   end

lib/graphql/resolvers/group.ex

@@ -96,8 +96,8 @@ defmodule Mobilizon.GraphQL.Resolvers.Group do
   # TODO Move me to somewhere cleaner
   defp save_attached_pictures(args) do
     Enum.reduce([:avatar, :banner], args, fn key, args ->
-      if Map.has_key?(args, key) && !is_nil(args[key][:picture]) do
-        pic = args[key][:picture]
+      if Map.has_key?(args, key) && !is_nil(args[key][:media]) do
+        pic = args[key][:media]
 
         with {:ok, %{name: name, url: url, content_type: content_type, size: _size}} <-
                Upload.store(pic.file, type: key, description: pic.alt) do

lib/graphql/resolvers/media.ex

@@ -1,50 +1,47 @@
-defmodule Mobilizon.GraphQL.Resolvers.Picture do
+defmodule Mobilizon.GraphQL.Resolvers.Media do
   @moduledoc """
-  Handles the picture-related GraphQL calls
+  Handles the media-related GraphQL calls
   """
 
   alias Mobilizon.Actors.Actor
-  alias Mobilizon.{Media, Users}
-  alias Mobilizon.Media.Picture
+  alias Mobilizon.{Medias, Users}
+  alias Mobilizon.Medias.Media
   alias Mobilizon.Users.User
   import Mobilizon.Web.Gettext
 
   @doc """
-  Get picture for an event
+  Get media for an event
 
   See Mobilizon.Web.Resolvers.Event.create_event/3
   """
-  def picture(%{picture_id: picture_id} = _parent, _args, _resolution) do
-    with {:ok, picture} <- do_fetch_picture(picture_id), do: {:ok, picture}
+  def media(%{picture_id: media_id} = _parent, _args, _resolution) do
+    with {:ok, media} <- do_fetch_media(media_id), do: {:ok, media}
   end
 
-  def picture(%{picture: picture} = _parent, _args, _resolution), do: {:ok, picture}
-  def picture(_parent, %{id: picture_id}, _resolution), do: do_fetch_picture(picture_id)
-  def picture(_parent, _args, _resolution), do: {:ok, nil}
+  def media(%{picture: media} = _parent, _args, _resolution), do: {:ok, media}
+  def media(_parent, %{id: media_id}, _resolution), do: do_fetch_media(media_id)
+  def media(_parent, _args, _resolution), do: {:ok, nil}
 
-  @spec do_fetch_picture(nil) :: {:error, nil}
-  defp do_fetch_picture(nil), do: {:error, nil}
+  def medias(%{media: medias}, _args, _resolution) do
+    {:ok, Enum.map(medias, &transform_media/1)}
+  end
 
-  @spec do_fetch_picture(String.t()) :: {:ok, Picture.t()} | {:error, :not_found}
-  defp do_fetch_picture(picture_id) do
-    case Media.get_picture(picture_id) do
-      %Picture{id: id, file: file} ->
-        {:ok,
-         %{
-           name: file.name,
-           url: file.url,
-           id: id,
-           content_type: file.content_type,
-           size: file.size
-         }}
+  @spec do_fetch_media(nil) :: {:error, nil}
+  defp do_fetch_media(nil), do: {:error, nil}
+
+  @spec do_fetch_media(String.t()) :: {:ok, Media.t()} | {:error, :not_found}
+  defp do_fetch_media(media_id) do
+    case Medias.get_media(media_id) do
+      %Media{} = media ->
+        {:ok, transform_media(media)}
 
       nil ->
         {:error, :not_found}
     end
   end
 
-  @spec upload_picture(map, map, map) :: {:ok, Picture.t()} | {:error, any}
-  def upload_picture(
+  @spec upload_media(map, map, map) :: {:ok, Media.t()} | {:error, any}
+  def upload_media(
         _parent,
         %{file: %Plug.Upload{} = file} = args,
         %{context: %{current_user: %User{} = user}}
@@ -57,16 +54,9 @@ defmodule Mobilizon.GraphQL.Resolvers.Picture do
            |> Map.put(:url, url)
            |> Map.put(:size, size)
            |> Map.put(:content_type, content_type),
-         {:ok, picture = %Picture{}} <-
-           Media.create_picture(%{"file" => args, "actor_id" => actor_id}) do
-      {:ok,
-       %{
-         name: picture.file.name,
-         url: picture.file.url,
-         id: picture.id,
-         content_type: picture.file.content_type,
-         size: picture.file.size
-       }}
+         {:ok, media = %Media{}} <-
+           Medias.create_media(%{"file" => args, "actor_id" => actor_id}) do
+      {:ok, transform_media(media)}
     else
       {:error, :mime_type_not_allowed} ->
         {:error, dgettext("errors", "File doesn't have an allowed MIME type.")}
@@ -76,28 +66,28 @@ defmodule Mobilizon.GraphQL.Resolvers.Picture do
     end
   end
 
-  def upload_picture(_parent, _args, _resolution), do: {:error, :unauthenticated}
+  def upload_media(_parent, _args, _resolution), do: {:error, :unauthenticated}
 
   @doc """
-  Remove a picture that the user owns
+  Remove a media that the user owns
   """
-  @spec remove_picture(map(), map(), map()) ::
-          {:ok, Picture.t()}
+  @spec remove_media(map(), map(), map()) ::
+          {:ok, Media.t()}
           | {:error, :unauthorized}
           | {:error, :unauthenticated}
           | {:error, :not_found}
-  def remove_picture(_parent, %{id: picture_id}, %{context: %{current_user: %User{} = user}}) do
-    with {:picture, %Picture{actor_id: actor_id} = picture} <-
-           {:picture, Media.get_picture(picture_id)},
+  def remove_media(_parent, %{id: media_id}, %{context: %{current_user: %User{} = user}}) do
+    with {:media, %Media{actor_id: actor_id} = media} <-
+           {:media, Medias.get_media(media_id)},
          {:is_owned, %Actor{} = _actor} <- User.owns_actor(user, actor_id) do
-      Media.delete_picture(picture)
+      Medias.delete_media(media)
     else
-      {:picture, nil} -> {:error, :not_found}
+      {:media, nil} -> {:error, :not_found}
       {:is_owned, _} -> {:error, :unauthorized}
     end
   end
 
-  def remove_picture(_parent, _args, _resolution), do: {:error, :unauthenticated}
+  def remove_media(_parent, _args, _resolution), do: {:error, :unauthenticated}
 
   @doc """
   Return the total media size for an actor
@@ -108,7 +98,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Picture do
         context: %{current_user: %User{} = user}
       }) do
     if can_get_actor_size?(user, actor_id) do
-      {:ok, Media.media_size_for_actor(actor_id)}
+      {:ok, Medias.media_size_for_actor(actor_id)}
     else
       {:error, :unauthorized}
     end
@@ -125,7 +115,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Picture do
         context: %{current_user: %User{} = logged_user}
       }) do
     if can_get_user_size?(logged_user, user_id) do
-      {:ok, Media.media_size_for_user(user_id)}
+      {:ok, Medias.media_size_for_user(user_id)}
     else
       {:error, :unauthorized}
     end
@@ -133,6 +123,17 @@ defmodule Mobilizon.GraphQL.Resolvers.Picture do
 
   def user_size(_parent, _args, _resolution), do: {:error, :unauthenticated}
 
+  @spec transform_media(Media.t()) :: map()
+  defp transform_media(%Media{id: id, file: file}) do
+    %{
+      name: file.name,
+      url: file.url,
+      id: id,
+      content_type: file.content_type,
+      size: file.size
+    }
+  end
+
   @spec can_get_user_size?(User.t(), integer()) :: boolean()
   defp can_get_actor_size?(%User{role: role} = user, actor_id) do
     role in [:moderator, :administrator] || owns_actor?(User.owns_actor(user, actor_id))