Altinn / altinn-events

Altinn platform microservice for handling events
MIT License
1 stars 2 forks source link

Subscription Creation Fails Due to Non-Specific Type Filter Condition #551

Closed chrrust closed 3 months ago

chrrust commented 3 months ago

Description of the bug

I'm trying to create a new subscription for the resource urn:altinn:resource:dgm-internal-events without specifying an event type filter. Generic events are published for this resource.

I already have a subscription on that resource, but that subscription specifies an event type filter. When I try to create a new subscription without the event type filter, I get returned the already existing subscription with an event type filter. Thus, no new subscription is created.

As far as I can see, this happens because in SubscriptionService.CompleteSubscriptionCreation the _repository.CreateSubscription statement is happening only if a subscription could not be found. However, the _repository.FindSubscription function that does the following condition:

AND (_typefilter IS NULL OR s.typefilter = _typefilter)

So, because my input to the creation of the subscription is null, it will return whatever subscription it finds for that resource. This makes the _repository.CreateSubscription function to never be called, and instead that subscription that was found is queued for validation and returned.

I'd expect a check to see if the found subscription is indeed the attempted created subscription.

Steps To Reproduce

  1. Create a subscription for a resource that produces generic events, and use a type filter
  2. Create a new subscription for the same resource, but without a type filter

Now see that the subscription from step 1 is returned, and not the one from step 2. Only one subscription will have been created, and that's the first one.

Additional Information

        internal async Task<(Subscription Subscription, ServiceError Error)> CompleteSubscriptionCreation(Subscription eventsSubscription)
        {
            // .. Omitted for brevity...
            Subscription subscription = await _repository.FindSubscription(eventsSubscription, CancellationToken.None);

            subscription ??= await _repository.CreateSubscription(eventsSubscription, eventsSubscription.SourceFilter?.GetMD5Hash());

            await _queue.EnqueueSubscriptionValidation(JsonSerializer.Serialize(subscription));
            return (subscription, null);
        }
CREATE OR REPLACE FUNCTION events.find_subscription(
    _resourcefilter character varying,
    _sourcefilter character varying,
    _subjectfilter character varying,
    _typefilter character varying,
    _consumer character varying,
    _endpointurl character varying)
    RETURNS TABLE(id bigint, resourcefilter character varying, sourcefilter character varying, subjectfilter character varying, typefilter character varying, consumer character varying, endpointurl character varying, createdby character varying, validated boolean, "time" timestamp with time zone)
    LANGUAGE 'plpgsql'
AS $BODY$

BEGIN
RETURN query
    SELECT
        s.id, s.resourcefilter, s.sourcefilter, s.subjectfilter, s.typefilter, s.consumer, s.endpointurl, s.createdby, s.validated, s."time"
    FROM
        events.subscription s
    WHERE
        s.resourcefilter = _resourcefilter
        AND (s.sourcefilter IS NULL OR s.sourcefilter = _sourcefilter)
        AND (_subjectfilter IS NULL OR s.subjectfilter = _subjectfilter)
        AND (_typefilter IS NULL OR s.typefilter = _typefilter)
        AND s.consumer = _consumer
        AND s.endpointurl = _endpointurl;
END
$BODY$;
chrrust commented 3 months ago

I can add that this is also problematic in our case if we would decide to do a workaround for #550 by creating a subscription for all event types on a resource by omitting the event type filter. This because we already have subscriptions for each event type, and I'd be risking losing out on events if I would have to delete the existing subscriptions on the resource in order to create one for all of them.

acn-sbuad commented 3 months ago

Fix verified in at22