Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
4d41555
[service] Add a new Question via D-Bus
mvidner Jul 22, 2022
2831430
[service] a simple testing question in the same service
mvidner Jul 27, 2022
78572e6
[service] Help with consistent indentation in VS Code
mvidner Jul 27, 2022
2a2611a
[service] Change CanAskQuestion protocol to #delete the result of #add
mvidner Jul 27, 2022
314c70f
[service] WIP: try asking a remote question
mvidner Jul 27, 2022
36e8d19
[service] expose Software#testing_question via D-Bus
mvidner Jul 28, 2022
3811e97
[service] continue implementing QuestionsManager+CanAskQuestion
mvidner Jul 28, 2022
b30be9c
[service] Clean up CanAskQuestion+QuestionsManager
mvidner Jul 29, 2022
3cdafec
[service] Make DInstaller::DBus::Question#backend public
mvidner Jul 29, 2022
4065680
[service] Use DInstaller::DBus::Clients::Question
mvidner Jul 29, 2022
69ed210
[service] Fix Manager tests
mvidner Jul 29, 2022
527d8f1
[service] Adapt test to changed #add #delete API truthy/falsy
mvidner Jul 29, 2022
8e08749
[service] Ask testing questions only if env DINSTALLER_TEST_QUESTIONS=1
mvidner Aug 8, 2022
db9ad52
[service] Fix DInsaller::DBus::Client::Question API to use symbols
mvidner Aug 8, 2022
beacdef
[service] QuestionsManager will #wait for a specific list of Questions
mvidner Aug 8, 2022
1e699e0
[service] Test DInstaller::DBus::Clients::QuestionsManager,Question
mvidner Aug 10, 2022
f3d1a2a
[service] argument checking for DInstaller::DBus::Questions#Delete
mvidner Aug 10, 2022
7120bf4
[service] Test CanAskQuestion
mvidner Aug 11, 2022
a4f4c73
[cli] Test coverage
mvidner Aug 11, 2022
7819fc2
[service] Test CanAskQuestion without relying on testing_question
mvidner Aug 12, 2022
75d9464
[service] Remove all the testing questions
mvidner Aug 12, 2022
70a279b
[service] Simplify QuestionsManager#wait
mvidner Aug 12, 2022
ea77d16
[service] Resolve fixmes for DInstaller::DBus::Clients::QuestionsManager
mvidner Aug 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cli/lib/dinstaller_cli/clients.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,4 @@ module Clients
end
end

require "dinstaller_cli/clients/language"
require "dinstaller_cli/clients/storage"
25 changes: 25 additions & 0 deletions cli/test/dinstaller_cli/clients_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

# Copyright (c) [2022] SUSE LLC
#
# All Rights Reserved.
#
# This program is free software; you can redistribute it and/or modify it
# under the terms of version 2 of the GNU General Public License as published
# by the Free Software Foundation.
#
# This program 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 this program; if not, contact SUSE LLC.
#
# To contact SUSE LLC about this file by physical or electronic mail, you may
# find current contact information at www.suse.com.

require_relative "../test_helper"
require "dinstaller_cli/clients"

# nothing else, this test just covers the require-only files
25 changes: 25 additions & 0 deletions cli/test/dinstaller_cli_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

# Copyright (c) [2022] SUSE LLC
#
# All Rights Reserved.
#
# This program is free software; you can redistribute it and/or modify it
# under the terms of version 2 of the GNU General Public License as published
# by the Free Software Foundation.
#
# This program 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 this program; if not, contact SUSE LLC.
#
# To contact SUSE LLC about this file by physical or electronic mail, you may
# find current contact information at www.suse.com.

require_relative "test_helper"
require "dinstaller_cli"

# nothing else, this test just covers the require-only files
9 changes: 9 additions & 0 deletions service/.editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# EditorConfig is awesome: https://EditorConfig.org

# top-most EditorConfig file
root = true

# 2 space indentation
[*.rb]
indent_style = space
indent_size = 2
14 changes: 8 additions & 6 deletions service/lib/dinstaller/can_ask_question.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module DInstaller
module CanAskQuestion
# @!method questions_manager
# @note Classes including this mixin must define a #questions_manager method
# @return [QuestionsManager]
# @return [QuestionsManager,DBus::Clients::QuestionsManager]

