Bug 21079 - Unify metadata schema across backends
Summary: Unify metadata schema across backends
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Andrew Isherwood
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 20996
Blocks: 20640
  Show dependency treegraph
 
Reported: 2018-07-16 10:22 UTC by Andrew Isherwood
Modified: 2019-10-14 19:57 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21079: Unify metadata schema across backends (5.36 KB, patch)
2018-07-17 14:02 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 21079: Unify metadata schema across backends (5.34 KB, patch)
2018-07-17 14:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 21079: Unify metadata schema across backends (5.30 KB, patch)
2018-10-01 14:33 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 21079: Unify metadata schema across backends (5.35 KB, patch)
2018-10-01 14:43 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 21079: Unify metadata schema across backends (5.40 KB, patch)
2018-10-09 14:40 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 21079: Unify metadata schema across backends (5.45 KB, patch)
2018-10-31 15:02 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21079: (follow-up) Move from Try::Tiny to eval (2.31 KB, patch)
2018-11-08 14:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2018-07-16 10:22:52 UTC
Currently different ILL backends return metadata in disparate property names, for example, an article will have the property "./metadata/titleLevel/title" for it's containing journal title on the BLDSS backend, whereas the FreeForm backend will store it in "title".

Some work was done by Martin in Bug 20640 that introduced the mapping of certain metadata properties to a common schema. For example, the property described above is mapped to "container_title". This mapping work currently only exists for the BLDSS backend.

Work on Bug 21063 has highlighted the fact that these metadata properties vary according to the backend. This bug proposes extending the mapping to other backends. This will be carried out for the FreeForm backend by Andrew Isherwood.
Comment 1 Andrew Isherwood 2018-07-16 10:31:08 UTC
It's more than likely this work won't touch core Koha, I just thought it was worth creating a bug to highlight that this will need to be done for any other backends in existence. I'll post details of what was done to the FreeForm backend once it is complete.
Comment 2 Andrew Isherwood 2018-07-17 14:02:55 UTC
Created attachment 77035 [details] [review]
Bug 21079: Unify metadata schema across backends

This patch contains a database upgrade that takes appropriate metadata
properties relating to articles in the FreeForm backend and creates
metadata that corresponds with the metadata being created by the BLDSS
backend. This enables us to create templates that can display metadata
equally across any backends that contain this metadata;

To test:
- Ensure you have at least one article request created with the FreeForm backend
- Check the metadata for the request:
  => TEST: You should have properties such as 'article_title', 'article_author' populated
  => TEST: You should NOT have properties such as 'container_title' &
  'pages'
- Run the upgrade
  => TEST: For the same requests you should now have the following
  properties:
    - container_title (this should correspond with 'title')
    - title (this should correspond with 'article_title')
    - pages (this should correspond with 'article_pages')
    - author (this should correspond with 'article_author')
Comment 3 Andrew Isherwood 2018-07-17 14:04:25 UTC
Comment on attachment 77035 [details] [review]
Bug 21079: Unify metadata schema across backends

Patch contains debugging code, obsoleting
Comment 4 Andrew Isherwood 2018-07-17 14:06:20 UTC
Created attachment 77036 [details] [review]
Bug 21079: Unify metadata schema across backends

This patch contains a database upgrade that takes appropriate metadata
properties relating to articles in the FreeForm backend and creates
metadata that corresponds with the metadata being created by the BLDSS
backend. This enables us to create templates that can display metadata
equally across any backends that contain this metadata;

To test:
- Ensure you have at least one article request created with the FreeForm backend
- Check the metadata for the request:
  => TEST: You should have properties such as 'article_title', 'article_author' populated
  => TEST: You should NOT have properties such as 'container_title' &
  'pages'
- Run the upgrade
  => TEST: For the same requests you should now have the following
  properties:
    - container_title (this should correspond with 'title')
    - title (this should correspond with 'article_title')
    - pages (this should correspond with 'article_pages')
    - author (this should correspond with 'article_author')
Comment 5 Andrew Isherwood 2018-07-17 14:07:32 UTC
So this bug did touch Koha core after all. We need to transform existing article FreeForm request metadata in order to copy some properties to new metadata properties with names that correspond with our unified schema.
Comment 6 Andrew Isherwood 2018-10-01 14:33:41 UTC
Created attachment 79720 [details] [review]
Bug 21079: Unify metadata schema across backends

This patch contains a database upgrade that takes appropriate metadata
properties relating to articles in the FreeForm backend and creates
metadata that corresponds with the metadata being created by the BLDSS
backend. This enables us to create templates that can display metadata
equally across any backends that contain this metadata;

To test:
- Ensure you have at least one article request created with the FreeForm backend
- Check the metadata for the request:
  => TEST: You should have properties such as 'article_title', 'article_author' populated
  => TEST: You should NOT have properties such as 'container_title' &
  'pages'
