[FOLIO-3447] user metadata fields are absent for reference and sample data Created: 14/Mar/22  Updated: 30/Mar/22  Resolved: 24/Mar/22

Status: Closed
Project: FOLIO
Components: None
Affects versions: None
Fix versions: None

Type: Bug Priority: P2
Reporter: Zak Burke Assignee: Unassigned
Resolution: Won't Do Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original estimate: Not Specified

Issue links:
Relates
relates to BF-224 Loan Type Created by [User] not popul... Draft
Sprint: CP: R3 2022 roadmap, DevOps Sprint 135
Development Team: Core: Platform
RCA Group: TBD

 Description   

Overview: The fields metadata.createdByUserId and metadata.updatedByUserId are not present for reference data or sample data values (e.g. /coursereserves/coursetypes, /groups, /acquisistions-units/units, /instance-formats, etc).

How to reproduce:

Invoke POST /_/tenant with neither X-Okapi-User-Id nor X-Okapi-Token header.

This is the code that fills the metadata property: https://github.com/folio-org/raml-module-builder/blob/v33.2.7/domain-models-runtime/src/main/java/org/folio/rest/tools/utils/MetadataUtil.java#L33-L55

It creates a metadata property like this:

"metadata": {
  "createdDate": "2022-03-15T10:34:03.966",
  "updatedDate": "2022-03-15T10:34:03.966+00:00"
} 

How to fix:

Invoke POST /_/tenant with X-Okapi-User-Id header, for example "X-Okapi-User-Id: 715d1d4b-0709-4e71-9066-761ce19674b9", or with a valid token with a user id.

Then MetadataUtil creates a metadata property like this:

"metadata": {
  "createdDate": "2022-03-15T10:34:03.966",
  "updatedDate": "2022-03-15T10:34:03.966+00:00",
  "createdByUserId": "715d1d4b-0709-4e71-9066-761ce19674b9",
  "updatedByUserId": "715d1d4b-0709-4e71-9066-761ce19674b9"
}

This works for all examples listed above (tested against master branch on March 15, 2022):

  • /coursereserves/coursetypes (mod-courses)
  • /groups (mod-users)
  • /acquisistions-units/units (mod-orders-storage)
  • /instance-formats (mod-inventory-storage)

Note that mod-users might not be enabled for the tenant at that time.

Background:

I don't remember it working this way before, but this also reproduces in folio-juniper and folio-kiwi, which suggests it did work this way before and I just didn't notice. It also reproduces on bugfest ( BF-244 Closed ), an environment that is built very differently (data are migrated from previous FOLIO releases, rather than reference and sample data being installed fresh).

In any case, there is an expectation among librarians viewing the UI that reference data will have these metadata fields populated, but they aren't and Prokopvych (the module owners) don't understand why. Maybe it's a problem with how the module stores these values or sends them to Okapi (a problem for the module owners) or maybe RMB has changed its expectations (a problem for the RMB owners) or maybe the system's assumptions about these data during setup and migration are incorrect (a problem for ref-env and bugfest devops teams).



 Comments   
Comment by Marc Johnson [ 15/Mar/22 ]

Jakub Skoczen John Malconian Adam Dickmeiss Julian Ladisch Wayne Schneider

