Details

      Description

      In mod_pubsub:publish_item, in case of pubsub#persist_items = 0, max items should be equal to 0.

      1. EJAB_1434_21x.patch
        0.9 kB
        Karim Gemayel
      2. EJAB_1434_30.patch
        0.9 kB
        Karim Gemayel

        Expenses

          Activity

          Hide
          cromain@process-one.net Christophe Romain added a comment -

          the max-items should not be fixed, if persist_items is false, we just write nothing into the item database whatever max_items is.
          to follow the spec, we added the handling of last_item_cache.

          A service MAY cache the last item published to a node (even if the "persistent-items" option is set to false); if it does default "cache-last-item" to true, it SHOULD send the last published item (or notification thereof) to subscribed entities based on configuration of the "send_last_published_item" field.

          see set_cached_item function, we use mnesia pubsub_last_item table if cache_last_item is true

          Show
          cromain@process-one.net Christophe Romain added a comment - the max-items should not be fixed, if persist_items is false, we just write nothing into the item database whatever max_items is. to follow the spec, we added the handling of last_item_cache. A service MAY cache the last item published to a node (even if the "persistent-items" option is set to false); if it does default "cache-last-item" to true, it SHOULD send the last published item (or notification thereof) to subscribed entities based on configuration of the "send_last_published_item" field. see set_cached_item function, we use mnesia pubsub_last_item table if cache_last_item is true
          Hide
          dsd Daniel Drake added a comment -

          This patch caused a regression for me - see EJAB-1533 - the data that I publish gets lost by the server. But it could be due to misconfigured ejabberd or bad message - I'm not quite grasping how this is supposed to work.

          One thing stands out in this patch: max_items explicitly does check for the persist_items case:

          max_items(Host, Options) ->
          case get_option(Options, persist_items) of
          true ->
          case get_option(Options, max_items) of
          false -> unlimited;
          Result when (Result < 0) -> 0;
          Result -> Result
          end;
          false ->
          ...

          So it is not clear to me that removing the call to it is definitely correct - if something is wrong with the logic of max_items in the non-persist case, shouldn't max_items be fixed instead?

          (Sorry if my lack of knowledge of XMPP and erlang is shining through - I'm learning as I go)

          Show
          dsd Daniel Drake added a comment - This patch caused a regression for me - see EJAB-1533 - the data that I publish gets lost by the server. But it could be due to misconfigured ejabberd or bad message - I'm not quite grasping how this is supposed to work. One thing stands out in this patch: max_items explicitly does check for the persist_items case: max_items(Host, Options) -> case get_option(Options, persist_items) of true -> case get_option(Options, max_items) of false -> unlimited; Result when (Result < 0) -> 0; Result -> Result end; false -> ... So it is not clear to me that removing the call to it is definitely correct - if something is wrong with the logic of max_items in the non-persist case, shouldn't max_items be fixed instead? (Sorry if my lack of knowledge of XMPP and erlang is shining through - I'm learning as I go)
          Hide
          kgemayel Karim Gemayel (Inactive) added a comment -

          Attached patches for 2.1.x and 3.0.

          Show
          kgemayel Karim Gemayel (Inactive) added a comment - Attached patches for 2.1.x and 3.0.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development