From 16557827b252bd653b15eead479ec5699eda7360 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sun, 19 Nov 2023 21:34:56 +0100 Subject: [PATCH] Add a dialog to show which screens have potentially-problematic certificate validity periods when making KDMs (#2645). --- src/lib/kdm_cli.cc | 12 ++- src/lib/kdm_util.cc | 24 ++++- src/lib/kdm_util.h | 28 +++++- src/lib/screen.cc | 2 +- src/tools/dcpomatic_kdm.cc | 24 +++-- src/wx/invalid_certificate_period_dialog.cc | 103 ++++++++++++++++++++ src/wx/invalid_certificate_period_dialog.h | 45 +++++++++ src/wx/kdm_dialog.cc | 24 +++-- src/wx/wscript | 1 + test/kdm_util_test.cc | 20 +++- 10 files changed, 242 insertions(+), 41 deletions(-) create mode 100644 src/wx/invalid_certificate_period_dialog.cc create mode 100644 src/wx/invalid_certificate_period_dialog.h diff --git a/src/lib/kdm_cli.cc b/src/lib/kdm_cli.cc index 5ea808967..ddc77e771 100644 --- a/src/lib/kdm_cli.cc +++ b/src/lib/kdm_cli.cc @@ -257,13 +257,21 @@ from_film ( } - if (find(period_checks.begin(), period_checks.end(), KDMCertificatePeriod::KDM_OUTSIDE_CERTIFICATE) != period_checks.end()) { + if (find_if( + period_checks.begin(), + period_checks.end(), + [](KDMCertificatePeriod const& p) { return p.overlap == KDMCertificateOverlap::KDM_OUTSIDE_CERTIFICATE; } + ) != period_checks.end()) { throw KDMCLIError( "Some KDMs would have validity periods which are completely outside the recipient certificate periods. Such KDMs are very unlikely to work, so will not be created." ); } - if (find(period_checks.begin(), period_checks.end(), KDMCertificatePeriod::KDM_OVERLAPS_CERTIFICATE) != period_checks.end()) { + if (find_if( + period_checks.begin(), + period_checks.end(), + [](KDMCertificatePeriod const& p) { return p.overlap == KDMCertificateOverlap::KDM_OVERLAPS_CERTIFICATE; } + ) != period_checks.end()) { out("For some of these KDMs the recipient certificate's validity period will not cover the whole of the KDM validity period. This might cause problems with the KDMs."); } diff --git a/src/lib/kdm_util.cc b/src/lib/kdm_util.cc index bf112ce11..dcfd1fe67 100644 --- a/src/lib/kdm_util.cc +++ b/src/lib/kdm_util.cc @@ -35,7 +35,13 @@ using boost::optional; KDMCertificatePeriod -check_kdm_and_certificate_validity_periods(dcp::Certificate const& recipient, dcp::LocalTime kdm_from, dcp::LocalTime kdm_to) +check_kdm_and_certificate_validity_periods( + string const& cinema_name, + string const& screen_name, + dcp::Certificate const& recipient, + dcp::LocalTime kdm_from, + dcp::LocalTime kdm_to + ) { auto overlaps = [](dcp::LocalTime from_a, dcp::LocalTime to_a, dcp::LocalTime from_b, dcp::LocalTime to_b) { return std::max(from_a, from_b) < std::min(to_a, to_b); @@ -45,16 +51,26 @@ check_kdm_and_certificate_validity_periods(dcp::Certificate const& recipient, dc return bigger_from <= smaller_from && bigger_to >= smaller_to; }; + KDMCertificatePeriod period( + cinema_name, + screen_name, + recipient.not_before(), + recipient.not_after() + ); + if (contains(recipient.not_before(), recipient.not_after(), kdm_from, kdm_to)) { - return KDMCertificatePeriod::KDM_WITHIN_CERTIFICATE; + period.overlap = KDMCertificateOverlap::KDM_WITHIN_CERTIFICATE; + return period; } if (overlaps(recipient.not_before(), recipient.not_after(), kdm_from, kdm_to)) { /* The KDM overlaps the certificate validity: maybe not the end of the world */ - return KDMCertificatePeriod::KDM_OVERLAPS_CERTIFICATE; + period.overlap = KDMCertificateOverlap::KDM_OVERLAPS_CERTIFICATE; + return period; } else { /* The KDM validity is totally outside the certificate validity: bad news */ - return KDMCertificatePeriod::KDM_OUTSIDE_CERTIFICATE; + period.overlap = KDMCertificateOverlap::KDM_OUTSIDE_CERTIFICATE; + return period; } } diff --git a/src/lib/kdm_util.h b/src/lib/kdm_util.h index 398819ae0..8dba1ff98 100644 --- a/src/lib/kdm_util.h +++ b/src/lib/kdm_util.h @@ -32,21 +32,43 @@ namespace dcp { } -enum class KDMCertificatePeriod { +enum class KDMCertificateOverlap { KDM_WITHIN_CERTIFICATE, KDM_OVERLAPS_CERTIFICATE, KDM_OUTSIDE_CERTIFICATE }; +struct KDMCertificatePeriod +{ + KDMCertificatePeriod(std::string cinema_name_, std::string screen_name_, dcp::LocalTime from_, dcp::LocalTime to_) + : cinema_name(std::move(cinema_name_)) + , screen_name(std::move(screen_name_)) + , from(std::move(from_)) + , to(std::move(to_)) + {} + + std::string cinema_name; + std::string screen_name; + KDMCertificateOverlap overlap = KDMCertificateOverlap::KDM_WITHIN_CERTIFICATE; + dcp::LocalTime from; + dcp::LocalTime to; +}; + + /** @param recipient Some KDM recipient certificate. * @param kdm_from Proposed KDM start time. * @param kdm_to Proposed KDM end time. * @return Relationship between certificate and KDM validity periods. */ -KDMCertificatePeriod -check_kdm_and_certificate_validity_periods(dcp::Certificate const& recipient, dcp::LocalTime kdm_from, dcp::LocalTime kdm_to); +KDMCertificatePeriod check_kdm_and_certificate_validity_periods( + std::string const& cinema_name, + std::string const& screen_name, + dcp::Certificate const& recipient, + dcp::LocalTime kdm_from, + dcp::LocalTime kdm_to + ); #endif diff --git a/src/lib/screen.cc b/src/lib/screen.cc index 097ff80b8..7e2918018 100644 --- a/src/lib/screen.cc +++ b/src/lib/screen.cc @@ -93,7 +93,7 @@ kdm_for_screen ( dcp::LocalTime const begin(valid_from, dcp::UTCOffset(cinema ? cinema->utc_offset_hour() : 0, cinema ? cinema->utc_offset_minute() : 0)); dcp::LocalTime const end (valid_to, dcp::UTCOffset(cinema ? cinema->utc_offset_hour() : 0, cinema ? cinema->utc_offset_minute() : 0)); - period_checks.push_back(check_kdm_and_certificate_validity_periods(screen->recipient.get(), begin, end)); + period_checks.push_back(check_kdm_and_certificate_validity_periods(cinema->name, screen->name, screen->recipient.get(), begin, end)); auto signer = Config::instance()->signer_chain(); if (!signer->valid()) { diff --git a/src/tools/dcpomatic_kdm.cc b/src/tools/dcpomatic_kdm.cc index 8cf381283..9bb8e20a8 100644 --- a/src/tools/dcpomatic_kdm.cc +++ b/src/tools/dcpomatic_kdm.cc @@ -22,6 +22,7 @@ #include "wx/about_dialog.h" #include "wx/dcpomatic_button.h" #include "wx/editable_list.h" +#include "wx/invalid_certificate_period_dialog.h" #include "wx/file_dialog.h" #include "wx/file_picker_ctrl.h" #include "wx/full_config_dialog.h" @@ -419,19 +420,16 @@ private: return; } - if (find(period_checks.begin(), period_checks.end(), KDMCertificatePeriod::KDM_OUTSIDE_CERTIFICATE) != period_checks.end()) { - error_dialog( - this, - _("Some KDMs would have validity periods which are completely outside the recipient certificate periods. Such KDMs are very unlikely to work, so will not be created.") - ); - return; - } - - if (find(period_checks.begin(), period_checks.end(), KDMCertificatePeriod::KDM_OVERLAPS_CERTIFICATE) != period_checks.end()) { - message_dialog( - this, - _("For some of these KDMs the recipient certificate's validity period will not cover the whole of the KDM validity period. This might cause problems with the KDMs.") - ); + if ( + find_if( + period_checks.begin(), + period_checks.end(), + [](KDMCertificatePeriod const& p) { return p.overlap != KDMCertificateOverlap::KDM_WITHIN_CERTIFICATE; } + ) != period_checks.end()) { + InvalidCertificatePeriodDialog dialog(this, period_checks); + if (dialog.ShowModal() == wxID_CANCEL) { + return; + } } auto result = _output->make ( diff --git a/src/wx/invalid_certificate_period_dialog.cc b/src/wx/invalid_certificate_period_dialog.cc new file mode 100644 index 000000000..54f699ba7 --- /dev/null +++ b/src/wx/invalid_certificate_period_dialog.cc @@ -0,0 +1,103 @@ +/* + Copyright (C) 2023 Carl Hetherington + + This file is part of DCP-o-matic. + + DCP-o-matic is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + DCP-o-matic is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with DCP-o-matic. If not, see . + +*/ + + +#include "invalid_certificate_period_dialog.h" +#include "wx_util.h" +#include "lib/kdm_util.h" +#include +#include + + +InvalidCertificatePeriodDialog::InvalidCertificatePeriodDialog(wxWindow* parent, std::vector const& periods) + : wxDialog(parent, wxID_ANY, _("Invalid certificates")) + , _list(new wxListCtrl(this, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxLC_REPORT)) +{ + { + wxListItem ip; + ip.SetId(0); + ip.SetText(_("Cinema")); + ip.SetWidth(200); + _list->InsertColumn(0, ip); + } + + { + wxListItem ip; + ip.SetId(1); + ip.SetText(_("Screen")); + ip.SetWidth(50); + _list->InsertColumn(1, ip); + } + + { + wxListItem ip; + ip.SetId(2); + ip.SetText(_("Certificate start")); + ip.SetWidth(200); + _list->InsertColumn(2, ip); + } + + { + wxListItem ip; + ip.SetId(3); + ip.SetText(_("Certificate end")); + ip.SetWidth(200); + _list->InsertColumn(3, ip); + } + + int id = 0; + for (auto const& period: periods) { + wxListItem item; + item.SetId(id); + _list->InsertItem(item); + _list->SetItem(0, 0, std_to_wx(period.cinema_name)); + _list->SetItem(0, 1, std_to_wx(period.screen_name)); + _list->SetItem(0, 2, std_to_wx(period.from.as_string())); + _list->SetItem(0, 3, std_to_wx(period.to.as_string())); + } + + auto overall_sizer = new wxBoxSizer(wxVERTICAL); + + auto constexpr width = 700; + + auto question = new wxStaticText(this, wxID_ANY, _("Some KDMs would have validity periods which are outside the recipient certificate validity periods. What do you want to do?")); + question->Wrap(width); + overall_sizer->Add( + question, + 0, + wxALL, + DCPOMATIC_DIALOG_BORDER + ); + + _list->SetSize({width, -1}); + overall_sizer->Add(_list, 1, wxALL | wxEXPAND, DCPOMATIC_DIALOG_BORDER); + + auto buttons = CreateStdDialogButtonSizer(0); + if (buttons) { + overall_sizer->Add(CreateSeparatedSizer(buttons), wxSizerFlags().Expand().DoubleBorder()); + buttons->SetAffirmativeButton(new wxButton(this, wxID_OK, _("Create KDMs anyway"))); + buttons->SetCancelButton(new wxButton(this, wxID_CANCEL, _("Cancel"))); + buttons->Realize(); + } + + overall_sizer->Layout(); + SetSizerAndFit(overall_sizer); +} + diff --git a/src/wx/invalid_certificate_period_dialog.h b/src/wx/invalid_certificate_period_dialog.h new file mode 100644 index 000000000..08933b70d --- /dev/null +++ b/src/wx/invalid_certificate_period_dialog.h @@ -0,0 +1,45 @@ +/* + Copyright (C) 2023 Carl Hetherington + + This file is part of DCP-o-matic. + + DCP-o-matic is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + DCP-o-matic is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with DCP-o-matic. If not, see . + +*/ + + +#ifndef DCPOMATIC_INVALID_CERTIFICATE_PERIOD_DIALOG_H +#define DCPOMATIC_INVALID_CERTIFICATE_PERIOD_DIALOG_H + + +#include + + +class KDMCertificatePeriod; +class wxListCtrl; + + +class InvalidCertificatePeriodDialog : public wxDialog +{ +public: + InvalidCertificatePeriodDialog(wxWindow* parent, std::vector const& periods); + +private: + wxListCtrl* _list; +}; + + +#endif + + diff --git a/src/wx/kdm_dialog.cc b/src/wx/kdm_dialog.cc index bec880852..9135baf23 100644 --- a/src/wx/kdm_dialog.cc +++ b/src/wx/kdm_dialog.cc @@ -21,6 +21,7 @@ #include "confirm_kdm_email_dialog.h" #include "dcpomatic_button.h" +#include "invalid_certificate_period_dialog.h" #include "kdm_cpl_panel.h" #include "kdm_dialog.h" #include "kdm_output_panel.h" @@ -185,19 +186,16 @@ KDMDialog::make_clicked () } } - if (find(period_checks.begin(), period_checks.end(), KDMCertificatePeriod::KDM_OUTSIDE_CERTIFICATE) != period_checks.end()) { - error_dialog( - this, - _("Some KDMs would have validity periods which are completely outside the recipient certificate periods. Such KDMs are very unlikely to work, so will not be created.") - ); - return; - } - - if (find(period_checks.begin(), period_checks.end(), KDMCertificatePeriod::KDM_OVERLAPS_CERTIFICATE) != period_checks.end()) { - message_dialog( - this, - _("For some of these KDMs the recipient certificate's validity period will not cover the whole of the KDM validity period. This might cause problems with the KDMs.") - ); + if ( + find_if( + period_checks.begin(), + period_checks.end(), + [](KDMCertificatePeriod const& p) { return p.overlap != KDMCertificateOverlap::KDM_WITHIN_CERTIFICATE; } + ) != period_checks.end()) { + InvalidCertificatePeriodDialog dialog(this, period_checks); + if (dialog.ShowModal() == wxID_CANCEL) { + return; + } } } catch (dcp::BadKDMDateError& e) { diff --git a/src/wx/wscript b/src/wx/wscript index 3e5483647..9c6ea6b84 100644 --- a/src/wx/wscript +++ b/src/wx/wscript @@ -88,6 +88,7 @@ sources = """ html_dialog.cc i18n_hook.cc image_sequence_dialog.cc + invalid_certificate_period_dialog.cc instant_i18n_dialog.cc interop_metadata_dialog.cc job_manager_view.cc diff --git a/test/kdm_util_test.cc b/test/kdm_util_test.cc index 27b98230d..8426f247a 100644 --- a/test/kdm_util_test.cc +++ b/test/kdm_util_test.cc @@ -28,58 +28,68 @@ BOOST_AUTO_TEST_CASE(check_kdm_and_certificate_validity_periods_good) { auto const result = check_kdm_and_certificate_validity_periods( + "Bob's Place", + "Country", dcp::Certificate(dcp::file_to_string("test/data/cert.pem")), dcp::LocalTime("2023-01-03T10:30:00"), dcp::LocalTime("2050-10-20T14:00:00") ); - BOOST_CHECK(result == KDMCertificatePeriod::KDM_WITHIN_CERTIFICATE); + BOOST_CHECK(result.overlap == KDMCertificateOverlap::KDM_WITHIN_CERTIFICATE); } BOOST_AUTO_TEST_CASE(check_kdm_and_certificate_validity_periods_overlap_start) { auto const result = check_kdm_and_certificate_validity_periods( + "Bob's Place", + "Western", dcp::Certificate(dcp::file_to_string("test/data/cert.pem")), dcp::LocalTime("2011-01-03T10:30:00"), dcp::LocalTime("2050-10-20T14:00:00") ); - BOOST_CHECK(result == KDMCertificatePeriod::KDM_OVERLAPS_CERTIFICATE); + BOOST_CHECK(result.overlap == KDMCertificateOverlap::KDM_OVERLAPS_CERTIFICATE); } BOOST_AUTO_TEST_CASE(check_kdm_and_certificate_validity_periods_overlap_end) { auto const result = check_kdm_and_certificate_validity_periods( + "Palace Hotel Ballroom", + "Lobby", dcp::Certificate(dcp::file_to_string("test/data/cert.pem")), dcp::LocalTime("2033-01-03T10:30:00"), dcp::LocalTime("2095-10-20T14:00:00") ); - BOOST_CHECK(result == KDMCertificatePeriod::KDM_OVERLAPS_CERTIFICATE); + BOOST_CHECK(result.overlap == KDMCertificateOverlap::KDM_OVERLAPS_CERTIFICATE); } BOOST_AUTO_TEST_CASE(check_kdm_and_certificate_validity_periods_overlap_start_and_end) { auto const result = check_kdm_and_certificate_validity_periods( + "Palace Hotel Ballroom", + "Stage", dcp::Certificate(dcp::file_to_string("test/data/cert.pem")), dcp::LocalTime("2011-01-03T10:30:00"), dcp::LocalTime("2095-10-20T14:00:00") ); - BOOST_CHECK(result == KDMCertificatePeriod::KDM_OVERLAPS_CERTIFICATE); + BOOST_CHECK(result.overlap == KDMCertificateOverlap::KDM_OVERLAPS_CERTIFICATE); } BOOST_AUTO_TEST_CASE(check_kdm_and_certificate_validity_periods_outside) { auto const result = check_kdm_and_certificate_validity_periods( + "Palace Hotel Ballroom", + "Drum Riser", dcp::Certificate(dcp::file_to_string("test/data/cert.pem")), dcp::LocalTime("2011-01-03T10:30:00"), dcp::LocalTime("2012-10-20T14:00:00") ); - BOOST_CHECK(result == KDMCertificatePeriod::KDM_OUTSIDE_CERTIFICATE); + BOOST_CHECK(result.overlap == KDMCertificateOverlap::KDM_OUTSIDE_CERTIFICATE); } -- 2.30.2