Problem with visitor id

[josefwilhelm-innio]

Hey,

Problem on matomo_tracker 1.4.1 and above.

If I call
await MatomoTracker.instance.initialize( siteId: 123, url: 'someurl', );

I always get

It works fine on 1.4.0.

Maybe you could look into it.

[TesteurManiak]

Hi,

I was not able to reproduce your issue with the example project using either version 1.5.0 or the future 1.6.0 available on master branch.

Can you please provide a minimal reproducible example ?

[josefwilhelm-innio]

I will try when I find time

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[luckyrat]

The problem is in matomo.dart:

final aVisitorId = visitorId ??
        _prefs?.getString(kVisitorId) ??
        const Uuid().v4().replaceAll('-', '').substring(0, 16);

This does not apply the hyphen replacement and substring to the stored value, which in the case of anyone that has used an earlier version of this library, will result in a full UUID string being passed to Visitor.init

Since I don't know why we must limit the visitorId to a 16 char hex string, I'm unsure what the correct resolution is. If the full UUID has been used previously then manipulating it into a new 16 char string will result in a loss of ongoing association of every visitor. But if anything other than a 16 char hex string is incompatible with matomo, we can't just remove the requirement for that ID format.

[TesteurManiak]

@luckyrat this is what Matomo's documentation says:

_id (recommended) — The unique visitor ID, must be a 16 characters hexadecimal string. [...]

[MeixDev]

I will note that, according to the git blame, we added the whole substring mechanism on version 1.0.1, which was published on pub.dev on Apr 13, the same day we published the initial 1.0.0

Unless you're coming from the package we worked from by poitch, the case of "using an earlier version of this library" shouldn't arise

[luckyrat]

Unless you're coming from the package we worked from by poitch, the case of "using an earlier version of this library" shouldn't arise

Indeed that is the case for me (and I suspect for the OP). I presume that you were motivated to create the fork because the original is no longer maintained so you'll probably find a lot of people moving over to it in order to get compatibility with the latest build environments.

I suppose that for a fork I would expect either documentation on how to migrate from the original package or backwards-compatibility with it so I am just hoping that this information can help us get to that situation. I appreciate that you've taken the time to pick up from where the previous package owner left off. If there's anything in particular you'd like me to look into or beta releases to test out in future, please just ask.

I'm not actually sure what happens with the UUIDs that are stored from the old package. I have checked in Matomo and find only 16 character hex strings as the documentation points to so maybe they are manipulated by the old package or Matomo to become identical to your format anyway.

[TesteurManiak]

I suppose that for a fork I would expect either documentation on how to migrate from the original package or backwards-compatibility with it so I am just hoping that this information can help us get to that situation.

I have to disagree here, the point of this fork was never to ensure backwards-compatibility. In this version of the package we wanted to implement our own vision of how tracking with Matomo could be implemented in Flutter, this is why you won't have TraceableStatelessWidget, TraceableStatefulWidget or TraceableInheritedWidget anymore or that you'll have assertion to ensure that the visitorId comply to the 16 characters rule from Matomo.

[TesteurManiak]

To recenter on the original issue, @josefwilhelm-innio if you cannot provide a minimal reproducible example of the issue you are encountering I'll have to close the issue, but feel free to re-open it if you have more info on its reproducibility.

[luckyrat]

I have to disagree here, the point of this fork was never to ensure backwards-compatibility.

That's no problem and entirely reasonable. I'm not sure what the objection is to offering documentation to outline how one can migrate from the older package.

By the way, I'm not even really thinking of backwards compatibility in the sense of the internal API changes - I'm only concerned with maintaining ongoing tracking accuracy within Matomo for existing users.

As far as a reproducible example goes, I can't see how that can be possible since the issue occurs only when pre-existing data from the old package is already present.

[TesteurManiak]

As far as a reproducible example goes, I can't see how that can be possible since the issue occurs only when pre-existing data from the old package is already present.

I don't think the issue comes from pre-existing data from the old package as the OP said himself:

Problem on matomo_tracker 1.4.1 and above. [...] It works fine on 1.4.0.

While v1.4.1 included a fix that changed the way the visitorId is created I wasn't able to reproduce the behavior described above and the issue he got is definitely thrown by this assertion:

assert(
  visitorId == null || visitorId.length == 16,
  'visitorId must be 16 characters',
);

Which is performed before checking any data from the shared preferences.

But the thing is that he didn't specify any visitorId parameter to the method so my only guess is that he didn't give the full code he used, hence why I've asked for a minimal reproducible example.

[luckyrat]

The assertion is being thrown here: https://github.com/Floating-Dartists/matomo-tracker/blob/main/lib/src/visitor.dart#L23

Since there are 2 assertions for the same thing, I've fixed this issue by removing the problematic assertion and leaving the one which enforces the correct use of a 16 char string through the initialize public API.

Because this fork uses the same shared preferences key as the original package, the only other feasible way to migrate from the original package would require reading from the shared preferences key and adjusting it before we even try to invoke this package. Given that this all happens on the critical startup path for most apps, I hope that the approach I've suggested will be acceptable. I'll open a PR in a sec.

[TesteurManiak]

Closing as should be fixed in #38