- Run the upgrade
  => TEST: For the same requests you should now have the following
  properties:
    - container_title (this should correspond with 'title')
    - title (this should correspond with 'article_title')
    - pages (this should correspond with 'article_pages')
    - author (this should correspond with 'article_author')
Comment 7 Andrew Isherwood 2018-10-01 14:43:30 UTC
Created attachment 79725 [details] [review]
Bug 21079: Unify metadata schema across backends

This patch contains a database upgrade that takes appropriate metadata
properties relating to articles in the FreeForm backend and creates
metadata that corresponds with the metadata being created by the BLDSS
backend. This enables us to create templates that can display metadata
equally across any backends that contain this metadata;

To test:
- Ensure you have at least one article request created with the FreeForm backend
- Check the metadata for the request:
  => TEST: You should have properties such as 'article_title', 'article_author' populated
  => TEST: You should NOT have properties such as 'container_title' &
  'pages'
- Run the upgrade
  => TEST: For the same requests you should now have the following
  properties (if their values were originally populated):
    - container_title (this should correspond with what *was* 'title')
    - title (this should correspond with 'article_title')
    - pages (this should correspond with 'article_pages')
    - author (this should correspond with 'article_author')
Comment 8 PTFS Europe Sandboxes 2018-10-09 14:40:38 UTC
Created attachment 80288 [details] [review]
Bug 21079: Unify metadata schema across backends

This patch contains a database upgrade that takes appropriate metadata
properties relating to articles in the FreeForm backend and creates
metadata that corresponds with the metadata being created by the BLDSS
backend. This enables us to create templates that can display metadata
equally across any backends that contain this metadata;

To test:
- Ensure you have at least one article request created with the FreeForm backend
- Check the metadata for the request:
  => TEST: You should have properties such as 'article_title', 'article_author' populated
  => TEST: You should NOT have properties such as 'container_title' &
  'pages'
- Run the upgrade
  => TEST: For the same requests you should now have the following
  properties (if their values were originally populated):
    - container_title (this should correspond with what *was* 'title')
    - title (this should correspond with 'article_title')
    - pages (this should correspond with 'article_pages')
    - author (this should correspond with 'article_author')

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>
Comment 9 Josef Moravec 2018-10-31 15:02:50 UTC
Created attachment 81740 [details] [review]
Bug 21079: Unify metadata schema across backends

This patch contains a database upgrade that takes appropriate metadata
properties relating to articles in the FreeForm backend and creates
metadata that corresponds with the metadata being created by the BLDSS
backend. This enables us to create templates that can display metadata
equally across any backends that contain this metadata;

To test:
- Ensure you have at least one article request created with the FreeForm backend
- Check the metadata for the request:
  => TEST: You should have properties such as 'article_title', 'article_author' populated
  => TEST: You should NOT have properties such as 'container_title' &
  'pages'
- Run the upgrade
  => TEST: For the same requests you should now have the following
  properties (if their values were originally populated):
    - container_title (this should correspond with what *was* 'title')
    - title (this should correspond with 'article_title')
    - pages (this should correspond with 'article_pages')
    - author (this should correspond with 'article_author')

Signed-off-by: Stephen Graham <s.graham4@herts.ac.uk>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 10 Nick Clemens 2018-11-08 13:25:55 UTC
This adds Try::Tiny to the updatedatabase, I think this is okay and there seems to be agreement polling the QA team.

The AutoCommit and RaiseError flags need to be reset after the attempt.

It appears the version is set even if the update fails, is this intentional?
Comment 11 Andrew Isherwood 2018-11-08 14:12:04 UTC
(In reply to Nick Clemens from comment #10)
> This adds Try::Tiny to the updatedatabase, I think this is okay and there
> seems to be agreement polling the QA team.

There will have been some logic behind taking this approach, though I can't remember what it was now. Investigating, the common approach in the DBI community appears to be to "eval" the contents of what I've got inside my "try" block, then test for "$@" to determine an error. I'm happy to switch to that approach if it would make people happier?

> The AutoCommit and RaiseError flags need to be reset after the attempt.

Good spot! Thanks.

> It appears the version is set even if the update fails, is this intentional?

Nope, an error, thanks!
Comment 12 Andrew Isherwood 2018-11-08 14:31:41 UTC
Created attachment 82092 [details] [review]
Bug 21079: (follow-up) Move from Try::Tiny to eval

In line with similar examples already in Koha, we now eval the
transaction, instead of putting it in a try/catch block.

This patch also addressed the other comments made in
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21079#c10

- AutoCommit & RaiseError are now reset
- Don't set the version if the update fails
Comment 13 Nick Clemens 2018-11-08 18:22:54 UTC
Awesome work all!

Pushed to master for 18.11
Comment 14 Martin Renvoize 2018-11-16 13:53:12 UTC
The enhancement will not be backported to 18.05.x series.