caronc / apprise-api

A lightweight REST framework that wraps the Apprise Notification Library
https://hub.docker.com/r/caronc/apprise
MIT License
638 stars 56 forks source link

Tags AND, OR logic not working as expected #135

Closed ThorpeJosh closed 1 year ago

ThorpeJosh commented 1 year ago

:beetle: Describe the bug The tagging logic described by https://github.com/caronc/apprise-api#tagging does not seem to be working.

:bulb: Screenshots and Logs

curl -s -X GET "http://10.0.10.10:8000/json/urls/abc"| jq
{
  "tags": [
    "signal",
    "email",
  ],
  "urls": [
    {
      "url": "mailtos://****:******@gmail.com/**************",
      "tags": [
        "email"
      ]
    },
    {
      "url": "signal://signal-api:8080/+*******/+*****",
      "tags": [
        "signal"
      ]
    }
  ]
}

Tag OR: Expected behavior is to send to only signal tag, and then email if it is unsuccessful. Actual behavior is it sends to both.

curl -X POST \
             -F "tag=signal, email" \
             -F "body=test body" \
             -F "title=test title" \
             "http://10.0.10.10:8000/notify/abc"
2023-09-12 15:11:35,318 [INFO] apprise: Applying Google Mail Defaults
2023-09-12 15:11:35,323 [INFO] apprise: Loaded 2 entries from memory://
2023-09-12 15:11:35,324 [INFO] apprise: Notifying 2 service(s) with threads.
2023-09-12 15:11:36,017 [INFO] apprise: Sent 1 Signal API notification to +*********.
2023-09-12 15:11:37,853 [INFO] apprise: Sent Email notification to "******@gmail.com".

Tag AND: Expected behavior is to send to both signal and email tag.

curl -X POST \
             -F "tag=signal+email" \
             -F "body=test body" \
             -F "title=test title" \
             "http://10.0.10.10:8000/notify/abc"
Unsupported characters found in tag definition.
curl -X POST \
             -F "tag=signal email" \
             -F "body=test body" \
             -F "title=test title" \
             "http://10.0.10.10:8000/notify/abc"
2023-09-12 15:14:55,670 [INFO] apprise: Applying Google Mail Defaults
2023-09-12 15:14:55,674 [INFO] apprise: Loaded 2 entries from memory://

:computer: Your System Details:

:crystal_ball: Additional context I also tried the same thing with the application/json type data

curl -X POST -d '{"tag":"signal, email", "title": "test title", "body":"test body"}' \
-H "Content-Type: application/json" \
http://10.0.10.10:8000/notify/abc
2023-09-12 15:22:18,837 [INFO] apprise: Applying Google Mail Defaults
2023-09-12 15:22:18,843 [INFO] apprise: Loaded 2 entries from memory://
2023-09-12 15:22:18,843 [INFO] apprise: Notifying 2 service(s) with threads.
2023-09-12 15:22:19,546 [INFO] apprise: Sent 1 Signal API notification to +*******.
2023-09-12 15:22:21,447 [INFO] apprise: Sent Email notification to "*******@gmail.com".

curl -X POST -d '{"tag":"signal email", "title": "test title", "body":"test body"}' \
-H "Content-Type: application/json" \
http://10.0.10.10:8000/notify/abc
{"error": "One or more notification could not be sent."}

curl -X POST -d '{"tag":"signal+email", "title": "test title", "body":"test body"}' \
-H "Content-Type: application/json" \
http://10.0.10.10:8000/notify/abc
{"error": "Unsupported characters found in tag definition."}
caronc commented 1 year ago

I'll look into why the + isn't working.

But a comma delimiters is OR and space is the AND

ThorpeJosh commented 1 year ago

Correct me if I am wrong but that is exactly what I posted.

tag=signal, email sends to both tags when it should be an OR (so it is actually behaving as an AND?) tag=signal email does not send any notifications when it should be an AND (ie send to both?)

caronc commented 1 year ago

It's a logical and & or. Not to be taken in the literal sense.

Think of it in this context instead:

I'm not sure if this helps?

ThorpeJosh commented 1 year ago

Ok I misunderstood how tagging AND/OR logic works. It is not actually a fallback for if one notification service fails. It is a way of selecting urls that meet mutli-tag criteria.

The confusing part is the documentation is contradicting itself. For example on this page a comma or space implies an AND:

If you identify more than one element on the same --tag using a space and/or comma, then these get treated as an AND. 
# notify only the services that have both a team and email tag
# In this example, there is only one match.
apprise -v --title="Meeting this Friday" \
   --body="Guys, there is a meeting this Friday with our director." \
   --tag=team,email

But then the apprise-api docs say the opposite.

ThorpeJosh commented 1 year ago

It's a logical and & or. Not to be taken in the literal sense.

Think of it in this context instead:

  • a b : only relay the notification along to services that have both a AND b tags. If only one or the other, or none at all, then skip a,b: contrary to the above, as long as either tag a or tag b is found assigned to any notification at all, then include it
  • a b c, d: this one is just to show how you can increase the complexity of the logical checks. A service must have ALL tags a, b and c associated with it to be included. Also, the result notification set will also include any service that has the d tag as well. If there was a case where a service was assigned a, b, c and d: this would match the expression in both means, but the notification would only be sent once.

I'm not sure if this helps?

Yeah, I just read this. I misunderstood how the tagging worked. The docs were a bit confusing as I was reading both apprise and the apprise-api and the cli docs all at once.

caronc commented 1 year ago

The + should still work, you found an honest bug; so let's keep this ticket open!🙂

caronc commented 1 year ago

So for the first part, it turns out the + is not supported; it's a bit of a task to add it (as the underlining Apprise Library also doesn't support it). I think the confusion comes from the URL GET Params where + is the same as a space.

The logic is is as discussed above (also documented here)

I did take your ticket to realize I had poor test coverage in this area and improved it significantly in the above PR.

I updated the Wiki to provide better examples for this as well.

Closing this ticket off as there is nothing further to do.

ThorpeJosh commented 1 year ago

Great. Thank you @caronc !