[Bug 43848] New: Share SIP request parsing and building between server, emulator and tester
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43848 Bug ID: 43848 Summary: Share SIP request parsing and building between server, emulator and tester Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: SIP2 Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org Depends on: 42769 Target Milestone: --- Bug 42769 adds a tool to the staff interface that runs a SIP2 message through the real SIP handlers (Koha::SIP2::MessageTester). To do that it has to parse a request and build a server context the way the SIP server does, and build requests the way a terminal does. All of this is currently buried in code that also does something else, so each caller ends up with its own copy: - C4::SIP::Sip::MsgType::handle both parses a request and answers it. - C4::SIP::Sip::MsgType::login_core both authenticates a terminal and builds the account, institution, policy and ILS connection it works with. - misc/sip_cli_emulator.pl builds every request a terminal sends, but as subs inside a script, so nothing else can use them. This bug pulls the shared parts out, so the SIP server, the SIP command line emulator and the SIP2 message tester use the same code: - MsgType::parse_message: the part of handle() that sets the field delimiter, checks and removes the error detection suffix and splits a message into fixed and variable length fields. handle() calls it, a caller that only wants the parsed message can get it without anything being printed. - MsgType::build_session: what login_core does once a terminal has authenticated. login_core calls it. - Koha::SIP2::RequestBuilder: the request builders of the emulator, now shared by the emulator and the message tester. - The message tester then reads a request exactly as the handler does (error detection suffix, checksum) and builds requests with the shared builders. There is no change in behaviour for the SIP server. Two small fixes to the emulator come with the move: the item information request carried a language field that the server's template for that message does not have (so the institution field was lost), and a field value of 0 was sent as an empty value. The new tests build each request and have the SIP server's own parser read it back. Test plan: 1. Apply the patches of bug 42769 and then the patches of this bug 2. Run prove t/db_dependent/SIP/ t/SIP/ t/Koha/SIP2/ t/db_dependent/Koha/SIP2/ t/db_dependent/api/v1/sip2_account_test_message.t 3. Confirm that all tests pass 4. Optionally run misc/sip_cli_emulator.pl against a SIP server (-a host -p port -su user -sp password -l institution --patron X --item Y -m patron_status_request -m patron_information -m item_information -m checkout -m checkin) and confirm login and every message still gets a response, and that the item information response names the institution 5. Sign off :-D Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42769 [Bug 42769] Add a SIP testing tool to the staff web interface -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43848 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208470 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208470&action=edit Bug 43848: Make request parsing and session setup reusable in the SIP server Code outside the SIP server that needs to parse a request or build a server context the way the server does ( the SIP2 message tester of bug 42769 is the first ) has to copy it, because both are buried in functions that also do something else, answering a request and authenticating a terminal. Pull them out: - parse_message: the part of handle() that sets the field delimiter, checks and removes the error detection suffix and splits the message into fixed and variable length fields. handle() calls it and answers a bad checksum as before, a caller that only wants the parsed message can get it without anything being printed. - build_session: what login_core does once a terminal has authenticated ( account, institution, policy and the ILS connection ). login_core calls it, nothing changes for a real login. MsgType called ->use on the ILS module without loading UNIVERSAL::require, which it only got away with because the SIP server loads it elsewhere. Load it, as a caller that uses MsgType on its own would otherwise fail. No change in behaviour for the SIP server. To test: 1) prove t/db_dependent/SIP/ t/SIP/ -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43848 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208471 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208471&action=edit Bug 43848: Share the SIP2 request builders with the SIP CLI emulator misc/sip_cli_emulator.pl knows how to build every request a terminal sends, but as subs inside a script, so code that needs to build a request ( the SIP2 message tester of bug 42769 ) writes its own builders next to them. Move them into Koha::SIP2::RequestBuilder and have both use it, so there is one place that knows how a request is laid out. The emulator builds what it did, apart from two fixes that sharing brought to light: - The item information request carried a language field. The server's template for that message is the 18 character date and nothing else, so the language pushed the date into the first field and the institution was lost. - build_field turned the value "0" into an empty value. build_field also refuses a value that contains the field delimiter, which would otherwise add fields to the request. The new tests build each request and have the SIP server's own parser read it, so a request that the server would read differently fails. To test: 1) prove t/Koha/SIP2/RequestBuilder.t 2) perl misc/sip_cli_emulator.pl against a SIP server still builds and sends the messages -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43848 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208472 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208472&action=edit Bug 43848: Use the SIP server's own parsing, builders and session setup in the SIP2 message tester Koha::SIP2::MessageTester ( bug 42769 ) had its own copies of things the SIP code already does. Use the real ones, now that they can be reused: - Parse the request with C4::SIP::Sip::MsgType::parse_message to find the patron and the item to check, instead of MsgType->new on the raw request. parse_message is what handle() uses, so the checks are made on exactly what the handler reads: the error detection suffix is taken off first and a wrong checksum is refused, where before a valid request that carried the suffix was refused for a patron that "was not found" and a bad checksum went unnoticed until the handler asked for a resend. - Build the server context with build_session, shared with login_core. - Build the requests with Koha::SIP2::RequestBuilder, shared with the SIP command line emulator, instead of a second set of builders. The requests differ in small ways the server doesn't care about: an empty patron password is sent as an empty field, as the emulator does, and a checkout and a checkin carry the cancel field. Nothing else changes for the user. All the tests of the message tester pass unchanged, and the new ones cover the request suffix, the checksum and the first of two identical fields. Depends on the two previous patches of this bug and on bug 42769. To test: 1) prove t/db_dependent/Koha/SIP2/MessageTester.t 2) prove t/db_dependent/api/v1/sip2_account_test_message.t -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43848 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43848 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43848 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org