It is my understanding that these properties are populated automatically by RMB when the incoming request for them contains specific headers (either {{X-Okapi-User-Id or parsing the token maybe).

That suggests to me that the request made by the Okapi to the Tenant API for each module has to receive those headers. And thus, the request to Okapi to trigger that (likely the install endpoint) would too.

How wrong is my understanding of how the creation of reference / sample records works?

If it's not far off, do the reference environments provide those headers when installing the modules for the diku tenant?

Comment by Wayne Schneider [ 15/Mar/22 ]

Reference/sample data is loaded by the module when the _\tenant API is called with the loadReference=true and/or loadSample=true parameters. In the reference environments, this is trigger by a POST request to the /_/proxy/tenants/diku/install endpoint with the list of modules to be enabled. The call to that API for is made in the context of the supertenant tenant (that is, with no x-okapi-tenant header) and without an x-okapi-token or x-okapi-user-id. From what I understand, the RMB modules call their own API when they load reference/sample data; I don't know the mechanics of those calls (i.e. if they are proxied through Okapi or indeed if they are using the network APIs at all).

Comment by Julian Ladisch [ 15/Mar/22 ]

When invoking POST /_/tenant for mod-users one need to know the user id of a user to put into metadata in advance because that user doesn't exist yet.

Comment by Julian Ladisch [ 15/Mar/22 ]

Zak Burke Which user id should be put into metadata.createdByUserId when creating the /groups sample data? There doesn't exist any user at that time.

Comment by Marc Johnson [ 15/Mar/22 ]

Wayne Schneider

The call to that API for is made in the context of the supertenant tenant (that is, with no x-okapi-tenant header) and without an x-okapi-token or x-okapi-user-id. From what I understand, the RMB modules call their own API when they load reference/sample data; I don't know the mechanics of those calls (i.e. if they are proxied through Okapi or indeed if they are using the network APIs at all).

Does that mean that both Okapi's install endpoint and thus the Tenant APIs in the modules are running outside of the context of a user, meaning that there cannot be a user associated with the creation of reference or sample data?

Comment by Julian Ladisch [ 15/Mar/22 ]

Yes, the Okapi APIs don't take a user id or a token:

  • POST /_/proxy/tenants/{tenant_id}/modules
  • POST /_/proxy/tenants/{tenant_id}/install

https://s3.amazonaws.com/foliodocs/api/okapi/r/okapi.html

We might change Okapi so that these APIs take a user id or token and pass it to the /_/tenant API of the module. Then the metadata will be populated as requested.

Comment by Marc Johnson [ 15/Mar/22 ]

Julian Ladisch

Yes, the Okapi APIs don't take a user id or a token:

  • POST /_/proxy/tenants/
    Unknown macro: {tenant_id}

    /modules

    • POST /_/proxy/tenants/

    /install

Oh, so no environment that uses the install endpoint will ever create reference or sample data (assuming RMB's implementation) will have a created / updated by on initial creation?

Have I interpreted that correctly?

Comment by Zak Burke [ 24/Mar/22 ]

This is a limitation of the system. These values are installed via Okapi's .../install endpoint, which does not take a token or user-id. IOW, the problem is not "we're not correctly providing the user-id to an endpoint that accepts one", the problem is "the endpoint does not accept a user-id", and that is (at least for now) by design.

Comment by Marc Johnson [ 29/Mar/22 ]

Zak Burke

What does won't do mean in the context of this issue? That we have decided not to change / remove this limitation of the system or something else?

Comment by Zak Burke [ 30/Mar/22 ]

Marc Johnson, I closed this prior to seeing your most recent comments on BF-224 Draft , and prior to a recent conversation with Anton Emelianov where he described how automated testing could be affected by this missing data. At the time I filed this, I expected it to be a problem like "We forgot to include the user's ID in a POST request" with a simple solution (include the ID). At the time I closed this it had become clear (based on comments from Wayne and Julian) that the solution was not simply "not simple" but in fact fundamentally impossible given the nature of how these data are installed. 

In short, I filed it thinking it would be an easy fix for a minor glitch and closed it on learning it would require fundamental changes to the system. 

Part of me wonders though: in the UI, we use a shared component to display object metadata (created-at, created-by) that accepts a "systemId" prop to label changes made by automated processes. Could we leverage similar logic here and sidestep this whole thing? e.g. if createdBy is empty, the UI shows "System" or "Root" or "Admin" or whatever. That won't benefit API users, but maybe it would allow us to sidestep this thorny issue in a useful way. 

Comment by Marc Johnson [ 30/Mar/22 ]

Zak Burke

In short, I filed it thinking it would be an easy fix for a minor glitch and closed it on learning it would require fundamental changes to the system.

Thank you for clarifying that.

It was ambiguous to me whether closing it meant we won't address it now or we won't address the design limitation at all.

Comment by Marc Johnson [ 30/Mar/22 ]

Zak Burke

in the UI, we use a shared component to display object metadata (created-at, created-by) that accepts a "systemId" prop to label changes made by automated processes. Could we leverage similar logic here and sidestep this whole thing? e.g. if createdBy is empty, the UI shows "System" or "Root" or "Admin" or whatever.

Yes, we could do that. I think it's a product question as to whether that is a viable option. I do think it's rather disingenuous (but then folks are ok with the existing system user workaround too).

To be honest, I thought that (presented missing as automated process) was what the shared component already did.

I thought that was how we got around blank updated by properties for background tasks triggered by a timer e.g. ageing loaned items to lost and charging fees / fines.

I think we are missing a modelling trick here:

  • is it legitimate for records to have no created by / updated by (I think we removed that constraint when we realised automated tasks as currently implemented wouldn't work)?
  • if it isn't, then we need to model workflows in a way that every step happens in the context of a user

That won't benefit API users, but maybe it would allow us to sidestep this thorny issue in a useful way.

At least for the reference UI. Any folks building their own interfaces will need to implement their own reaction.

Comment by Zak Burke [ 30/Mar/22 ]

Marc Johnson, there are multiple places where metadata is displayed. In one place, we use a shared component to show "System" given a system-id; in other places we do nothing. My half-baked proposal is for the "other places" to show "System" when given nothing. 

Comment by Marc Johnson [ 30/Mar/22 ]

Zak Burke

there are multiple places where metadata is displayed. In one place, we use a shared component to show "System" given a system-id; in other places we do nothing. My half-baked proposal is for the "other places" to show "System" when given nothing.

Sure, I understood the proposal.

For some reason, I thought we already did say system for blank created / updated by (which obviously we don't, otherwise folks wouldn't have noticed this issue)

Out of interest, what would be an example of a system ID?

Comment by Zak Burke [ 30/Mar/22 ]

Marc Johnson, handling of "System" users was described in UIDATIMP-156 Closed and added in PR #453 where the description includes, "if it contains only zeros then assume that this is System ... render system user which has a default value". The actual implementation is not hard-coded to "only zeros" but is configurable and could handle empty string, provided all necessary props are present.

Comment by Marc Johnson [ 30/Mar/22 ]

Zak Burke

handling of "System" users was described in UIDATIMP-156 Closed and added in PR #453

Thank you for repeating it here (and including references). That isn't an area I'm familiar with.

"if it contains only zeros then assume that this is System ... render system user which has a default value". The actual implementation is not hard-coded to "only zeros" but is configurable and could handle empty string, provided all necessary props are present.

Ah, we chose the nil UUID to denote a system user in that context. I didn't know that. Thank you, that is useful to know.

Generated at Thu Feb 08 23:28:12 UTC 2024 using Jira 1001.0.0-SNAPSHOT#100246-sha1:7a5c50119eb0633d306e14180817ddef5e80c75d.