Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement Match module functions #116

Merged
merged 1 commit into from
Sep 23, 2015
+60 −0
Merged

implement Match module functions #116

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
60 changes: 60 additions & 0 deletions src/main/java/com/suse/saltstack/netapi/calls/modules/Match.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package com.suse.saltstack.netapi.calls.modules;

import com.google.gson.reflect.TypeToken;
import com.suse.saltstack.netapi.calls.LocalCall;

import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

/**
* salt.modules.match
*/
public class Match {
Copy link

Choose a reason for hiding this comment

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

BTW, @renner suggests to have at least have a documentation for the whole class with the URL link to it. Maybe Johannes will write more here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will open a separate issue to track the documentation of the modules where we also can discuss what to do. In any case adding documentation should be done in a separate PR that covers all modules.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for opening the issue, @lucidd! From my perspective there is no need to add javadoc for the Match module class within this patch, especially I wouldn't want to copy it from the salt documentation for the reasons mentioned in #117. For now let's just do it as the other module classes we already have, i.e. keep it as it is.


public static LocalCall<Boolean> compound(String tgt, Optional<String> minionId) {
Copy link

Choose a reason for hiding this comment

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

What is tgt and where all javadocs were yanked from the entire class?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not make sense to document the module functions since they are already documented on the salt side. The variable names are borrowed from the documentation page as well to keep it consistent. The only thing that may vary is the types if we can provide something saner on the java side.

Copy link

Choose a reason for hiding this comment

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

It makes sense to have a simple javadoc, what will popup in the IDE, so everyone can see it while programming. I don't know what means "tgt". Inability to write "target"? That Goofy Thing? Tyrese, Ginuwine and Tank R&B singers?..

Copy link
Member Author

Choose a reason for hiding this comment

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

Even if we provide documentation it would be mostly copied from the salt docs and would not be precise since our library may work with different versions of the module which could have slightly different but documented behavior.

Copy link

Choose a reason for hiding this comment

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

No
Bindings that are not documented are crappy. It is not a port of Salt in Java that we could just refer to the original documentation. E.g. ZMQ has bindings documented although they do replicate the original docs.

Copy link

Choose a reason for hiding this comment

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

You probably don't need to document it so much as in Salt, but at least you could say in one sentence what it is, what is return and what are params. I bet your IDE cries for it and checkstyle hates you for this. 😉

LinkedHashMap<String, Object> args = new LinkedHashMap<>();
args.put("tgt", tgt);
minionId.ifPresent(id -> args.put("minion_id", id));
return new LocalCall<>("match.compound", Optional.empty(),
Optional.of(args), new TypeToken<Boolean>(){});
}

public static LocalCall<Boolean> compound(String tgt) {
return compound(tgt, Optional.empty());
}

public static LocalCall<Boolean> glob(String tgt, Optional<String> minionId) {
LinkedHashMap<String, Object> args = new LinkedHashMap<>();
args.put("tgt", tgt);
minionId.ifPresent(id -> args.put("minion_id", id));
return new LocalCall<>("match.glob", Optional.empty(),
Optional.of(args), new TypeToken<Boolean>(){});
}

public static LocalCall<Boolean> glob(String tgt) {
return glob(tgt, Optional.empty());
}

public static LocalCall<Boolean> grain(String tgt, Optional<String> delimiter) {
LinkedHashMap<String, Object> args = new LinkedHashMap<>();
args.put("tgt", tgt);
delimiter.ifPresent(d -> args.put("delimiter", d));
return new LocalCall<>("match.grain", Optional.empty(),
Optional.of(args), new TypeToken<Boolean>(){});
}

public static LocalCall<Boolean> list(List<String> tgt, Optional<String> minionId) {
LinkedHashMap<String, Object> args = new LinkedHashMap<>();
args.put("tgt", tgt.stream().collect(Collectors.joining(",")));
minionId.ifPresent(id -> args.put("minion_id", id));
return new LocalCall<>("match.list", Optional.empty(),
Optional.of(args), new TypeToken<Boolean>(){});
}

public static LocalCall<Boolean> list(String... tgt) {
return list(Arrays.asList(tgt), Optional.empty());
}
}