# Asks the given question and waits until the question is answered
#
Expand All @@ -33,14 +33,16 @@ module CanAskQuestion
# ask(question2) { |q| q.answer == :yes } #=> Boolean
#
# @param question [Question]
# @yield [Question] Gives the answered question to the block.
# @yield [Question,DBus::Clients::Question] Gives the answered question to the block.
# @return [Symbol, Object] The question answer, or the result of the block in case a block is
# given.
def ask(question)
questions_manager.add(question)
questions_manager.wait
result = block_given? ? yield(question) : question.answer
questions_manager.delete(question)
# asked_question has the same interface as question
# but it may be a D-Bus proxy, if our questions_manager is also one
asked_question = questions_manager.add(question)
questions_manager.wait([asked_question])
result = block_given? ? yield(asked_question) : asked_question.answer
questions_manager.delete(asked_question)

result
end
Expand Down
63 changes: 63 additions & 0 deletions service/lib/dinstaller/dbus/clients/question.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# frozen_string_literal: true

# Copyright (c) [2022] SUSE LLC
#
# All Rights Reserved.
#
# This program is free software; you can redistribute it and/or modify it
# under the terms of version 2 of the GNU General Public License as published
# by the Free Software Foundation.
#
# This program 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 this program; if not, contact SUSE LLC.
#
# To contact SUSE LLC about this file by physical or electronic mail, you may
# find current contact information at www.suse.com.

require "dinstaller/dbus/clients/base"

module DInstaller
module DBus
module Clients
# D-Bus client for asking a question.
# Its interface is a subset of {DInstaller::Question}
# so it can be used in the block of {DInstaller::CanAskQuestion#ask}.
class Question < Base
# @return [::DBus::ProxyObject]
attr_reader :dbus_object

# @param [::DBus::ObjectPath] object_path
def initialize(object_path)
super()

@dbus_object = service[object_path]
@dbus_iface = @dbus_object["org.opensuse.DInstaller.Question1"]
end

# @return [String]
def service_name
@service_name ||= "org.opensuse.DInstaller"
end

# TODO: what other methods are useful?

# @return [Symbol] no answer yet = :""
def answer
@dbus_iface["Answer"].to_sym
end

# Whether the question is already answered
#
# @return [Boolean]
def answered?
answer != :""
end
end
end
end
end
91 changes: 91 additions & 0 deletions service/lib/dinstaller/dbus/clients/questions_manager.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# frozen_string_literal: true

# Copyright (c) [2022] SUSE LLC
#
# All Rights Reserved.
#
# This program is free software; you can redistribute it and/or modify it
# under the terms of version 2 of the GNU General Public License as published
# by the Free Software Foundation.
#
# This program 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 this program; if not, contact SUSE LLC.
#
# To contact SUSE LLC about this file by physical or electronic mail, you may
# find current contact information at www.suse.com.

require "dinstaller/dbus/clients/base"
require "dinstaller/dbus/clients/question"

module DInstaller
module DBus
module Clients
# D-Bus client for asking a question.
# It has the same interface as {DInstaller::QuestionsManager}
# so it can be used for {DInstaller::CanAskQuestion}.
class QuestionsManager < Base
def initialize
super

@dbus_object = service["/org/opensuse/DInstaller/Questions1"]
@dbus_object.default_iface = "org.opensuse.DInstaller.Questions1"
end

# @return [String]
def service_name
@service_name ||= "org.opensuse.DInstaller"
end

# Adds a question
#
# @param question [DInstaller::Question]
# @return [DBus::Clients::Question]
def add(question)
q_path = @dbus_object.New(
question.text,
question.options.map(&:to_s),
Array(question.default_option&.to_s)
)
DBus::Clients::Question.new(q_path)
end

# Deletes the given question
#
# @param question [DBus::Clients::Question]
# @return [void]
# @raise [::DBus::Error] if trying to delete a question twice
def delete(question)
@dbus_object.Delete(question.dbus_object.path)
end

# Waits until specified questions are answered.
# @param questions [Array<DBus::Clients::Question>]
# @return [void]
def wait(questions)
# TODO: detect if no UI showed up to display the questions and time out?
# for example:
# (0..Float::INFINITY).each { |i| break if i > 100 && !question.displayed; ... }

