nettika/geary
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Remove ListParameter.parent property so lists can have multiple parents.

Browse source 
The ListParameter.parent property only existed to make the deserialier's
life easier, but is also why sketchy code was needed for appending search
criteria from a ServerSearchEmail replay op to the actual IMAP search
command sent to the server.

This removes the property altogether, and replaces its only use in
Deserialier with a stack, as nature intended. This means lists can be
added to more than one other list, and e.g. when a search is executed,
the search critera can be used for multiple requests.
This commit is contained in:
Michael James Gratton 2018-07-01 14:47:23 +10:00
parent e11bdc863b
commit 51b44f0f4b
6 changed files with 196 additions and 119 deletions
Download patch file Download diff file Expand all files Collapse all files

116
src/engine/imap/parameter/imap-list-parameter.vala
View file

@ -25,76 +25,37 @@ public class Geary.Imap.ListParameter : Geary.Imap.Parameter {
return list.size;
}
}
/**
* Returns null if no parent (top-level list).
*
* In a fully-formed set of {@link Parameter}s, this means this {@link ListParameter} is
* probably a {@link RootParameters}.
*/
public weak ListParameter? parent { get; private set; default = null; }
private Gee.List<Parameter> list = new Gee.ArrayList<Parameter>();
/**
* Creates an empty ListParameter with no parent.
*/
public ListParameter() {
}
~ListParameter() {
// Drop back links because, although it's a weak ref, sometimes ListParameters are temporarily
// made and current Vala doesn't reset weak refs
foreach (Parameter param in list) {
ListParameter? listp = param as ListParameter;
if (listp != null) {
assert(listp.parent == this);
listp.parent = null;
}
}
}
/**
* Adds the {@link Parameter} to the end of the {@link ListParameter}.
* Appends a parameter to the end of this list.
*
* If the Parameter is itself a ListParameter, it's {@link parent} will be set to this
* ListParameter.
*
* The same {@link Parameter} can't be added more than once to the same {@link ListParameter}.
* There are no other restrictions, however.
* The same {@link Parameter} can't be added more than once to the
* same {@link ListParameter}. There are no other restrictions,
* however.
*
* @return true if added.
*/
public bool add(Parameter param) {
// if adding a ListParameter, set its parent
ListParameter? listp = param as ListParameter;
if (listp != null) {
if (listp.parent != null)
listp.parent.list.remove(listp);
listp.parent = this;
}
return list.add(param);
return this.list.add(param);
}
/**
* Adds all the {@link Parameter}s to the end of the {@link ListParameter}.
* Adds all parameters in the given collection to this list.
*
* If any Parameter is itself a ListParameter, it's {@link parent} will be set to this
* ListParameter.
*
* The same {@link Parameter} can't be added more than once to the same {@link ListParameter}.
* There are no other restrictions, however.
* The same {@link Parameter} can't be added more than once to the
* same {@link ListParameter}. There are no other restrictions,
* however.
*
* @return number of Parameters added.
*/
public int add_all(Gee.Collection<Parameter> params) {
int count = 0;
foreach (Parameter param in params)
foreach (Parameter param in params) {
count += add(param) ? 1 : 0;
}
return count;
}
@ -118,23 +79,14 @@ public class Geary.Imap.ListParameter : Geary.Imap.Parameter {
public int extend(ListParameter listp) {
return add_all(listp.list);
}
/**
* Clears the {@link ListParameter} of all its children.
*
* This also clears (sets to null) the parents of all ListParameter's children.
*/
public void clear() {
// sever ties to ListParameter children
foreach (Parameter param in list) {
ListParameter? listp = param as ListParameter;
if (listp != null)
listp.parent = null;
}
list.clear();
}
//
// Parameter retrieval
//
@ -370,19 +322,16 @@ public class Geary.Imap.ListParameter : Geary.Imap.Parameter {
public ListParameter? get_as_nullable_list(int index) throws ImapError {
return (ListParameter?) get_as_nullable(index, typeof(ListParameter));
}
/**
* Returns [@link ListParameter} at index, an empty list if NIL.
*
* If an empty ListParameter has to be manufactured in place of a NIL parameter, its parent
* will be null.
* Returns {@link ListParameter} at index, an empty list if NIL.
*/
public ListParameter get_as_empty_list(int index) throws ImapError {
ListParameter? param = get_as_nullable_list(index);
return param ?? new ListParameter();
}
/**
* Returns a {@link ListParameter} at index, null if not a list.
*
@ -474,27 +423,12 @@ public class Geary.Imap.ListParameter : Geary.Imap.Parameter {
public Parameter replace(int index, Parameter parameter) throws ImapError {
if (list.size <= index)
throw new ImapError.TYPE_ERROR("No parameter at index %d", index);
Parameter old = list[index];
list[index] = parameter;
// add parent to new Parameter if a list
ListParameter? listp = parameter as ListParameter;
if (listp != null) {
if (listp.parent != null)
listp.parent.list.remove(listp);
listp.parent = this;
}
// clear parent of old Parameter if a list
listp = old as ListParameter;
if (listp != null)
listp.parent = null;
Parameter old = this.list[index];
this.list[index] = parameter;
return old;
}
/**
* Moves all child parameters from the supplied list into this list, clearing this list first.
*

57
src/engine/imap/transport/imap-deserializer.vala
View file

@ -74,11 +74,14 @@ public class Geary.Imap.Deserializer : BaseObject {
private string identifier;
private DataInputStream dins;
private Geary.State.Machine fsm;
private ListParameter context;
private Gee.LinkedList<ListParameter> context_stack =
new Gee.LinkedList<ListParameter>();
private Cancellable? cancellable = null;
private Nonblocking.Semaphore closed_semaphore = new Nonblocking.Semaphore();
private Geary.Stream.MidstreamConverter midstream = new Geary.Stream.MidstreamConverter("Deserializer");
private RootParameters root = new RootParameters();
private StringBuilder? current_string = null;
private size_t literal_length_remaining = 0;
private Geary.Memory.GrowableBuffer? block_buffer = null;
@ -144,9 +147,7 @@ public class Geary.Imap.Deserializer : BaseObject {
dins = new DataInputStream(cins);
dins.set_newline_type(DataStreamNewlineType.CR_LF);
dins.set_close_base_stream(false);
context = root;
Geary.State.Mapping[] mappings = {
new Geary.State.Mapping(State.TAG, Event.CHAR, on_tag_char),
new Geary.State.Mapping(State.TAG, Event.EOS, on_eos),
@ -203,10 +204,12 @@ public class Geary.Imap.Deserializer : BaseObject {
new Geary.State.Mapping(State.CLOSED, Event.EOS, Geary.State.nop),
new Geary.State.Mapping(State.CLOSED, Event.ERROR, Geary.State.nop)
};
fsm = new Geary.State.Machine(machine_desc, mappings, on_bad_transition);
this.fsm = new Geary.State.Machine(machine_desc, mappings, on_bad_transition);
reset_params();
}
/**
* Install a custom Converter into the input stream.
*
@ -456,34 +459,34 @@ public class Geary.Imap.Deserializer : BaseObject {
private void save_parameter(Parameter param) {
context.add(param);
}
// ListParameter's parent *must* be current context
private void push(ListParameter child) {
context.add(child);
context = child;
this.context.add(child);
this.context_stack.insert(0, child);
this.context = child;
}
private char get_current_context_terminator() {
return (context is ResponseCode) ? ']' : ')';
}
private State pop() {
ListParameter? parent = context.parent;
if (parent == null) {
State ret = State.START_PARAM;
if (this.context_stack.size > 1) {
this.context_stack.remove_at(0);
this.context = this.context_stack[0];
} else {
warning("Attempt to close unopened list/response code");
return State.FAILED;
ret = State.FAILED;
}
context = parent;
return State.START_PARAM;
return ret;
}
private void flush_params() {
bool okay = true;
if (this.context != this.root) {
if (this.context_stack.size > 1) {
Logging.debug(
Logging.Flag.DESERIALIZER,
"[%s] Unclosed list in parameters",
@ -501,17 +504,17 @@ public class Geary.Imap.Deserializer : BaseObject {
okay = false;
}
if (okay && this.root != null && root.size > 0) {
parameters_ready(this.root);
if (okay && this.context.size > 0) {
parameters_ready((RootParameters) this.context);
}
reset_params();
}
private void reset_params() {
this.context = this.root = new RootParameters();
this.current_string = null;
this.literal_length_remaining = 0;
this.context = new RootParameters();
this.context_stack.clear();
this.context_stack.add(this.context);
}
//

30
test/engine/imap/parameter/imap-list-parameter-test.vala Normal file
View file

@ -0,0 +1,30 @@
/*
* Copyright 2018 Michael Gratton <mike@vee.net>
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
*/
class Geary.Imap.ListParameterTest : TestCase {
public ListParameterTest() {
base("Geary.Imap.ListParameterTest");
add_test("add_to_multiple_parents", add_to_multiple_parents);
}
// See GitLab Issue #26
public void add_to_multiple_parents() throws GLib.Error {
ListParameter child = new ListParameter();
ListParameter parent_1 = new ListParameter();
ListParameter parent_2 = new ListParameter();
parent_1.add(child);
parent_2.add(child);
assert_int(1, parent_1.size, "Parent 1 does not contain child");
assert_int(1, parent_2.size, "Parent 2 does not contain child");
}
}

110
test/engine/imap/transport/imap-deserializer-test.vala
View file

@ -1,5 +1,5 @@
/*
* Copyright 2017 Michael Gratton <mike@vee.net>
* Copyright 2017-2018 Michael Gratton <mike@vee.net>
*
* This software is licensed under the GNU Lesser General Public License
* (version 2.1 or later). See the COPYING file in this distribution.
@ -11,6 +11,7 @@ class Geary.Imap.DeserializerTest : TestCase {
protected enum Expect { MESSAGE, EOS, DESER_FAIL; }
private const string ID = "test";
private const string UNTAGGED = "* ";
private const string EOL = "\r\n";
private Deserializer? deser = null;
@ -19,6 +20,14 @@ class Geary.Imap.DeserializerTest : TestCase {
public DeserializerTest() {
base("Geary.Imap.DeserializerTest");
add_test("parse_unquoted", parse_unquoted);
add_test("parse_quoted", parse_quoted);
add_test("parse_number", parse_number);
add_test("parse_list", parse_list);
add_test("parse_response_code", parse_response_code);
add_test("parse_bad_list", parse_bad_list);
add_test("parse_bad_code", parse_bad_response_code);
add_test("gmail_greeting", gmail_greeting);
add_test("cyrus_2_4_greeting", cyrus_2_4_greeting);
add_test("aliyun_greeting", aliyun_greeting);
@ -44,6 +53,105 @@ class Geary.Imap.DeserializerTest : TestCase {
public override void tear_down() {
this.deser.stop_async.begin((obj, ret) => { async_complete(ret); });
async_result();
this.stream = null;
}
public void parse_unquoted() throws Error {
string bytes = "OK";
this.stream.add_data(UNTAGGED.data);
this.stream.add_data(bytes.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert_int(2, message.size);
assert_true(message.get(1) is UnquotedStringParameter, "Not parsed as atom");
assert_string(bytes, message.get(1).to_string());
}
public void parse_quoted() throws Error {
string bytes = "\"OK\"";
this.stream.add_data(UNTAGGED.data);
this.stream.add_data(bytes.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert_int(2, message.size);
assert_true(message.get(1) is QuotedStringParameter, "Not parsed as quoted");
assert_string(bytes, message.get(1).to_string());
}
public void parse_number() throws Error {
string bytes = "1234";
this.stream.add_data(UNTAGGED.data);
this.stream.add_data(bytes.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert_int(2, message.size);
assert_true(message.get(1) is NumberParameter, "Not parsed as number");
assert_string(bytes, message.get(1).to_string());
}
public void parse_list() throws Error {
string bytes = "(OK)";
this.stream.add_data(UNTAGGED.data);
this.stream.add_data(bytes.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert_int(2, message.size);
assert_true(message.get(1) is ListParameter, "Not parsed as list");
assert_string(bytes, message.get(1).to_string());
}
public void parse_response_code() throws Error {
string bytes = "[OK]";
this.stream.add_data(UNTAGGED.data);
this.stream.add_data(bytes.data);
this.stream.add_data(EOL.data);
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
assert_int(2, message.size);
assert_true(message.get(1) is ResponseCode, "Not parsed as response code");
assert_string(bytes, message.get(1).to_string());
}
public void parse_bad_list() throws Error {
string bytes = "(UHH";
this.stream.add_data(UNTAGGED.data);
this.stream.add_data(bytes.data);
this.stream.add_data(EOL.data);
// XXX We expect EOS here rather than DESER_FAIL since the
// deserializer currently silently ignores lines with
// malformed lists and continues parsing, so we get to the end
// of the stream.
this.process.begin(Expect.EOS, (obj, ret) => { async_complete(ret); });
this.process.end(async_result());
}
public void parse_bad_response_code() throws Error {
string bytes = "[UHH";
this.stream.add_data(UNTAGGED.data);
this.stream.add_data(bytes.data);
this.stream.add_data(EOL.data);
// XXX We expect EOS here rather than DESER_FAIL since the
// deserializer currently silently ignores lines with
// malformed lists and continues parsing, so we get to the end
// of the stream.
this.process.begin(Expect.EOS, (obj, ret) => { async_complete(ret); });
this.process.end(async_result());
}
public void gmail_greeting() throws Error {

1
test/meson.build
View file

@ -32,6 +32,7 @@ geary_test_engine_sources = [
'engine/imap/command/imap-create-command-test.vala',
'engine/imap/message/imap-data-format-test.vala',
'engine/imap/message/imap-mailbox-specifier-test.vala',
'engine/imap/parameter/imap-list-parameter-test.vala',
'engine/imap/response/imap-namespace-response-test.vala',
'engine/imap/transport/imap-deserializer-test.vala',
'engine/imap-db/imap-db-attachment-test.vala',

1
test/test-engine.vala
View file

@ -42,6 +42,7 @@ int main(string[] args) {
engine.add_suite(new Geary.Imap.DeserializerTest().get_suite());
engine.add_suite(new Geary.Imap.MailboxSpecifierTest().get_suite());
engine.add_suite(new Geary.Imap.NamespaceResponseTest().get_suite());
engine.add_suite(new Geary.Imap.ListParameterTest().get_suite());
engine.add_suite(new Geary.ImapDB.AttachmentTest().get_suite());
engine.add_suite(new Geary.ImapDB.AttachmentIoTest().get_suite());
engine.add_suite(new Geary.ImapDB.DatabaseTest().get_suite());