# We should register the InterfacesAdded callback... BEFORE adding to avoid races.
# Stupid but simple way: poll the answer property, sleep, repeat
loop do
questions = questions.find_all { |q| !q.answered? }
break if questions.empty?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not break if questions.all?(&:answered?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because our D-Bus clients do not cache properties and this way we don't make calls about questions that we already know are answered.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so please document it. or maybe use better variable name.


sleep(0.5)
end
end

private

# @return [::DBus::Object]
attr_reader :dbus_object
end
end
end
end
4 changes: 2 additions & 2 deletions service/lib/dinstaller/dbus/question.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,11 @@ def initialize(path, backend, logger)
add_interfaces
end

private

# @return [DInstaller::Question]
attr_reader :backend

private

# Adds interfaces to the question
def add_interfaces
INTERFACES_TO_INCLUDE[backend.class].each { |i| singleton_class.include(i) }
Expand Down
28 changes: 28 additions & 0 deletions service/lib/dinstaller/dbus/questions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ class Questions < ::DBus::Object
PATH = "/org/opensuse/DInstaller/Questions1"
private_constant :PATH

QUESTIONS_INTERFACE = "org.opensuse.DInstaller.Questions1"
private_constant :QUESTIONS_INTERFACE

OBJECT_MANAGER_INTERFACE = "org.freedesktop.DBus.ObjectManager"
private_constant :OBJECT_MANAGER_INTERFACE

Expand Down Expand Up @@ -73,6 +76,31 @@ def managed_objects
dbus_signal(:InterfacesRemoved, "object:o, interfaces:as")
end

dbus_interface QUESTIONS_INTERFACE do
# default_option is an array of 0 or 1 elements
dbus_method :New, "in text:s, in options:as, in default_option:as, out q:o" do
|text, options, default_option|

backend_q = DInstaller::Question.new(
text,
options: options.map(&:to_sym),
default_option: default_option.map(&:to_sym).first
)
backend.add(backend_q)
path_for(backend_q)
end

dbus_method :Delete, "in question:o" do |question_path|
dbus_q = @service.get_node(question_path)&.object
raise ArgumentError, "Object path #{question_path} not found" unless dbus_q
raise ArgumentError, "Object #{question_path} is not a Question" unless
dbus_q.is_a? DInstaller::DBus::Question
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please do not use trailing if/unless when it is not single line


backend_q = dbus_q.backend
backend.delete(backend_q)
end
end

private

# @return [DInstaller::QuestionsManager]
Expand Down
2 changes: 2 additions & 0 deletions service/lib/dinstaller/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
require "yast"
require "bootloader/proposal_client"
require "bootloader/finish_client"
require "dinstaller/can_ask_question"
require "dinstaller/config"
require "dinstaller/network"
require "dinstaller/security"
Expand All @@ -45,6 +46,7 @@ module DInstaller
# other services via D-Bus (e.g., `org.opensuse.DInstaller.Software`).
class Manager
include WithProgress
include CanAskQuestion

# @return [Logger]
attr_reader :logger
Expand Down
29 changes: 13 additions & 16 deletions service/lib/dinstaller/questions_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ def initialize(logger)
# @yieldparam question [Question] added question
#
# @param question [Question]
# @return [Boolean] whether the question was added
# @return [Question,nil] the actually added question (to be passed to {#delete} later)
def add(question)
return false if include?(question)
return nil if include?(question)

questions << question
on_add_callbacks.each { |c| c.call(question) }

true
question
end

# Deletes the given question
Expand All @@ -63,26 +63,30 @@ def add(question)
# @yieldparam question [Question] deleted question
#
# @param question [Question]
# @return [Boolean] whether the question was deleted
# @return [Question,nil] whether the question was deleted
def delete(question)
return false unless include?(question)
return nil unless include?(question)

questions.delete(question)
on_delete_callbacks.each { |c| c.call(question) }

true
question
end

# Waits until all questions are answered
# Waits until all specified questions are answered.
# There may be other questions, asked from other services, which
# are waited for by remote question managers, so we ignore those.
#
# Callbacks are periodically called while waiting, see {#on_wait}.
def wait
# @param questions [Array<Question>]
def wait(questions)
logger.info "Waiting for questions to be answered"

loop do
on_wait_callbacks.each(&:call)
break if questions.all?(&:answered?)

sleep(0.1)
break if questions_answered?
end
end

Expand Down Expand Up @@ -134,12 +138,5 @@ def on_wait(&block)
def include?(question)
questions.any? { |q| q.id == question.id }
end

# Whether all questions are already answered
#
# @return [Boolean]
def questions_answered?
questions.all?(&:answered?)
end
end
end
Loading